From 503f80a05e5b058490f5b548364d6322e2d9b3ca Mon Sep 17 00:00:00 2001 From: Guanzhou Song Date: Mon, 3 Aug 2026 20:47:04 -0400 Subject: [PATCH] Make the base path readable from the browser bundle 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 (#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. --- app/services/sitePath.ts | 11 +++++- next.config.ts | 10 +++++ tests/sitePath.test.ts | 79 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/sitePath.test.ts diff --git a/app/services/sitePath.ts b/app/services/sitePath.ts index befab1f..d60ea0f 100644 --- a/app/services/sitePath.ts +++ b/app/services/sitePath.ts @@ -1,4 +1,13 @@ -const rawBasePath = process.env.NEXT_BASE_PATH?.trim(); +// next.config republishes the normalized NEXT_BASE_PATH as +// NEXT_PUBLIC_BASE_PATH so this value survives into the browser bundle. Reading +// the private variable alone would make withBasePath silently wrong in a client +// component: prefixed during the static render, unprefixed after hydration, and +// identical in the exported HTML either way. The private variable stays as a +// fallback for server-only callers and standalone scripts, which never see the +// republished one. +const configuredBasePath = + process.env.NEXT_PUBLIC_BASE_PATH || process.env.NEXT_BASE_PATH; +const rawBasePath = configuredBasePath?.trim(); const normalizedBasePath = rawBasePath ? `/${rawBasePath.replace(/^\/+|\/+$/g, "")}` : ""; diff --git a/next.config.ts b/next.config.ts index 1d24560..0fc40b9 100644 --- a/next.config.ts +++ b/next.config.ts @@ -16,6 +16,16 @@ const nextConfig: NextConfig = { assetPrefix: normalizedBasePath, } : {}), + // NEXT_BASE_PATH is a private build variable, so Next strips it from the + // browser bundle. Anything that prefixes a path by hand - a plain anchor to + // a route Next does not own, an image src - also runs in client components, + // where reading it directly yields one value during the static render and + // another after hydration. Republishing the normalized value under a + // NEXT_PUBLIC_ name inlines it into both bundles, while deployments keep + // setting the single variable they already set. + env: { + NEXT_PUBLIC_BASE_PATH: normalizedBasePath ?? "", + }, }; export default nextConfig; diff --git a/tests/sitePath.test.ts b/tests/sitePath.test.ts new file mode 100644 index 0000000..dd25027 --- /dev/null +++ b/tests/sitePath.test.ts @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// sitePath reads the environment once at module scope, so each case re-imports +// it after stubbing. +async function importWithEnv(env: Record) { + vi.resetModules(); + for (const [key, value] of Object.entries(env)) { + if (value !== undefined) { + vi.stubEnv(key, value); + } + } + return (await import('../app/services/sitePath')).withBasePath; +} + +afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); +}); + +describe('withBasePath', () => { + it('returns the path unchanged when no base path is configured', async () => { + const withBasePath = await importWithEnv({}); + + expect(withBasePath('/blogs/')).toBe('/blogs/'); + }); + + // The bug this guards: NEXT_BASE_PATH is private, so Next strips it from the + // browser bundle. A client component reading it would prefix during the + // static render and not after hydration. next.config republishes the value + // under a NEXT_PUBLIC_ name, which is what must be read. + it('reads the republished public variable, which survives into the browser bundle', async () => { + const withBasePath = await importWithEnv({ + NEXT_PUBLIC_BASE_PATH: '/preview', + }); + + expect(withBasePath('/blogs/')).toBe('/preview/blogs/'); + }); + + it('falls back to the private variable for server-only callers and scripts', async () => { + const withBasePath = await importWithEnv({ NEXT_BASE_PATH: '/preview' }); + + expect(withBasePath('/blogs/')).toBe('/preview/blogs/'); + }); + + it('agrees whichever variable carries the value', async () => { + const fromPublic = await importWithEnv({ NEXT_PUBLIC_BASE_PATH: '/preview' }); + const publicResult = fromPublic('/images/logo.png'); + + vi.unstubAllEnvs(); + const fromPrivate = await importWithEnv({ NEXT_BASE_PATH: '/preview' }); + + expect(publicResult).toBe(fromPrivate('/images/logo.png')); + }); + + it('normalizes surrounding slashes', async () => { + const withBasePath = await importWithEnv({ + NEXT_PUBLIC_BASE_PATH: 'preview/', + }); + + expect(withBasePath('/blogs/')).toBe('/preview/blogs/'); + }); + + it('treats an empty republished value as no base path', async () => { + const withBasePath = await importWithEnv({ NEXT_PUBLIC_BASE_PATH: '' }); + + expect(withBasePath('/blogs/')).toBe('/blogs/'); + }); + + it('leaves relative and absolute URLs alone', async () => { + const withBasePath = await importWithEnv({ + NEXT_PUBLIC_BASE_PATH: '/preview', + }); + + expect(withBasePath('blogs/')).toBe('blogs/'); + expect(withBasePath('https://example.com/logo.png')).toBe( + 'https://example.com/logo.png', + ); + }); +});