fix: locale detection should respect runtime-configured domains (#2931) #3697
Conversation
WalkthroughA new test was introduced to verify locale detection using a domain defined in runtime configuration for Korean. The i18n configuration was updated to include Korean translations. The locale detection logic was enhanced to utilize runtime-configured domain locales, updating the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant App as Application
participant Detection as Locale Detection
participant Config as RuntimeConfig
Test->>App: Send request with host "kr.staging.nuxt-app.localhost"
App->>Detection: Call getHostLocale(event, path, domainLocales)
Detection->>Config: Retrieve domainLocales from runtimeConfig
Detection->>Detection: Map locales with domain from domainLocales
Detection->>App: Return detected locale ("kr")
App->>Test: Respond with Korean welcome text
Possibly related PRs
Poem
✨ 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 (
|
![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 (4)
src/runtime/shared/detection.ts (2)
8-9: Avoid importing from a siblingshareddirectory
useRuntimeI18nis already in the samesharedfolder as this file. Importing via../shared/utilsworks, but it weakens the folder-cohesion signal and is a brittle relative path if the file ever moves. Prefer a root-alias import (e.g.#i18n-shared/utils) like the other kit imports to keep paths stable.
54-61:useDetectorsnow depends on runtime config – cache or inject
useRuntimeI18n()is executed every timeuseDetectorsis called.
Given thatuseDetectorsitself is typically called for each navigation, consider:
- Reading
runtimeI18nonce at module scope, or- Accepting it as an argument that the caller passes only once.
Not critical, but will cut a bit of overhead on the client.
specs/different_domains/different_domains.spec.ts (2)
14-18: Runtime-only domains: assert they override build-time onesNice addition! 👍
Consider an extra assertion ensuringkr.nuxt-app.localhostdoes not match once runtime overrides are in place. This guarantees the override is exclusive and keeps future refactors from silently re-introducing the build-time domain.await expect( undiciRequest('/', { headers: { Host: 'kr.nuxt-app.localhost' } }) ).resolves.toHaveProperty('statusCode', 404) // or whatever the expected fallback is
154-162: Consolidate duplicated host-detection testsThe new test is almost identical to the previous host-loop.
Reduce duplication and improve discoverability:- test('(#2931) detect using runtimeConfig domain', async () => { … }) + test.each([ + ['kr.staging.nuxt-app.localhost', '환영하다'], + ])('(#2931) detect %s using runtimeConfig domain', async (host, expected) => { … })This keeps all host-locale assertions in the same table.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
specs/different_domains/different_domains.spec.ts(1 hunks)specs/fixtures/different_domains/i18n/i18n.config.ts(1 hunks)src/runtime/shared/detection.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
specs/different_domains/different_domains.spec.ts (2)
specs/utils/server.ts (1)
undiciRequest(109-111)specs/helper.ts (1)
getDom(47-49)
src/runtime/shared/detection.ts (4)
src/types.ts (1)
I18nPublicRuntimeConfig(356-370)src/runtime/shared/domain.ts (1)
matchDomainLocale(7-23)src/runtime/kit/routing.ts (1)
getLocaleFromRoutePath(32-32)src/runtime/shared/utils.ts (1)
useRuntimeI18n(4-6)
🔇 Additional comments (1)
specs/fixtures/different_domains/i18n/i18n.config.ts (1)
43-56: Translation quality – confirm Korean stringsThe Korean copy uses literal dictionary forms (
환영하다,동적).
Native UI wording is usually polite style (환영합니다,동적 페이지, etc.).
If the strings ship to production, have a native speaker review them to avoid an awkward tone.
| const getHostLocale = ( | ||
| event: H3Event | undefined, | ||
| path: string, | ||
| domainLocales: I18nPublicRuntimeConfig['domainLocales'] | ||
| ) => { | ||
| const host = import.meta.client | ||
| ? new URL(window.location.href).host | ||
| : getRequestURL(event!, { xForwardedHost: true }).host | ||
| return matchDomainLocale(normalizedLocales, host, getLocaleFromRoutePath(path)) | ||
|
|
||
| const locales = normalizedLocales.map(l => ({ | ||
| ...l, | ||
| domain: domainLocales[l.code]?.domain ?? l.domain | ||
| })) | ||
| return matchDomainLocale(locales, host, getLocaleFromRoutePath(path)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Per-request cloning of normalizedLocales is wasteful
getHostLocale maps over normalizedLocales on every invocation to attach runtime domains.
For heavily trafficked apps this runs once per request, causing unnecessary allocations.
- const locales = normalizedLocales.map(l => ({
- ...l,
- domain: domainLocales[l.code]?.domain ?? l.domain
- }))
+ // cache by reference – memoised on first call
+ const locales =
+ cachedRuntimeDomains ??
+ (cachedRuntimeDomains = normalizedLocales.map(l => ({
+ ...l,
+ domain: domainLocales[l.code]?.domain ?? l.domain
+ })))You can stick a simple let cachedRuntimeDomains: LocaleObject[] | undefined outside the function to memoise until domainLocales changes (which is only on hot-reload).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/runtime/shared/detection.ts around lines 33 to 47, the function
getHostLocale creates a new mapped array from normalizedLocales on every call,
causing unnecessary allocations per request. To fix this, declare a variable
cachedRuntimeDomains outside the function to store the mapped locales with
runtime domains, and update this cache only when domainLocales changes (such as
on hot-reload). Inside getHostLocale, return the cached array instead of
remapping normalizedLocales each time.
🔗 Linked issue
Ports #3693 to the next branch.
📚 Description
This change checks for runtime-configured domain overrides when determining the locale from the host.
Summary by CodeRabbit