Uh oh!
There was an error while loading. Please reload this page.
Make the base path readable from the browser bundle - #138
Merged
guanzhousongmicrosoft merged 1 commit intoAug 4, 2026
Merged
Conversation
withBasePath reads NEXT_BASE_PATH, which is a private build variable, so Next strips it from the browser bundle. Navbar is a client component and calls it at module scope for the blogs anchor and the logo image: under a configured base path the static render produced /preview/blogs/, and the same code after hydration produced /blogs/, changing the href under the reader and leaving React a mismatch to reconcile. Nothing on disk shows this. The export is written by the server-side render, which reads the variable correctly, so the emitted markup is right in both the broken and the fixed build - only a real browser disagrees. That is what made it survive review twice. next.config now republishes the normalized value as NEXT_PUBLIC_BASE_PATH, which Next inlines into both bundles, and sitePath prefers it while keeping the private variable as a fallback for server-only callers and standalone scripts that never see the republished one. Deployments still set the single variable they already set, and every existing caller is fixed without being touched, including any added later. Covered with the environment stubbed both ways: the public variable, the private fallback, agreement between them, slash normalization, an empty republished value meaning no base path, and relative and absolute URLs left alone. Exercised directly under node across all four combinations of the two variables. The vitest suite is left to CI, since npm install fails against the registry proxy on this machine. Found while fixing the same defect in the Markdown link resolver (documentdb#137), which solves it differently - that path is an internal route, so it can hand the prefixing to next/link and read no environment at all.
guanzhousongmicrosoft
approved these changes
Aug 4, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The defect
withBasePathreadsNEXT_BASE_PATH, which is a private build variable — Next strips it from the browser bundle, whereprocess.env.NEXT_BASE_PATHcompiles toundefined.Navbaris a client component and calls it at module scope, twice:Under a configured base path the static render emits
/preview/blogs/; the same code after hydration emits/blogs/. The href changes under the reader, with a hydration mismatch alongside it.Why it survived review twice
Nothing on disk shows it. The export is written by the server-side render, which reads the variable correctly, so the emitted markup is identical in the broken and fixed builds. A build-level assertion would pass either way — which is also why I have not added one. Only a real browser disagrees.
The same trap caught unit tests in #137: vitest runs in node, so tests exercised the one environment where the bug does not exist and went green while asserting the wrong thing.
The fix
next.configrepublishes the normalized value asNEXT_PUBLIC_BASE_PATH, which Next inlines into both bundles.sitePathprefers it, keeping the private variable as a fallback for server-only callers and standalone scripts that never see the republished one.Deployments keep setting the single variable they already set. Every existing caller is fixed without being touched, including any added later — which matters more than the two current call sites, since the failure mode is silent.
Coverage
tests/sitePath.test.ts, with the environment stubbed both ways: the public variable, the private fallback, agreement between the two, slash normalization, an empty republished value meaning no base path, and relative and absolute URLs left alone.Verified directly under node across all four combinations of the two variables. The suite itself is left to CI —
npm cifails against the registry proxy on my machine.Relationship to #137
Found while fixing the same defect in the Markdown link resolver. That one is solved differently and deliberately: a resolved document link is an internal route, so it hands prefixing to
next/linkand reads no environment at all. This PR covers the cases that cannot do that — an anchor to/blogs/, which Next does not route, and an imagesrc.The two are independent and can merge in either order.