fix(ssg): avoid hydration mismatches on static root redirects#3964
fix(ssg): avoid hydration mismatches on static root redirects#3964Mat4m0 wants to merge 2 commits intonuxt-modules:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new fixture for issue Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
specs/fixtures/issues/3988/package.json (1)
9-12: Consider pinning fixture dependency versions for reproducibility.Using
"latest"fornuxtand@nuxtjs/i18nin a test fixture can make CI results non-deterministic if a future release regresses the behavior this fixture pins down. If other fixtures already use"latest", this is consistent; otherwise, pinning to known-good versions would make test failures easier to bisect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/fixtures/issues/3988/package.json` around lines 9 - 12, The devDependencies in package.json currently use floating tags ("nuxt" and "@nuxtjs/i18n" set to "latest") which makes the fixture non-deterministic; change those two entries in package.json (the "nuxt" and "@nuxtjs/i18n" keys under devDependencies) to explicit, pinned semantic versions (e.g., "x.y.z") known-good for the fixture, then regenerate the lockfile (npm/yarn/pnpm install) so CI uses reproducible installs; if other fixtures intentionally use "latest", either align them or add a comment in this fixture explaining why it must be pinned.src/runtime/plugins/ssg-detect.ts (1)
18-24: Minor:ctx.setCookieLocale(locale)call on line 22 is likely redundant.
loadAndSetLocale(defaultsyncCookie: true) routes throughctx.setLocaleSuspend, which already invokesctx.setCookieLocale(locale)inside__resolvePendingLocalePromise(seesrc/runtime/context.ts:118-149). Leaving the explicit call is harmless and defensively guarantees the cookie is written even if the short-circuit atloadAndSetLocale’s top is hit (when detected === current locale and no pending switch), so it can stay — but a brief comment noting that intent would help future readers, or drop it if you prefer to rely solely on the suspend path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/plugins/ssg-detect.ts` around lines 18 - 24, The explicit ctx.setCookieLocale(locale) after loadAndSetLocale is redundant because loadAndSetLocale (via ctx.setLocaleSuspend -> __resolvePendingLocalePromise) already writes the cookie; either remove the ctx.setCookieLocale(locale) call in the nuxt.hook block or, if you want the defensive write retained, add a one-line comment above it explaining that loadAndSetLocale/suspend path already sets the cookie but this call is kept as a fallback. Locate the call in the nuxt.hook where detectLocale, loadAndSetLocale, and navigate are used and apply one of the two changes to keep intent clear.specs/issues/2790.spec.ts (3)
53-66: Error path swallows the real failure and uses 500 for not-found.The
catchblock returns 500 for every failure includingENOENT, and discards the error. If the test ever regresses (e.g. publicDir path changes, prerender output layout changes), debugging will be painful because the console shows only a generic 500. Log the error (at leastconsole.error(err)) and/or distinguishENOENT→ 404 from other errors → 500.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/issues/2790.spec.ts` around lines 53 - 66, The request handler's catch block in the createServer callback swallows errors and always returns 500; update the catch to log the caught error (e.g., console.error(err)) and distinguish not-found errors by checking err.code === 'ENOENT' (return 404) while keeping other errors as 500; apply this change around the resolvePublicAsset/readFile usage in the createServer async handler so missing public assets yield 404 and other failures are logged and return 500.
38-49: Describe block labeled#2790but reuses the3988fixture.The
rootDirpoints tofixtures/issues/3988while the suite name is#2790. This is intentional (reusing the fixture with astrategy: 'prefix'override), but it makes grepping for the fixture/spec relationship confusing, and a future change to the 3988 fixture can silently affect the 2790 regression. Consider either (a) adding a short comment here noting the shared fixture, or (b) extracting the shared page/locales into a common fixture directory and giving each issue its ownnuxt.config.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/issues/2790.spec.ts` around lines 38 - 49, The describe block named "#2790" is reusing the fixture at fixtures/issues/3988 via the setup call (rootDir: fileURLToPath(new URL(`../fixtures/issues/3988`, import.meta.url))) which is confusing and brittle; add a short inline comment immediately above the setup(...) call (or above the describe) stating that this spec intentionally reuses the 3988 fixture and why (e.g., to override nuxtConfig.i18n.strategy to 'prefix'), or alternatively extract the shared pages/locales into a common fixture and point rootDir to a new per-issue fixture with its own nuxt.config.ts so changes to 3988 won’t silently affect `#2790` — reference the describe('#2790'), setup(...), and rootDir lines when making the change.
16-36: Fallback handler returns200 200.htmlfor missing assets — may mask real 404s.
resolvePublicAssetfalls through to200.htmlfor any unresolved path, and the handler then responds withstatusCode = 200. For a missing JS chunk or locale file this would return an HTML shell with a 200 status, which the browser will try to parse as JS and can manifest as confusing hydration/console errors rather than a clear 404. Since200.htmlis really only a SPA fallback for navigations, consider restricting the fallback to requests that accept HTML (or paths without a file extension), and returning 404 otherwise.🔧 Suggested tightening
- return join(publicDir, '200.html') + // Only use the SPA shell as fallback for navigations, not for asset requests. + if (!extname(pathname)) { + return join(publicDir, '200.html') + } + return null…and have the request handler respond with 404 when
nullis returned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/issues/2790.spec.ts` around lines 16 - 36, resolvePublicAsset currently falls back to returning 200.html for any missing path; change it so missing paths with a file extension return null (indicating a 404) and only perform the SPA fallback to 200.html for navigator-style requests (no extension) or the root path. Concretely, update resolvePublicAsset to inspect pathname (e.g. via path.extname) and if an extension exists and the exact/index files are missing, return null instead of join(publicDir,'200.html'); keep existing behavior for '/' and extensionless paths. Also update the request handler that calls resolvePublicAsset to treat a null return as a 404 response (set statusCode = 404) rather than serving 200.html.specs/issues/3988.spec.ts (2)
29-33: Hydration-warning assertion may miss Vue's actual warning channel.Vue emits hydration mismatch warnings via
console.warn, but in Playwright the message type forconsole.warnis reported as'warning'— so this is correct. However, some hydration errors surface as'error'type (e.g.[Vue warn]: Hydration ... mismatch) depending on Vue version/dev-vs-prod. Consider broadening to['warning', 'error'].includes(log.type)to avoid false negatives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/issues/3988.spec.ts` around lines 29 - 33, The current assertion that scans consoleLogs for hydration warnings only checks log.type === 'warning' and can miss Vue hydration messages emitted as 'error'; update the expect block that inspects consoleLogs (the variable used in the failing assertion) to treat both 'warning' and 'error' as candidates (e.g., replace the single-type check with an inclusion check against ['warning','error'] when testing log.type) so hydration mismatch messages reported as errors are also caught.
14-15: Brittle substring check for redirect injection.Asserting the raw text
window.location.replace(from the prerendered/response tightly couples the test to the current inline-script implementation increateStaticRootLocaleRedirectScript. If the script is refactored (e.g. renamed helper, split into a module, or minified), this assertion breaks without any behavioral regression. Consider asserting a more stable marker (e.g. a data-attribute on the injected<script>, or simply that the subsequent navigation lands on/en, which the rest of the test already covers).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/issues/3988.spec.ts` around lines 14 - 15, The test's brittle check for the literal string "window.location.replace(" should be replaced with a stable assertion: either assert that the injected script element contains a stable marker (e.g. a data attribute added by createStaticRootLocaleRedirectScript like data-root-locale-redirect) or remove the substring assertion and instead verify the behavioral outcome (that navigation ends up on /en). Update the spec in specs/issues/3988.spec.ts to look for the script element with that data attribute or assert the final navigation target rather than asserting the inline helper implementation text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/server/plugin.ts`:
- Around line 75-78: The bootstrap config currently assigns
detection.fallbackLocale directly (detection.fallbackLocale || '') which can be
an array or object and later breaks findSupportedLocale(String(...)); fix by
normalizing detection.fallbackLocale to a single locale string before
serializing (e.g., if detection.fallbackLocale is an array pick the first string
item, if it's an object pick the first value) so config.fallbackLocale is always
a string, or alternatively update the inline bootstrap logic that calls
findSupportedLocale to accept and iterate arrays/objects; update references in
this file to detection.fallbackLocale and config.fallbackLocale and ensure
findSupportedLocale is fed a normalized string.
---
Nitpick comments:
In `@specs/fixtures/issues/3988/package.json`:
- Around line 9-12: The devDependencies in package.json currently use floating
tags ("nuxt" and "@nuxtjs/i18n" set to "latest") which makes the fixture
non-deterministic; change those two entries in package.json (the "nuxt" and
"@nuxtjs/i18n" keys under devDependencies) to explicit, pinned semantic versions
(e.g., "x.y.z") known-good for the fixture, then regenerate the lockfile
(npm/yarn/pnpm install) so CI uses reproducible installs; if other fixtures
intentionally use "latest", either align them or add a comment in this fixture
explaining why it must be pinned.
In `@specs/issues/2790.spec.ts`:
- Around line 53-66: The request handler's catch block in the createServer
callback swallows errors and always returns 500; update the catch to log the
caught error (e.g., console.error(err)) and distinguish not-found errors by
checking err.code === 'ENOENT' (return 404) while keeping other errors as 500;
apply this change around the resolvePublicAsset/readFile usage in the
createServer async handler so missing public assets yield 404 and other failures
are logged and return 500.
- Around line 38-49: The describe block named "#2790" is reusing the fixture at
fixtures/issues/3988 via the setup call (rootDir: fileURLToPath(new
URL(`../fixtures/issues/3988`, import.meta.url))) which is confusing and
brittle; add a short inline comment immediately above the setup(...) call (or
above the describe) stating that this spec intentionally reuses the 3988 fixture
and why (e.g., to override nuxtConfig.i18n.strategy to 'prefix'), or
alternatively extract the shared pages/locales into a common fixture and point
rootDir to a new per-issue fixture with its own nuxt.config.ts so changes to
3988 won’t silently affect `#2790` — reference the describe('#2790'), setup(...),
and rootDir lines when making the change.
- Around line 16-36: resolvePublicAsset currently falls back to returning
200.html for any missing path; change it so missing paths with a file extension
return null (indicating a 404) and only perform the SPA fallback to 200.html for
navigator-style requests (no extension) or the root path. Concretely, update
resolvePublicAsset to inspect pathname (e.g. via path.extname) and if an
extension exists and the exact/index files are missing, return null instead of
join(publicDir,'200.html'); keep existing behavior for '/' and extensionless
paths. Also update the request handler that calls resolvePublicAsset to treat a
null return as a 404 response (set statusCode = 404) rather than serving
200.html.
In `@specs/issues/3988.spec.ts`:
- Around line 29-33: The current assertion that scans consoleLogs for hydration
warnings only checks log.type === 'warning' and can miss Vue hydration messages
emitted as 'error'; update the expect block that inspects consoleLogs (the
variable used in the failing assertion) to treat both 'warning' and 'error' as
candidates (e.g., replace the single-type check with an inclusion check against
['warning','error'] when testing log.type) so hydration mismatch messages
reported as errors are also caught.
- Around line 14-15: The test's brittle check for the literal string
"window.location.replace(" should be replaced with a stable assertion: either
assert that the injected script element contains a stable marker (e.g. a data
attribute added by createStaticRootLocaleRedirectScript like
data-root-locale-redirect) or remove the substring assertion and instead verify
the behavioral outcome (that navigation ends up on /en). Update the spec in
specs/issues/3988.spec.ts to look for the script element with that data
attribute or assert the final navigation target rather than asserting the inline
helper implementation text.
In `@src/runtime/plugins/ssg-detect.ts`:
- Around line 18-24: The explicit ctx.setCookieLocale(locale) after
loadAndSetLocale is redundant because loadAndSetLocale (via ctx.setLocaleSuspend
-> __resolvePendingLocalePromise) already writes the cookie; either remove the
ctx.setCookieLocale(locale) call in the nuxt.hook block or, if you want the
defensive write retained, add a one-line comment above it explaining that
loadAndSetLocale/suspend path already sets the cookie but this call is kept as a
fallback. Locate the call in the nuxt.hook where detectLocale, loadAndSetLocale,
and navigate are used and apply one of the two changes to keep intent clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0f11d7b6-217e-493c-93c2-fcff25d05ffd
📒 Files selected for processing (12)
specs/fixtures/issues/3988/app.vuespecs/fixtures/issues/3988/i18n/locales/de.jsonspecs/fixtures/issues/3988/i18n/locales/en.jsonspecs/fixtures/issues/3988/nuxt.config.tsspecs/fixtures/issues/3988/package.jsonspecs/fixtures/issues/3988/pages/index.vuespecs/issues/2790.spec.tsspecs/issues/3988.spec.tssrc/runtime/plugins/route-locale-detect.tssrc/runtime/plugins/ssg-detect.tssrc/runtime/server/plugin.tssrc/runtime/utils.ts
| fallbackLocale: detection.fallbackLocale || '', | ||
| strategy: __I18N_STRATEGY__, | ||
| useCookie: detection.useCookie, | ||
| } |
There was a problem hiding this comment.
fallbackLocale may be an array; the bootstrap script only handles the string case.
In Nuxt i18n, the browser-detection fallbackLocale can be a string, an array of strings, or an object map. Here you coerce with detection.fallbackLocale || '' and later call findSupportedLocale(config.fallbackLocale), whose internal String(tag) on an array yields e.g. 'en,de' — which never matches any locale. For array/object fallbacks the bootstrap silently falls through to defaultLocale.
🛠️ Proposed fix — normalize to first-string form before serializing
- const config = {
- cookieKey: detection.cookieKey,
- defaultLocale,
- fallbackLocale: detection.fallbackLocale || '',
- strategy: __I18N_STRATEGY__,
- useCookie: detection.useCookie,
- }
+ const normalizedFallback =
+ typeof detection.fallbackLocale === 'string'
+ ? detection.fallbackLocale
+ : Array.isArray(detection.fallbackLocale)
+ ? (detection.fallbackLocale[0] || '')
+ : ''
+ const config = {
+ cookieKey: detection.cookieKey,
+ defaultLocale,
+ fallbackLocale: normalizedFallback,
+ strategy: __I18N_STRATEGY__,
+ useCookie: detection.useCookie,
+ }Or, pass an array through and iterate it inside the inline script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/server/plugin.ts` around lines 75 - 78, The bootstrap config
currently assigns detection.fallbackLocale directly (detection.fallbackLocale ||
'') which can be an array or object and later breaks
findSupportedLocale(String(...)); fix by normalizing detection.fallbackLocale to
a single locale string before serializing (e.g., if detection.fallbackLocale is
an array pick the first string item, if it's an object pick the first value) so
config.fallbackLocale is always a string, or alternatively update the inline
bootstrap logic that calls findSupportedLocale to accept and iterate
arrays/objects; update references in this file to detection.fallbackLocale and
config.fallbackLocale and ensure findSupportedLocale is fed a normalized string.
There was a problem hiding this comment.
Applied the test hardening suggestions that improve the PR without changing runtime behavior:
- removed the brittle raw
window.location.replace(assertion - broadened hydration log checks to cover both
warninganderror - tightened the custom static server in
#2790so missing asset requests return404instead of falling back to200.html - added error logging in that test server
- added a short note that
#2790intentionally reuses the#3988fixture - added a clarifying comment on the explicit
ctx.setCookieLocale(locale)call
I did not take the fallbackLocale suggestion. In this codepath we are using detectBrowserLanguage.fallbackLocale, which is typed as Locale | null, not the broader Vue i18n fallback-locale union. The proposed array/object normalization would add complexity for a case this API does not expose here.
I also left the fixture dependency versions as-is because this repo consistently uses latest across many fixtures, so pinning only this one would be inconsistent.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the type of fallbackLocale in the detection/i18n detection utilities
rg -n "fallbackLocale" src/runtime/shared/utils.ts src/runtime/shared/detection.ts src/module/utils.ts --type=ts -C3 2>/dev/null || \
rg -n "fallbackLocale" --type=ts -C2
Length of output: 14504
@Mat4m0, thanks for the detailed explanation and for verifying the type. You're correct — src/types.ts confirms that detectBrowserLanguage.fallbackLocale is typed as Locale | null, so it's always a string or null in this code path. My suggestion conflated it with Vue i18n's broader FallbackLocale union (which can be string, array, or object map), which is a separate concern. The detection.fallbackLocale || '' coercion is perfectly safe here. I'll record this distinction for future reviews.
✏️ Learnings added
Learnt from: Mat4m0
Repo: nuxt-modules/i18n PR: 3964
File: src/runtime/server/plugin.ts:75-78
Timestamp: 2026-04-23T11:22:23.684Z
Learning: In `src/runtime/server/plugin.ts` (nuxt-modules/i18n), `detection.fallbackLocale` used inside `createStaticRootLocaleRedirectScript` comes from `detectBrowserLanguage.fallbackLocale`, which is typed as `Locale | null` in `src/types.ts` — i.e., always a string or null. It is NOT the broader Vue i18n `FallbackLocale` union (string | string[] | object). Array/object normalization is therefore unnecessary in this context.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
🔗 Linked issue
📚 Description
This fixes a static-generation bug where browser language detection could switch locale and redirect while the prerendered page was still hydrating.
At a high level, the problem looked like this:
prefix_except_default/is prerendered with the default locale HTMLprefix/is not prerendered as a real page200.html) for/That early client-side locale flip also explains the related reports where:
localePath/NuxtLinkLocalelinks stayed in the prerendered localev-text,v-html, and translated attributes such asplaceholderstayed stale until refreshWhat changed
/index.htmlfor strategies where/is a real prerendered page200.htmlforstrategy: 'prefix'Simple examples
Before
defaultLocale: 'de', browser localeen,strategy: 'prefix_except_default'/renders German HTMLenduring hydration/enAfter
/stays German through hydration/enbefore Vue mountsFor
strategy: 'prefix', the same bootstrap logic now runs from the static fallback shell (200.html), which is the entry static hosts use for/.Issue assessment
Test coverage
Added / expanded coverage for:
prefix_except_defaultroot redirect without hydration mismatchprefixfallback-shell redirect without hydration mismatchlocalePathandNuxtLinkLocalev-text,v-html, andplaceholderbindings after redirectTargeted validation run:
pnpm test:typespnpm exec eslint --no-warn-ignored src/runtime/plugins/route-locale-detect.ts src/runtime/plugins/ssg-detect.ts src/runtime/server/plugin.ts src/runtime/utils.tspnpm exec vitest run specs/issues/3988.spec.ts specs/issues/2790.spec.ts specs/browser_language_detection/prefix_except_default.spec.ts specs/browser_language_detection/prefix_and_default.spec.ts specs/issues/3407.spec.ts specs/ssg/no_prefix.spec.ts specs/ssg/different-domains.spec.ts specs/routing_strategies/prefix.spec.tsSummary by CodeRabbit
New Features
Bug Fixes
Tests