-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Next Router Middleware Support #3690
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Ignored Deployments
|
|
Benchmark for 7452089
Click to view full benchmark
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test case to next-dev-tests
24afad8
to
667421f
Compare
Added tests (and addressed some review comments), but the tests are failing because Next.js doesn't handle the middleware yet. |
Benchmark for 84aaa14
Click to view full benchmark
|
Benchmark for c317177
Click to view full benchmark
|
fd6d752
to
5b6ccbe
Compare
Benchmark for e537c77
Click to view full benchmark
|
Benchmark for 3a15a31
Click to view full benchmark
|
return await makeResolver(dir, nextConfig); | ||
const edgeInfo = { | ||
name: "edge", | ||
paths: middlewareChunkGroup.map((chunk: string) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be called files
and be relative to the distDir
to match our middleware plugin
x-ref: https://github.com/vercel/next.js/blob/7d8f85bd8dbd767dfac0d92eed651877268e62cb/packages/next/src/build/webpack/plugins/middleware-plugin.ts#L35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this from
turbo/crates/next-core/js/src/entry/server-edge-api.tsx
Lines 53 to 59 in 805a5ae
const edgeInfo = { | |
name: "edge", | |
paths: chunkGroup.map((chunk: string) => join(process.cwd(), chunk)), | |
wasm: [], | |
env: [], | |
assets: [], | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I aligned with the return value of getEdgeFunctionInfo
, which converts the format in the manifest file into one with paths
So when this info object is provided it could skip calling getEdgeFunctionInfo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, yeah aligning on that return type seems alright
Removing the test case while we wait on Next to land the middleware changes. Once we update to the next release, I'll readd the test. |
782ac89
to
618617c
Compare
Benchmark for f005173
Click to view full benchmark
|
…jjsweb.site> Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com> # New Features - vercel/turbo#3771 - vercel/turbo#3690 # Performance - vercel/turbo#3768 # Fixes - vercel/turbo#3795 - vercel/turbo#3746 - vercel/turbo#3832 - vercel/turbo#3827 - vercel/turbo#3847 # Other - vercel/turbo#3803 - vercel/turbo#3685 - vercel/turbo#3848 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site> Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
This updates our Next.js router, passing the
edgeInfo
manifest generated from themiddleware.js
file (or any other configured page extension).Fixes WEB-277
Fixes WEB-370