-
-
Notifications
You must be signed in to change notification settings - Fork 514
fix: index route for ssg with strategy prefix #3629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: index route for ssg with strategy prefix #3629
Conversation
WalkthroughThe changes adjust logic related to locale-prefixed routing and root path handling in a Nuxt i18n setup. The updates simplify root index page inclusion, remove explicit root path exclusion from prerendering, add client-side redirection for SSG root requests, and prevent navigation logic from running on the SSG root in prefix mode. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server (Nitro Plugin)
participant RuntimeConfig
Browser->>Server (Nitro Plugin): Request '/'
Server (Nitro Plugin)->>RuntimeConfig: Get i18n config
alt SSG build & prefix strategy
Server (Nitro Plugin)-->>Browser: Injects redirection script in HTML
Browser->>Browser: Runs script, checks rootRedirect/cookie/browser lang
Browser->>Browser: Redirects to locale-prefixed path
else Normal flow
Server (Nitro Plugin)-->>Browser: Standard HTML response
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/runtime/server/plugin.ts (2)
3-4: UnusedisStringimport
isStringis used later only inside the generated script template literal; the reference is resolved at build time, so the runtime import is unnecessary. Removing it reduces bundle size a little.-import { isString } from '@intlify/shared'
31-53: Redirection script could skip the always-trueif (…)wrapperInside the generated script you build:
if(true){ window.location.replace('/foo') }The condition is constant and therefore redundant.
Trimming it will shave a few bytes and prevent ESLint warnings when users view the source.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
src/pages.ts(1 hunks)src/prepare/strategy.ts(1 hunks)src/runtime/server/plugin.ts(2 hunks)src/runtime/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/runtime/server/plugin.ts (5)
src/runtime/shared/vue-i18n.ts (1)
setupVueI18nOptions(11-25)src/types.ts (2)
I18nPublicRuntimeConfig(361-430)RootRedirectOptions(56-59)src/runtime/shared/locales.ts (1)
createLocaleConfigs(7-17)src/runtime/server/context.ts (1)
createI18nContext(27-37)internals.d.ts (2)
localeCodes(9-9)localeCodes(19-19)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (lts/*, windows-latest)
- GitHub Check: test (lts/*, ubuntu-latest)
🔇 Additional comments (1)
src/prepare/strategy.ts (1)
6-13: 👍 Simpler prerender configurationRemoving the explicit
/ignore aligns Nitro prerendering with the new client-side redirect flow; the comment also clarifies intent. I don’t see any regressions introduced here.
| const indexPage = pages.find(x => x.path === '/') | ||
| if (!nuxt.options._generate && options.strategy === 'prefix' && indexPage != null) { | ||
| if (options.strategy === 'prefix' && indexPage != null) { | ||
| localizedPages.unshift(indexPage as NarrowedNuxtPage) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Possible duplicate root entry when localizeRoutes keeps /
localizeRoutes() used above may (depending on its internal logic + includeUnprefixedFallback) already keep the original / page.
Blindly unshift-ing indexPage can therefore create two identical entries in the pages array, which will later make vue-router complain about duplicate route names / paths.
-if (options.strategy === 'prefix' && indexPage != null) {
- localizedPages.unshift(indexPage as NarrowedNuxtPage)
-}
+if (options.strategy === 'prefix' && indexPage != null && !localizedPages.includes(indexPage as NarrowedNuxtPage)) {
+ localizedPages.unshift(indexPage as NarrowedNuxtPage)
+}A cheap identity check (includes) avoids the duplication cost-free.
(If localizeRoutes always drops /, feel free to ignore – but documenting that invariant near the call-site would still help future readers.)
To confirm whether localizeRoutes() can already include the / entry (and thus if we need a guard), let’s inspect its implementation and its use of includeUnprefixedFallback:
🏁 Script executed:
#!/bin/bash
# Show where localizeRoutes is called in src/pages.ts
rg -n "localizeRoutes" -A 5 -B 5 src/pages.ts
# Find the implementation signature of localizeRoutes
rg -n "function localizeRoutes" -A 10 -B 5 --glob "*.ts"
# Check how includeUnprefixedFallback is used/defaulted
rg -n "includeUnprefixedFallback" -A 5 -B 5 --glob "*.ts"Length of output: 12923
Guard against duplicate / route when using prefix strategy
Wrap the manual unshift in an includes check so you don’t add / twice if localizeRoutes already emitted it (e.g. when includeUnprefixedFallback: true).
• File: src/pages.ts (inside your components:dirs hook, around lines 86–89)
--- a/src/pages.ts
+++ b/src/pages.ts
@@ -86,7 +86,7 @@ setupPages(...) {
const indexPage = pages.find(x => x.path === '/')
- if (options.strategy === 'prefix' && indexPage != null) {
+ if (options.strategy === 'prefix' && indexPage && !localizedPages.includes(indexPage as NarrowedNuxtPage)) {
localizedPages.unshift(indexPage as NarrowedNuxtPage)
}Consider adding a brief comment here noting that includeUnprefixedFallback controls whether localizeRoutes() already keeps the default / route, so this guard remains future-proof.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const indexPage = pages.find(x => x.path === '/') | |
| if (!nuxt.options._generate && options.strategy === 'prefix' && indexPage != null) { | |
| if (options.strategy === 'prefix' && indexPage != null) { | |
| localizedPages.unshift(indexPage as NarrowedNuxtPage) | |
| } | |
| const indexPage = pages.find(x => x.path === '/') | |
| if ( | |
| options.strategy === 'prefix' && | |
| indexPage && | |
| !localizedPages.includes(indexPage as NarrowedNuxtPage) | |
| ) { | |
| localizedPages.unshift(indexPage as NarrowedNuxtPage) | |
| } |
🤖 Prompt for AI Agents
In src/pages.ts around lines 86 to 89, prevent adding a duplicate root '/' route
by checking if localizedPages already includes indexPage before unshifting it.
Wrap the unshift call in an if statement that uses includes to verify indexPage
is not already present. Additionally, add a comment explaining that this guard
is necessary because localizeRoutes may keep the '/' route depending on
includeUnprefixedFallback, ensuring future maintainers understand the reason for
this check.
| if (__I18N_STRATEGY__ === 'prefix' && __IS_SSG__ && routePath === '/') { | ||
| return | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Early-return disables forced navigation to /
The new guard short-circuits all calls to navigate() on / during SSG prefix builds – even when force === true.
That means:
- Manual
navigate('/', /* force */ true)from the app will now silently do nothing. - Future refactors may expect
forceto bypass all guards.
Consider respecting force (and avoiding repetition of the compile-time checks):
-if (__I18N_STRATEGY__ === 'prefix' && __IS_SSG__ && routePath === '/') {
- return
-}
+if (
+ routePath === '/' &&
+ __I18N_STRATEGY__ === 'prefix' &&
+ __IS_SSG__ &&
+ !force // let the caller override
+) {
+ return
+}This keeps the optimisation while preserving the public contract.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (__I18N_STRATEGY__ === 'prefix' && __IS_SSG__ && routePath === '/') { | |
| return | |
| } | |
| if ( | |
| routePath === '/' && | |
| __I18N_STRATEGY__ === 'prefix' && | |
| __IS_SSG__ && | |
| !force // let the caller override | |
| ) { | |
| return | |
| } |
🤖 Prompt for AI Agents
In src/runtime/utils.ts around lines 265 to 267, the early return disables
forced navigation to '/' by ignoring the 'force' parameter during SSG prefix
builds. Modify the condition to check if 'force' is not true before returning
early, so that when 'force' is true, the navigation proceeds as expected. This
preserves the optimization while respecting the public contract of forced
navigation.
| function getPrefixRedirectionScript() { | ||
| if (runtimeI18n.rootRedirect) { | ||
| return ` | ||
| if(${JSON.stringify(!!runtimeI18n.rootRedirect)}) { | ||
| window.location.replace('${isString(runtimeI18n.rootRedirect) ? runtimeI18n.rootRedirect : (runtimeI18n.rootRedirect as RootRedirectOptions).path}'); | ||
| }` | ||
| } | ||
| return ` | ||
| function getCookieValue(cookieName) { | ||
| if (${JSON.stringify(!useCookie)}) return ''; | ||
| for (const cookie of document.cookie.split('; ')) { | ||
| const [name, value] = cookie.split('='); | ||
| if (name === ${genString(cookieKey || __DEFAULT_COOKIE_KEY__)}) return value; | ||
| } | ||
| } | ||
| const locale = getCookieValue() || navigator.language; | ||
| const locales = ${JSON.stringify(localeCodes)}; | ||
| const defaultLocale = ${genString(options.locale || localeCodes[0])}; | ||
| window.location.replace('/' + (locales.includes(locale) ? locale : defaultLocale));` | ||
| } | ||
| const ssgPrefixRedirectionScript = | ||
| __IS_SSG__ && __I18N_STRATEGY__ === 'prefix' && `<script>${getPrefixRedirectionScript()}</script>` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__DEFAULT_COOKIE_KEY__ is referenced but never imported
genString(cookieKey || __DEFAULT_COOKIE_KEY__) will cause a TS/rollup error if the global constant is not defined in this file’s scope.
Unless you rely on Vite’s define globals, you need to import or inline the default value.
- const [name, value] = cookie.split('=');
- if (name === ${genString(cookieKey || __DEFAULT_COOKIE_KEY__)}) return value;
+ const [name, value] = cookie.split('=');
+ if (name === ${genString(
+ cookieKey ?? 'i18n_redirected' /* fallback to avoid undefined global */
+ )}) return value;Double-check the build to avoid ReferenceError: __DEFAULT_COOKIE_KEY__ is not defined.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getPrefixRedirectionScript() { | |
| if (runtimeI18n.rootRedirect) { | |
| return ` | |
| if(${JSON.stringify(!!runtimeI18n.rootRedirect)}) { | |
| window.location.replace('${isString(runtimeI18n.rootRedirect) ? runtimeI18n.rootRedirect : (runtimeI18n.rootRedirect as RootRedirectOptions).path}'); | |
| }` | |
| } | |
| return ` | |
| function getCookieValue(cookieName) { | |
| if (${JSON.stringify(!useCookie)}) return ''; | |
| for (const cookie of document.cookie.split('; ')) { | |
| const [name, value] = cookie.split('='); | |
| if (name === ${genString(cookieKey || __DEFAULT_COOKIE_KEY__)}) return value; | |
| } | |
| } | |
| const locale = getCookieValue() || navigator.language; | |
| const locales = ${JSON.stringify(localeCodes)}; | |
| const defaultLocale = ${genString(options.locale || localeCodes[0])}; | |
| window.location.replace('/' + (locales.includes(locale) ? locale : defaultLocale));` | |
| } | |
| const ssgPrefixRedirectionScript = | |
| __IS_SSG__ && __I18N_STRATEGY__ === 'prefix' && `<script>${getPrefixRedirectionScript()}</script>` | |
| function getPrefixRedirectionScript() { | |
| if (runtimeI18n.rootRedirect) { | |
| return ` | |
| if(${JSON.stringify(!!runtimeI18n.rootRedirect)}) { | |
| window.location.replace('${ | |
| isString(runtimeI18n.rootRedirect) | |
| ? runtimeI18n.rootRedirect | |
| : (runtimeI18n.rootRedirect as RootRedirectOptions).path | |
| }'); | |
| }` | |
| } | |
| return ` | |
| function getCookieValue(cookieName) { | |
| if (${JSON.stringify(!useCookie)}) return ''; | |
| for (const cookie of document.cookie.split('; ')) { | |
| const [name, value] = cookie.split('='); | |
| if (name === ${genString( | |
| cookieKey ?? 'i18n_redirected' /* fallback to avoid undefined global */ | |
| )}) return value; | |
| } | |
| } | |
| const locale = getCookieValue() || navigator.language; | |
| const locales = ${JSON.stringify(localeCodes)}; | |
| const defaultLocale = ${genString(options.locale || localeCodes[0])}; | |
| window.location.replace('/' + (locales.includes(locale) ? locale : defaultLocale));` | |
| } | |
| const ssgPrefixRedirectionScript = | |
| __IS_SSG__ && __I18N_STRATEGY__ === 'prefix' && `<script>${getPrefixRedirectionScript()}</script>` |
🤖 Prompt for AI Agents
In src/runtime/server/plugin.ts around lines 31 to 53, the constant
__DEFAULT_COOKIE_KEY__ is used without being imported or defined, which can
cause a ReferenceError during build or runtime. To fix this, either import
__DEFAULT_COOKIE_KEY__ from its module if it is exported, or define a local
constant with the default value before using it in genString(cookieKey ||
__DEFAULT_COOKIE_KEY__). Ensure the constant is properly in scope to avoid build
errors.
| if (ssgPrefixRedirectionScript && event.path === '/') { | ||
| htmlContext.body.length = 0 | ||
| htmlContext.bodyAppend.unshift(ssgPrefixRedirectionScript) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Clearing htmlContext.body removes existing SSR markup
When the root page is requested in SSG + prefix mode you:
htmlContext.body.length = 0
htmlContext.bodyAppend.unshift(script)This yields an HTML document whose <body> only contains the redirect script; all other critical tags (for example inlined CSS, noscript meta, etc.) are stripped.
A safer approach is to prepend the script instead:
-htmlContext.body.length = 0
-htmlContext.bodyAppend.unshift(ssgPrefixRedirectionScript)
+htmlContext.body.unshift(ssgPrefixRedirectionScript)This keeps existing markup intact and still ensures first-paint execution.
🏁 Script executed:
#!/bin/bash
set -e
# Locate occurrences and context for htmlContext in plugin.ts
rg -n "htmlContext" -C5 src/runtime/server/plugin.tsLength of output: 923
🏁 Script executed:
#!/bin/bash
set -e
# Output the first 200 lines of plugin.ts for context
sed -n '1,200p' src/runtime/server/plugin.tsLength of output: 4572
Prepend the redirect script instead of clearing all SSR markup
Clearing htmlContext.body here removes all existing SSR-generated tags (inlined CSS, <noscript>, meta tags, etc.), leaving only your redirection script. To preserve the original markup while ensuring the redirect runs first, inject the script at the front of the body array:
• File: src/runtime/server/plugin.ts
• Hook: render:html (around line 55)
Suggested diff:
-if (ssgPrefixRedirectionScript && event.path === '/') {
- htmlContext.body.length = 0
- htmlContext.bodyAppend.unshift(ssgPrefixRedirectionScript)
- return
-}
+if (ssgPrefixRedirectionScript && event.path === '/') {
+ htmlContext.body.unshift(ssgPrefixRedirectionScript)
+ return
+}This change preserves all existing <body> content and still guarantees the redirect script runs first.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (ssgPrefixRedirectionScript && event.path === '/') { | |
| htmlContext.body.length = 0 | |
| htmlContext.bodyAppend.unshift(ssgPrefixRedirectionScript) | |
| return | |
| } | |
| if (ssgPrefixRedirectionScript && event.path === '/') { | |
| htmlContext.body.unshift(ssgPrefixRedirectionScript) | |
| return | |
| } |
🤖 Prompt for AI Agents
In src/runtime/server/plugin.ts around lines 55 to 60, instead of clearing
htmlContext.body which removes all existing SSR markup, modify the code to
prepend the ssgPrefixRedirectionScript to htmlContext.body. This preserves all
existing body content like inlined CSS and meta tags while ensuring the redirect
script runs first. Replace the line that sets htmlContext.body.length = 0 with
an insertion that adds the script at the beginning of htmlContext.body.
55edbe8 to
acaadcf
Compare
🔗 Linked issue
📚 Description
This PR changes the index route handling when using the prefix strategy with SSG, instead of being removed from the output, it will be replaced with a basic language detection and redirection script.
This PR lacks documentation and tests, it may take a while until this is finished, stable release of v10 has priority over this.
Summary by CodeRabbit