fix(domain): resolve domain config the same way at build time and runtime - #4090
Conversation
WalkthroughThe changes gate compact route compilation for domain-based and no-prefix routing, normalize domain data during no-prefix validation, and detect duplicate hosts across locale domain configurations. Runtime domain overrides now update related domain metadata. Tests cover host matching, default-locale resolution, duplicate-domain validation, multi-domain locales, and unprefixed route generation. Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/shared/domain.ts`:
- Around line 100-102: Update the domain normalization flow around the existing
domain comparison so a matching primary domain does not return early when
properties.domains contains other hosts. For a runtime override, always collapse
domains and defaultForDomains to the single override domain, while preserving
the current no-domain behavior.
🪄 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 Plus
Run ID: 22b7ccd1-d351-4ae4-bb53-22f8c3974df9
⛔ Files ignored due to path filters (1)
test/pages/__snapshots__/localize_routes.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/bundler.tssrc/routing.tssrc/runtime/shared/domain.tstest/domain.test.tstest/pages/localize_routes.test.ts
| if (!domain || domain === properties.domain) { | ||
| return locale | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not skip normalization when the primary domain already matches.
If domain === properties.domain but properties.domains still contains other hosts, this return leaves the locale matching those old hosts. A runtime override must still collapse domains (and defaultForDomains) to the single override domain.
Proposed fix
- if (!domain || domain === properties.domain) {
+ if (!domain) {
return locale
}📝 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 (!domain || domain === properties.domain) { | |
| return locale | |
| } | |
| if (!domain) { | |
| return locale | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/shared/domain.ts` around lines 100 - 102, Update the domain
normalization flow around the existing domain comparison so a matching primary
domain does not return early when properties.domains contains other hosts. For a
runtime override, always collapse domains and defaultForDomains to the single
override domain, while preserving the current no-domain behavior.
…time `normalizeDomainLocale` now always sets `domains` and `defaultForDomains`, so domain logic can read them without a fallback, and the result is typed as `NormalizedLocaleObject`. `domainDefault` is fully resolved into `defaultForDomains`, so the type takes it away - reading it past normalization is how the build and the runtime came to disagree in #4090. The test helpers resolve their locales through the normalizer the same way `resolveContext` does, rather than passing a shape the build never produces.
…time `normalizeDomainLocale` now always sets `domains` and `defaultForDomains`, so domain logic can read them without a fallback, and the result is typed as `NormalizedLocaleObject`. `domainDefault` is fully resolved into `defaultForDomains`, so the type takes it away - reading it past normalization is how the build and the runtime came to disagree in #4090. The test helpers resolve their locales through the normalizer the same way `resolveContext` does, rather than passing a shape the build never produces.
…time `normalizeDomainLocale` now always sets `domains` and `defaultForDomains`, so domain logic can read them without a fallback, and the result is typed as `NormalizedLocaleObject`. `domainDefault` is fully resolved into `defaultForDomains`, so the type takes it away - reading it past normalization is how the build and the runtime came to disagree in #4090. The test helpers resolve their locales through the normalizer the same way `resolveContext` does, rather than passing a shape the build never produces.
1 participant
Four places where a domain fact was derived twice, once at build time and once at runtime, and the two derivations disagreed.
shouldLocalizeRoutesread only the singulardomain, sono_prefixaccepted two locales sharing a domain when they were configured throughdomainsand refused the same setup written asdomain.getDomainDefaultLocalesread the rawdomainDefaultwhile the runtime resolves domain defaults fromdefaultForDomainsalone, so a locale carryingdomainDefaultwithout any domain was given unprefixed___defaultroutes the runtime could never select it for. Both now resolve throughnormalizeDomainLocale, which is where that mapping already lives.withRuntimeDomainreplaced a locale'sdomainand leftdomainsas it was built, so a locale moved withNUXT_PUBLIC_I18N_DOMAIN_LOCALES_{code}_DOMAINmatched its new host and the one it was configured with, which host membership now decides navigation and relocation from. It replaces both, and carriesdefaultForDomainsover so an overridden locale stays the default for the domain it moved to.__I18N_COMPACT_ROUTES__was set fromexperimental.compactRoutesalone while route generation also requires no domain routing and a prefixing strategy, so an incompatible config left the runtime taking compact branches against a route table that had none, including dropping alocaleroute param. The flag now mirrors the generation gate thatprepare/options.tsalready warns about.The updated snapshot loses only routes, the
___defaultvariants that were generated for a locale with no domain to be the default for.Summary by CodeRabbit
Bug Fixes
Tests