-
Notifications
You must be signed in to change notification settings - Fork 91
fix: next 16 adjustments #3155
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
fix: next 16 adjustments #3155
Changes from 15 commits
68eed5c
817975d
6aac5e8
f83a27a
91b5687
c9db2ce
09a8d76
cb2128f
e78f851
a67d101
3041ba5
44c9e7b
a109f74
1bec12c
cacbce2
d1ec9d2
090185c
db8b606
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,7 +106,7 @@ export const copyNextServerCode = async (ctx: PluginContext): Promise<void> => { | |
`server/*`, | ||
`server/chunks/**/*`, | ||
`server/edge-chunks/**/*`, | ||
`server/edge/chunks/**/*`, | ||
`server/edge/**/*`, | ||
`server/+(app|pages)/**/*.js`, | ||
], | ||
{ | ||
|
@@ -291,6 +291,8 @@ async function patchNextModules( | |
export const copyNextDependencies = async (ctx: PluginContext): Promise<void> => { | ||
await tracer.withActiveSpan('copyNextDependencies', async () => { | ||
const entries = await readdir(ctx.standaloneDir) | ||
const filter = ctx.constants.IS_LOCAL ? undefined : nodeModulesFilter | ||
|
||
const promises: Promise<void>[] = entries.map(async (entry) => { | ||
// copy all except the distDir (.next) folder as this is handled in a separate function | ||
// this will include the node_modules folder as well | ||
|
@@ -299,7 +301,6 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> => | |
} | ||
const src = join(ctx.standaloneDir, entry) | ||
const dest = join(ctx.serverHandlerDir, entry) | ||
const filter = ctx.constants.IS_LOCAL ? undefined : nodeModulesFilter | ||
await cp(src, dest, { | ||
recursive: true, | ||
verbatimSymlinks: true, | ||
|
@@ -321,7 +322,7 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> => | |
// see: https://github.com/vercel/next.js/issues/50072 | ||
if (existsSync(rootSrcDir) && ctx.standaloneRootDir !== ctx.standaloneDir) { | ||
promises.push( | ||
cp(rootSrcDir, rootDestDir, { recursive: true, verbatimSymlinks: true }).then(() => | ||
cp(rootSrcDir, rootDestDir, { recursive: true, verbatimSymlinks: true, filter }).then(() => | ||
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is for:
previous fix only applied to non-monorepos - this applies it in morepos as well |
||
recreateNodeModuleSymlinks(resolve('node_modules'), rootDestDir), | ||
), | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ test('should serve 404 page when requesting non existing page (no matching route | |
const headers = response?.headers() || {} | ||
expect(response?.status()).toBe(404) | ||
|
||
expect(await page.textContent('h1')).toBe('404') | ||
await expect(page.locator('h1')).toHaveText('404') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shared for all I thought this was causing problems in at least some of our tests. This ended up not being problem, but as I already migrated away from it, might as well keep it in this PR I think |
||
|
||
// https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, | ||
// after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that it | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,6 +246,11 @@ for (const { expectedRuntime, isNodeMiddleware, label, testWithSwitchableMiddlew | |
const pageResponse = await page.goto(`${edgeOrNodeMiddlewarePages.url}/link`) | ||
expect(await pageResponse?.headerValue('x-runtime')).toEqual(expectedRuntime) | ||
|
||
// wait for hydration to finish before doing client navigation | ||
await expect(page.getByTestId('hydration')).toHaveText('hydrated', { | ||
timeout: 10_000, | ||
}) | ||
|
||
Comment on lines
+249
to
+253
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There seemed to be a problem where test was clicking links too fast (before hydration happened) so the rest of the test expecting |
||
await page.evaluate(() => { | ||
// set some value to window to check later if browser did reload and lost this state | ||
;(window as ExtendedWindow).didReload = false | ||
|
@@ -305,6 +310,11 @@ for (const { expectedRuntime, isNodeMiddleware, label, testWithSwitchableMiddlew | |
) | ||
expect(await pageResponse?.headerValue('x-runtime')).toEqual(expectedRuntime) | ||
|
||
// wait for hydration to finish before doing client navigation | ||
await expect(page.getByTestId('hydration')).toHaveText('hydrated', { | ||
timeout: 10_000, | ||
}) | ||
|
||
await page.evaluate(() => { | ||
// set some value to window to check later if browser did reload and lost this state | ||
;(window as ExtendedWindow).didReload = false | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
this is for:
we do need to bundle
edge/assets
at least (it's needed for@vercel/og
/next/og
) supportThere 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.
Wondering if it would be preferred to add
server/edge/assets/**/*
to the list instead of editing theserver/edge/chunks
entryThere 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.
Debugging this took quite a while as original error was looking like so
It took peeling few layers to understand what's wrong and few dead ends along the way. Generally all those files are recomended to be bundled. In initial PR for turbopack support I also attempted to be conservative with what we bundle and this is how we ended up with just
edge/chunks
. I think this shown me that there are just cases that I don't know about and that debugging them is painful.This of course is tradeoff between supporting cases we potentially don't know about today (other than
@vercel/og
case) vs potential of getting into lambda too large cases. At least latter ones we have some ideas to make easier to debug if we learn that we start bundling too much