Deprecate nested Middleware in favor of root middleware #36772
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javivelasco
requested review from
timneutkens,
ijjk,
shuding,
padmaia and
huozhi
as code owners
javivelasco
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
javivelasco
force-pushed
the
root-middleware-handling
branch
3 times, most recently
from
3abe43f to
38f006b
Compare
timneutkens
reviewed
May 10, 2022
There was a problem hiding this comment.
Looks good to me but @ijjk should review the routing parts of the server 👍
javivelasco
force-pushed
the
root-middleware-handling
branch
2 times, most recently
from
97351e3 to
56dc9e1
Compare
1 task
javivelasco
force-pushed
the
root-middleware-handling
branch
from
56dc9e1 to
107ae0e
Compare
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
ijjk
reviewed
May 17, 2022
| getMiddlewareRegex(middleware, { | ||
| catchAll: !isSSR, | ||
| }) | ||
| )(cleanedAs) | ||
| }) | ||
|
|
||
| if (!requiresPreflight) { |
There was a problem hiding this comment.
Can the above code/checks all be removed in favor a single flag like process.env.__NEXT_HAS_ROOT_MIDDLEWARE which is set via the webpack config here?
next.js/packages/next/build/webpack-config.ts
Line 1522 in c72d32d
There was a problem hiding this comment.
Since this is a quite big change already I'd love to do that in a follow-up PR if that's ok for you @ijjk WDYT?
There was a problem hiding this comment.
Yeah we can do that in a follow-up 👍
javivelasco
force-pushed
the
root-middleware-handling
branch
from
15dc442 to
ff8dfc6
Compare
Co-authored-by: Delba de Oliveira <32464864+delbaoliveira@users.noreply.github.com> Update errors/nested-middleware.md Co-authored-by: Delba de Oliveira <32464864+delbaoliveira@users.noreply.github.com> Update errors/nested-middleware.md Co-authored-by: Delba de Oliveira <32464864+delbaoliveira@users.noreply.github.com> Update errors/nested-middleware.md Co-authored-by: Delba de Oliveira <32464864+delbaoliveira@users.noreply.github.com> Update errors/nested-middleware.md Co-authored-by: Delba de Oliveira <32464864+delbaoliveira@users.noreply.github.com>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
javivelasco
force-pushed
the
root-middleware-handling
branch
from
b2fd604 to
a7bf05e
Compare
javivelasco
force-pushed
the
root-middleware-handling
branch
from
a7bf05e to
a23dbe5
Compare
ijjk
approved these changes
May 19, 2022
3 tasks
1 task
This was referenced May 30, 2022
|
@javivelasco the filename not always starts with “middleware“. Actually, it starts with root directory.
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR deprecates declaring a middleware under
pagesin favour of the project root naming it aftermiddlewareinstead of_middleware. This is in the context of having a simpler execution model for middleware and also ships some refactor work. There is a ton of a code to be simplified after this deprecation but I think it is best to do it progressively.With this PR, when in development, we will fail whenever we find a nested middleware but we do not include it in the compiler so if the project is using it, it will no longer work. For production we will fail too so it will not be possible to build and deploy a deprecated middleware. The error points to a page that should also be reviewed as part of documentation.
Aside from the deprecation, this migrates all middleware tests to work with a single middleware. It also splits tests into multiple folders to make them easier to isolate and work with. Finally it ships some small code refactor and simplifications.