fix: register the router server context under the build-time project dir - #1229
Open
rome2o wants to merge 1 commit into
Open
fix: register the router server context under the build-time project dir#1229rome2o wants to merge 1 commit into
rome2o wants to merge 1 commit into
Conversation
Next.js delegates pages router `notFound` results to `routerServerContext.render404`, falling back to a bare "This page could not be found" body when that context is missing. `NextNodeServer` registers the context itself, but keys it on `path.relative(process.cwd(), server.dir)`, while the route modules read it back using the `relativeProjectDir` baked in at build time, which is always an empty string here. The two only line up because `server-adapter.ts` calls `process.chdir(__dirname)` on cold start, so a core rendering path depends on the working directory being changed by the adapter. Adapters that cannot change the working directory register the context under a key nothing reads and serve the fallback body instead of the app's 404 page. Register the context upfront under the build-time key so it no longer depends on the working directory. There is no behaviour change on AWS: the entry lands under the same key Next.js already used there, and Next.js still augments it per request.
🦋 Changeset detectedLatest commit: 6c0e3b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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
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.
What
Register the Next.js router server context in the shared core, under the project directory key the route modules actually read.
Why
Next.js does not render pages router
notFoundresults in the route module itself. It delegates torouterServerContext.render404, and falls back to a bareThis page could not be foundbody when that context is missing (pages-handler.js).NextNodeServerdoes register that context, but the write key and the read key are computed differently:handleCatchallRenderRequest:path.relative(process.cwd(), this.dir)relativeProjectDirbaked in at build time, which is always""hereThose only agree when the working directory is the server directory. On AWS they do agree, because
adapters/server-adapter.tscallsprocess.chdir(__dirname)on cold start. So the behaviour of a core rendering path depends on an adapter levelchdirworkaround.Adapters that cannot change the working directory do not get that. On Cloudflare Workers there is no
process.chdir,__dirnameis bundled as"", andsetNextjsServerWorkingDirectoryis patched out (opennextjs/opennextjs-cloudflare#899). The context is written under".."while the route modules read"", sorender404is never found and everynotFoundresult serves the fallback body instead of the app's 404 page.What changed
packages/open-next/src/core/util.tsregisters the context immediately after constructingNextServer, keyed on the build-timerelativeProjectDir. It no longer depends on the working directory, and it runs beforeserver-adapter.tscallschdirrather than after.Also adds a
ssr-not-foundpage to the pages router example and an e2e test, so the 404 path is covered rather than only the "route does not exist" path that404.test.tsalready covers.No behaviour change on AWS
The entry lands under
"", the same key Next.js already resolves to there, and Next.js still augments it per request. In particularisWrappedByNextServeris unaffected: Next.js writes that flag onto the same entry on every render request, so pre-registering cannot suppress it.Verification
AWS,
examples/pages-routerbuilt withopenbuild:local:/ssr-not-found/returns404 text/html, 2278 bytes, containing__NEXT_DATA__, rather than the fallback body/api/dynamic/hello-world-123returns{"slug":"hello-world-123"},/api/dynamic/catch-all/a/breturns{"slug":["a","b"]}tests-unitgreen, biome andtsc --noEmitcleanCloudflare, to confirm this fixes the adapter that is actually broken. I overlaid this build of
core/util.jsintoopennextjs-cloudflare's vendored@opennextjs/aws, disabled the build patch from opennextjs/opennextjs-cloudflare#1346 entirely, and rebuilt the worker:/ssr-not-foundreturns 404 HTML with__NEXT_DATA__on the first request into a fresh isolatepages-routere2e suite: 37 passed, 1 skipped, including #1346's own testSo this change alone fixes Cloudflare and opennextjs/opennextjs-cloudflare#1346's ast-grep patch can be dropped once this lands. That PR is open against the Cloudflare adapter and I will close it in favour of this one.
Ref: opennextjs/opennextjs-cloudflare#1346, where @vicb suggested the fix belongs here.