fix: next 16 adjustments #3155
Merged
fix: next 16 adjustments #3155
Conversation
This file contains hidden or 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
…ore clicking links
pieh
added
the
test all versions
pieh
commented
Oct 6, 2025
| `server/chunks/**/*`, | ||
| `server/edge-chunks/**/*`, | ||
| `server/edge/chunks/**/*`, | ||
| `server/edge/**/*`, |
There was a problem hiding this comment.
this is for:
fix: bundle edge-runtime assets for turbopack builds
we do need to bundle edge/assets at least (it's needed for @vercel/og / next/og) support
There was a problem hiding this comment.
Wondering if it would be preferred to add server/edge/assets/**/* to the list instead of editing the server/edge/chunks entry
There was a problem hiding this comment.
Debugging this took quite a while as original error was looking like so
stderr | tests/integration/wasm.test.ts > 'wasm-src' > should work in app route
⨯ Error: failed to pipe response
at ignore-listed frames {
[cause]: TypeError: fetch failed
at ignore-listed frames {
[cause]: Error: invalid method
at ignore-listed frames
}
}
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
pieh
commented
Oct 6, 2025
| 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(() => |
There was a problem hiding this comment.
this is for:
fix: exclude musl binaries from function bundle when building on Netlify with pnpm monorepos
previous fix only applied to non-monorepos - this applies it in morepos as well
📊 Package size report -0%↓
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
commented
Oct 6, 2025
| 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.
shared for all page.textContent replacements - it's discouraged and deprecated - see https://playwright.dev/docs/api/class-page#page-text-content
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
pieh
commented
Oct 6, 2025
Comment on lines
+249
to
+253
| // wait for hydration to finish before doing client navigation | ||
| await expect(page.getByTestId('hydration')).toHaveText('hydrated', { | ||
| timeout: 10_000, | ||
| }) | ||
|
|
There was a problem hiding this comment.
There seemed to be a problem where test was clicking links too fast (before hydration happened) so the rest of the test expecting next/link navigation was being very flaky. I did add useEffect in fixture to allow tests to wait for hydration to happen here
pieh
commented
Oct 6, 2025
| Thumbs.db | ||
|
|
||
| .nx/cache | ||
| .nx/workspace-data |
There was a problem hiding this comment.
NX dependencies in nx fixture had to be updated to work with next@16, I did run upgrade migration and as a result there were some additional changes applied to fixture
pieh
commented
Oct 6, 2025
pieh
commented
Oct 6, 2025
Comment on lines
+72
to
+75
| // on failures we don't delete the deploy, but we do cleanup the fixture from filesystem in CI | ||
| if (process.env.CI) { | ||
| return cleanup(isolatedFixtureRoot, undefined) | ||
| } |
There was a problem hiding this comment.
not strictly needed now, but on repeated failures github action runners were running out of disk space because those isoloated fixtures were not being deleted
pieh
force-pushed
the
next-16-adjustments
branch
from
b5083d6 to
db8b606
Compare
mrstork
reviewed
Oct 6, 2025
| export const EDGE_MIDDLEWARE_FUNCTION_NAME = '___netlify-edge-handler-middleware' | ||
| export const EDGE_MIDDLEWARE_SRC_FUNCTION_NAME = '___netlify-edge-handler-src-middleware' | ||
| // Turbopack has different output than webpack | ||
| export const EDGE_MIDDLEWARE_SRC_FUNCTION_NAME = hasDefaultTurbopackBuilds() |
There was a problem hiding this comment.
Turbopack changes the name of the edge handler? Or is this more about the next.js version?
There was a problem hiding this comment.
Turbopack changes the name of the edge handler?
Indirectly. We use name from middleware-manifest.json to produce our handler name -
opennextjs-netlify/src/build/functions/edge.ts
Lines 309 to 310 in 4aaeb79
In webpack builds:
- if
middleware.(js|ts)file is in root of workspace - the name in manifest will bemiddleware - if
middleware.(js|ts)file is insrcdirectory of workspace - the name in manifest will besrc/middleware
In turbopack the name in manifest is always middleware regardless if file is in root or in src
mrstork
approved these changes
Oct 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Next.js canary switched builds to turbopack by default, which exposed some quirks that were not noticed in hello-world type of test fixture we used before. This PR applies some fixes, but most of it are test adjustments
Override below is attempt to create 2 entries in changelog, as there are 2 separate fixes in this PR
BEGIN_COMMIT_OVERRIDE
fix: bundle edge-runtime assets for turbopack builds
fix: exclude
muslbinaries from function bundle when building on Netlify withpnpmmonoreposEND_COMMIT_OVERRIDE