Skip to content

Commit ec78b7d

Browse files
authored
Make the base path readable from the browser bundle (#138)
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.
1 parent fdbf663 commit ec78b7d

3 files changed

Lines changed: 99 additions & 1 deletion

File tree

app/services/sitePath.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
const rawBasePath = process.env.NEXT_BASE_PATH?.trim();
1+
// next.config republishes the normalized NEXT_BASE_PATH as
2+
// NEXT_PUBLIC_BASE_PATH so this value survives into the browser bundle. Reading
3+
// the private variable alone would make withBasePath silently wrong in a client
4+
// component: prefixed during the static render, unprefixed after hydration, and
5+
// identical in the exported HTML either way. The private variable stays as a
6+
// fallback for server-only callers and standalone scripts, which never see the
7+
// republished one.
8+
const configuredBasePath =
9+
process.env.NEXT_PUBLIC_BASE_PATH || process.env.NEXT_BASE_PATH;
10+
const rawBasePath = configuredBasePath?.trim();
211
const normalizedBasePath = rawBasePath
312
? `/${rawBasePath.replace(/^\/+|\/+$/g, "")}`
413
: "";

next.config.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ const nextConfig: NextConfig = {
1616
assetPrefix: normalizedBasePath,
1717
}
1818
: {}),
19+
// NEXT_BASE_PATH is a private build variable, so Next strips it from the
20+
// browser bundle. Anything that prefixes a path by hand - a plain anchor to
21+
// a route Next does not own, an image src - also runs in client components,
22+
// where reading it directly yields one value during the static render and
23+
// another after hydration. Republishing the normalized value under a
24+
// NEXT_PUBLIC_ name inlines it into both bundles, while deployments keep
25+
// setting the single variable they already set.
26+
env: {
27+
NEXT_PUBLIC_BASE_PATH: normalizedBasePath ?? "",
28+
},
1929
};
2030

2131
export default nextConfig;

tests/sitePath.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
3+
// sitePath reads the environment once at module scope, so each case re-imports
4+
// it after stubbing.
5+
async function importWithEnv(env: Record<string, string | undefined>) {
6+
vi.resetModules();
7+
for (const [key, value] of Object.entries(env)) {
8+
if (value !== undefined) {
9+
vi.stubEnv(key, value);
10+
}
11+
}
12+
return (await import('../app/services/sitePath')).withBasePath;
13+
}
14+
15+
afterEach(() => {
16+
vi.unstubAllEnvs();
17+
vi.resetModules();
18+
});
19+
20+
describe('withBasePath', () => {
21+
it('returns the path unchanged when no base path is configured', async () => {
22+
const withBasePath = await importWithEnv({});
23+
24+
expect(withBasePath('/blogs/')).toBe('/blogs/');
25+
});
26+
27+
// The bug this guards: NEXT_BASE_PATH is private, so Next strips it from the
28+
// browser bundle. A client component reading it would prefix during the
29+
// static render and not after hydration. next.config republishes the value
30+
// under a NEXT_PUBLIC_ name, which is what must be read.
31+
it('reads the republished public variable, which survives into the browser bundle', async () => {
32+
const withBasePath = await importWithEnv({
33+
NEXT_PUBLIC_BASE_PATH: '/preview',
34+
});
35+
36+
expect(withBasePath('/blogs/')).toBe('/preview/blogs/');
37+
});
38+
39+
it('falls back to the private variable for server-only callers and scripts', async () => {
40+
const withBasePath = await importWithEnv({ NEXT_BASE_PATH: '/preview' });
41+
42+
expect(withBasePath('/blogs/')).toBe('/preview/blogs/');
43+
});
44+
45+
it('agrees whichever variable carries the value', async () => {
46+
const fromPublic = await importWithEnv({ NEXT_PUBLIC_BASE_PATH: '/preview' });
47+
const publicResult = fromPublic('/images/logo.png');
48+
49+
vi.unstubAllEnvs();
50+
const fromPrivate = await importWithEnv({ NEXT_BASE_PATH: '/preview' });
51+
52+
expect(publicResult).toBe(fromPrivate('/images/logo.png'));
53+
});
54+
55+
it('normalizes surrounding slashes', async () => {
56+
const withBasePath = await importWithEnv({
57+
NEXT_PUBLIC_BASE_PATH: 'preview/',
58+
});
59+
60+
expect(withBasePath('/blogs/')).toBe('/preview/blogs/');
61+
});
62+
63+
it('treats an empty republished value as no base path', async () => {
64+
const withBasePath = await importWithEnv({ NEXT_PUBLIC_BASE_PATH: '' });
65+
66+
expect(withBasePath('/blogs/')).toBe('/blogs/');
67+
});
68+
69+
it('leaves relative and absolute URLs alone', async () => {
70+
const withBasePath = await importWithEnv({
71+
NEXT_PUBLIC_BASE_PATH: '/preview',
72+
});
73+
74+
expect(withBasePath('blogs/')).toBe('blogs/');
75+
expect(withBasePath('https://example.com/logo.png')).toBe(
76+
'https://example.com/logo.png',
77+
);
78+
});
79+
});

0 commit comments

Comments
 (0)