Skip to content

Conversation

@BobbieGoede
Copy link
Member

@BobbieGoede BobbieGoede commented May 21, 2025

🔗 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

  • New Features
    • Added automatic client-side redirection from the root path to the appropriate locale-prefixed page during static site generation when using the 'prefix' strategy.
  • Bug Fixes
    • Improved navigation logic to prevent unwanted redirects from the root path in specific static site scenarios.
  • Refactor
    • Simplified the logic for including the root index page in localized page lists.
    • Updated prerendering behavior to rely on default handling for the root path.

@BobbieGoede BobbieGoede self-assigned this May 21, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 21, 2025

Walkthrough

The 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

File(s) Change Summary
src/pages.ts Simplified the condition for prepending the root index page to the localized pages list; now always adds for 'prefix' mode.
src/prepare/strategy.ts Removed logic that explicitly ignores the root path / in Nitro's prerender ignore list; added clarifying comment.
src/runtime/server/plugin.ts Added runtime config access and a client-side script for locale prefix redirection on SSG root; script injected as needed.
src/runtime/utils.ts Added early return in navigate to skip logic for prefix strategy on SSG root path.

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
Loading

Possibly related PRs

Poem

🐇
At the root of the site, a new hop appears,
With scripts that redirect, so no user veers.
No more ignoring slashes, the logic is neat,
Prefixes in place, locale journeys complete.
With every new build, the routing feels right—
A bunny’s delight in the i18n night!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55edbe8 and acaadcf.

📒 Files selected for processing (4)
  • src/pages.ts (1 hunks)
  • src/prepare/strategy.ts (1 hunks)
  • src/runtime/server/plugin.ts (3 hunks)
  • src/runtime/utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/runtime/utils.ts
  • src/prepare/strategy.ts
  • src/runtime/server/plugin.ts
  • src/pages.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: test (lts/*, ubuntu-latest)
  • GitHub Check: test (lts/*, windows-latest)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@pkg-pr-new
Copy link

pkg-pr-new bot commented May 21, 2025

npm i https://pkg.pr.new/@nuxtjs/i18n@3629

commit: acaadcf

@pkg-pr-new
Copy link

pkg-pr-new bot commented May 21, 2025

npm i https://pkg.pr.new/@nuxtjs/i18n@3629

commit: 55edbe8

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: Unused isString import

isString is 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-true if (…) wrapper

Inside 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

📥 Commits

Reviewing files that changed from the base of the PR and between af812f8 and 55edbe8.

📒 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 configuration

Removing 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.

Comment on lines 86 to 93
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)
}
Copy link
Contributor

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.

Suggested change
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.

Comment on lines +265 to +309
if (__I18N_STRATEGY__ === 'prefix' && __IS_SSG__ && routePath === '/') {
return
}
Copy link
Contributor

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 force to 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.

Suggested change
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.

Comment on lines +31 to +55
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>`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

__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.

Suggested change
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.

Comment on lines +55 to +63
if (ssgPrefixRedirectionScript && event.path === '/') {
htmlContext.body.length = 0
htmlContext.bodyAppend.unshift(ssgPrefixRedirectionScript)
return
}

Copy link
Contributor

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.ts

Length 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.ts

Length 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.

Suggested change
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.

@BobbieGoede BobbieGoede force-pushed the fix/prefix-strategy-ssg-index-route-with-redirect branch from 55edbe8 to acaadcf Compare May 28, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants