fix: incorrect path of middleware nft in setups using base directory #3262
+100
−13
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
Contributor
📊 Package size report -0%↓
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
commented
Nov 19, 2025
| const entry = 'server/middleware.js' | ||
| const nft = `${entry}.nft.json` | ||
| const nftFilesPath = join(process.cwd(), ctx.distDir, nft) | ||
| const nftFilesPath = join(ctx.publishDir, nft) |
Contributor
Author
There was a problem hiding this comment.
This is second attempt at fixing this path (previous one was in #3211 ).
This time using publishDir as it already should point to absolute path to .next.
We already use publishDir in middleware handling to find relevant manifest files:
opennextjs-netlify/src/build/plugin-context.ts
Lines 262 to 286 in 6451ec3
and since those work (we wouldn't get to code path trying to read
.nft file if they didn't), it's good idea to just reuse this method.
Some explanation why current method is not working in some cases:
In case of monorepos that are configured to use base directory instead of packagePath, the CWD will be workspace and not root of monorepo. The ctx.distDir will be path from monorepo root to .next directory (so will be prefixed with base directory). Both combined means that base directory is repeated in produced final path. Using example with repo that has app workspace and Netlify setup using base directory set to app:
// before
cwd: '/opt/build/repo/app'
distDir: 'app/.next', // <- this is produced using some values from Next.js manifest files, in particular `relativeAppDir` from `required-server-files.json`, but this path is not `base` aware and shouldn't be used if we are reading files outside of `.next/standalone` directory
join(cwd, distDir): '/opt/build/repo/app/app/.next' // <-- duplicated `app` dir leading to fatal "Error: ENOENT: no such file or directory, open '/opt/build/repo/app/app/.next/server/middleware.js.nft.json'" error
// with this change
publishDir: '/opt/build/repo/app/.next' // <- correct path
Note: with last fix attempt we did add smoke fixture using packagePath. In this PR I add another smoke fixture replicating monorepo setup using base directory instead of packagePath. Both are passing with the changes
…d of package path and containing proxy / node middleware
pieh
force-pushed
the
michalpiechowiak/frb-2052-fix-nextjs-middlewareproxy-bundling-in-pnpm-base-directory
branch
from
d5b1cac to
a10748f
Compare
pieh
commented
Nov 19, 2025
| publishDirectory: '.next', | ||
| runtimeInstallationPath: 'app', | ||
| smoke: true, | ||
| useBuildbot: true, |
Contributor
Author
There was a problem hiding this comment.
This is annoying because we don't see build failure logs when using buildbot, but I couldn't replicate the exact conditions with CLI deploys.
https://app.netlify.com/projects/next-runtime-testing/deploys/691db94c23e26600e071528e (this might get automatically deleted soon!) is example build reproducing
1:34:41 PM: Error message
1:34:41 PM: Error: ENOENT: no such file or directory, open '/opt/build/repo/app/app/.next/server/middleware.js.nft.json'
kind of errors without any runtime code changes (1st commit) that is fixed by runtime changes (2nd commit)
pieh
commented
Nov 19, 2025
| 'Content-Type': 'application/zip', | ||
| Authorization: `Bearer ${process.env.NETLIFY_AUTH_TOKEN}`, | ||
| const result = await fetch( | ||
| `https://api.netlify.com/api/v1/sites/${siteId}/builds?clear_cache=true`, |
Contributor
Author
There was a problem hiding this comment.
Added clear_cache=true here. We used buildbot zip builds for skew protection only so far which creates its own temporary sites, so build cache was not a problem.
However with this test we use shared next-runtime-testing site, so we do potentially get a build cache which seems to prevent using npm-packed runtime and instead use auto-installed one without code changes from pull request
pieh
commented
Nov 19, 2025
Comment on lines
+360
to
+361
| entry.includes('node_modules') || | ||
| entry.includes('.git') || |
Contributor
Author
There was a problem hiding this comment.
new fixture has nested directory with node_modules, so this ensures we skip zipping those
pieh
commented
Nov 19, 2025
Comment on lines
231
to
259
| let relativePathToPackage = relative( | ||
| join(isolatedFixtureRoot, siteRelDir), | ||
| join(isolatedFixtureRoot, packageName), | ||
| ) | ||
| if (!relativePathToPackage.startsWith('.')) { | ||
| relativePathToPackage = `./${relativePathToPackage}` | ||
| } | ||
|
|
||
| switch (packageManger) { | ||
| case 'npm': | ||
| command = `npm install --ignore-scripts --no-audit --legacy-peer-deps ${packageName} ${ | ||
| workspaceRelPath ? `-w ${workspaceRelPath}` : '' | ||
| }` | ||
| break | ||
| case 'yarn': | ||
| command = `yarn ${workspaceName ? `workspace ${workspaceName}` : '-W'} add file:${join( | ||
| isolatedFixtureRoot, | ||
| packageName, | ||
| )} --ignore-scripts` | ||
| break | ||
| case 'berry': | ||
| command = `yarn ${workspaceName ? `workspace ${workspaceName}` : ''} add ${join( | ||
| isolatedFixtureRoot, | ||
| packageName, | ||
| )}` | ||
| env['YARN_ENABLE_SCRIPTS'] = 'false' | ||
| break | ||
| case 'pnpm': | ||
| command = `pnpm add file:${join(isolatedFixtureRoot, packageName)} ${ | ||
| command = `pnpm add file:${relativePathToPackage} ${ |
Contributor
Author
There was a problem hiding this comment.
relative path needed for buildbot to actually find tarballed runtime in zip file - this possibly will have to be replicated for other package managers if we will need to use buildbot for those (I only did pnpm here, npm already worked when buildbot builds were added for skew protection)
pieh
commented
Nov 19, 2025
| @@ -0,0 +1,7 @@ | |||
| [build] | |||
| base = "app" | |||
Contributor
Author
There was a problem hiding this comment.
Most important part of this fixture is using base directory
pieh
deleted the
michalpiechowiak/frb-2052-fix-nextjs-middlewareproxy-bundling-in-pnpm-base-directory
branch
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
Details in #3262 (review)
Tests
Added test case for monorepo setup that is using base directory instead of package path and use proxy / node middleware