fix(router): reset cached tViews between SSR requests for correct i18n locale switching#2295
Conversation
…orrect i18n locale switching Angular caches component consts() results on def.tView, which bakes in $localize translations from the first render. This adds a component def registry and tView reset mechanism so each SSR request re-evaluates templates with the correct locale's translations. - router: new registerI18nComponentDef / resetI18nComponentDefCache registry, called from provideI18n's app initializer and from a BEFORE_APP_SERIALIZED hook in the server render path - router: switch to @angular/localize loadTranslations/clearTranslations for the parsed translation format $localize.translate expects - router: resolveActiveLocale reads LOCALE / REQUEST fallbacks via provideAppInitializer instead of ENVIRONMENT_INITIALIZER - platform: new i18nComponentRegistryPlugin that instruments compiled SSR output to register every ɵcmp definition at load time (covers route-loaded page components that the runtime walker misses) - root: add @angular/localize 21.2.7 dev dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This PR touches multiple package scopes: Please confirm the changes are closely related. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds runtime i18n lifecycle and SSR component-def registry plumbing: a new Vite plugin instruments compiled Angular SSR modules to register component defs; the platform plugin conditionally includes it when i18n is enabled. provideI18n was refactored to use a provideAppInitializer that resolves the active locale, clears runtime translations before loading them asynchronously (preferring Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes BREAKING CHANGE
Review callers for synchronous assumptions and any external imports of I18N_CONFIG. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/platform/src/lib/i18n-component-registry-plugin.ts (1)
113-145: Minor:classHasCmpDefcontinues walking after finding a match.Once
found = true, the walker still traverses the rest of the class body. For large classes, this is wasteful.♻️ Optional: early-exit from walk when found
One option is to throw a sentinel value to break out of the recursion, or refactor
walkto support a return value that signals "stop". For most components this is negligible, so it's a nice-to-have.function classHasCmpDef(classNode: AstNode): boolean { - let found = false; - - walk(classNode, (node: any) => { - if (found) return; + const FOUND = Symbol('found'); + try { + walk(classNode, (node: any) => { // ... detection logic ... - found = true; + throw FOUND; + }); + } catch (e) { + if (e === FOUND) return true; + throw e; + } + return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/platform/src/lib/i18n-component-registry-plugin.ts` around lines 113 - 145, classHasCmpDef currently continues traversing after setting found = true which wastes time for large classes; modify the walker callback to early-exit once a match is found by signalling stop to walk: either throw a sentinel (e.g., a unique StopWalk Error) immediately after setting found = true or, if walk can be modified, have the callback return a boolean/unused value that walk treats as "stop" and implement that in walk; update the callback in classHasCmpDef (the places that set found = true for PropertyDefinition and AssignmentExpression) to trigger this stop mechanism so traversal halts as soon as ɵcmp is detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router/src/index.ts`:
- Around line 21-31: The export list removed I18N_CONFIG causing a breaking
change for consumers; update the project's release notes to include a BREAKING
CHANGE footer per CONTRIBUTING.md stating that `I18N_CONFIG` is no longer
exported from `@analogjs/router` and show the before/after usage (importing
I18N_CONFIG and inject(I18N_CONFIG) -> use `injectSwitchLocale()` instead),
referencing the affected symbol names `I18N_CONFIG` and `injectSwitchLocale()`
so users know to migrate; ensure the exact phrasing matches the provided snippet
and is included in the changelog/release notes for this release.
In `@packages/router/src/lib/i18n/provide-i18n.spec.ts`:
- Around line 10-14: The test import names use the wrong `ɵɵ` prefix; update the
imports in provide-i18n.spec.ts so they match the exported symbols from
provide-i18n.ts — replace ɵɵregisterI18nComponentDef with
ɵregisterI18nComponentDef and ɵɵresetI18nComponentDefCache with
ɵresetI18nComponentDefCache (leave getI18nComponentDefRegistrySize and
clearI18nComponentDefRegistry as-is) so the test imports exactly match the
module exports.
---
Nitpick comments:
In `@packages/platform/src/lib/i18n-component-registry-plugin.ts`:
- Around line 113-145: classHasCmpDef currently continues traversing after
setting found = true which wastes time for large classes; modify the walker
callback to early-exit once a match is found by signalling stop to walk: either
throw a sentinel (e.g., a unique StopWalk Error) immediately after setting found
= true or, if walk can be modified, have the callback return a boolean/unused
value that walk treats as "stop" and implement that in walk; update the callback
in classHasCmpDef (the places that set found = true for PropertyDefinition and
AssignmentExpression) to trigger this stop mechanism so traversal halts as soon
as ɵcmp is detected.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a3e28d5-2692-4b8b-823d-000700b8b8dd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (7)
package.jsonpackages/platform/src/lib/i18n-component-registry-plugin.tspackages/platform/src/lib/platform-plugin.tspackages/router/server/src/render.tspackages/router/src/index.tspackages/router/src/lib/i18n/provide-i18n.spec.tspackages/router/src/lib/i18n/provide-i18n.ts
…sformResult magic-string types `sourcesContent` as optional while Rolldown's TransformResult requires `string[]`. With `includeContent: true` the content is always populated, so cast to the expected type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ce2a167 to
795a809
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router/src/lib/i18n/provide-i18n.spec.ts`:
- Line 71: The empty mock implementation for console.warn triggers the
no-empty-function lint rule; update the spy setup (vi.spyOn(console,
'warn').mockImplementation(...)) to provide a non-empty return expression such
as returning undefined (e.g., mockImplementation(() => undefined) or () => void
0) so the mock is not an empty function while preserving behavior; ensure you
update the declaration for warnSpy accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 892e59e0-c10a-4dc7-9be4-9a96cf14565c
📒 Files selected for processing (2)
packages/platform/src/lib/i18n-component-registry-plugin.tspackages/router/src/lib/i18n/provide-i18n.spec.ts
PR Checklist
Fixes three issues that prevented
provideI18n()from switching locales correctly across SSR requests in a single Node process.Closes #
Affected scope
Recommended merge strategy for maintainer [optional]
What is the new behavior?
Three related problems were fixed, plus the public API was trimmed:
1. Server-context
LOCALEis no longer shadowed. PreviouslyprovideI18n()unconditionally providedLOCALEat environment level using a value computed at module-load time (detectClientLocale), which returneddefaultLocaleon the server becausetypeof window === 'undefined'. That shadowed the platform-levelLOCALEset byprovideServerContext(), soinjectLocale()always returned the default locale during SSR regardless of the URL. The provider is now registered only on the client; on the server, injection falls through to the platform-level value.2.
loadTranslationsRuntimestores parsed translations. The previous implementation set raw strings on$localize.TRANSLATIONS, which$localize.translate()cannot consume — it expects the{ text, messageParts, placeholderNames }shape produced byparseTranslation(). The helper now delegates to@angular/localize'sloadTranslations()via dynamic import, with a fallback for environments where the package is not installed.3. APP_INITIALIZER built into
provideI18n(). The oldENVIRONMENT_INITIALIZERreturned an unawaited Promise, so translations could be in flight while components began rendering. Replaced withprovideAppInitializerso bootstrap blocks on translation loading. The initializer detects locale viainject(LOCALE, { optional: true })(which now correctly reads the server-context value) with fallbacks toREQUEST.originalUrlandwindow.location.pathname.Per-request tView reset. Added a process-level registry of component defs plus a reset helper, both
ɵ-prefixed. The registry is populated at module load time by a new Vite plugin in@analogjs/platform(i18nComponentRegistryPlugin) that instruments compiled component output under SSR transforms, injecting aɵregisterI18nComponentDef(ClassName)call at module scope. The plugin usesthis.parse()from the Rollup plugin context rather than regex matching and produces correctly anchored source maps viamagic-string. The server renderer callsɵresetI18nComponentDefCache()before eachrenderApplication()so the next render re-evaluatesconsts()with the locale that its APP_INITIALIZER just loaded.Public API reduction. The i18n surface is now 4 names (
provideI18n,I18nConfig,injectSwitchLocale,loadTranslationsRuntime). Framework plumbing isɵ-prefixed following Angular's convention (ɵregisterI18nComponentDef,ɵresetI18nComponentDefCache).I18N_CONFIGandclearTranslationsRuntimeare no longer exported from the package entry — they have no external consumers.Test plan
packages/router/src/lib/i18n/provide-i18n.spec.tsupdated for the new behavior (parsed translation shape,clearTranslationsRuntimeside effects, component def registry)/fr,/de,/enagainst a runningvitedev server each return their own translations with the correct<html lang>node dist/analog/server/index.mjsexercised with the same sequence — every request handled correctly from a single process<html lang>nx format:checkpnpm buildpnpm testDoes this PR introduce a breaking change?
The runtime i18n API is still in beta.
registerI18nComponentDefandresetI18nComponentDefCachehave been renamed toɵregisterI18nComponentDef/ɵresetI18nComponentDefCacheto mark them as framework plumbing, andI18N_CONFIG/clearTranslationsRuntimeare no longer exported from@analogjs/router— all four were internal and unlikely to be called directly.Other information
Why clear
tViewbetween SSR requestsAngular caches the result of a component def's
consts()factory ondef.tViewthe first time the component renders. The factory is where$localizetagged templates are evaluated, so once atViewis created under one locale, every later render in the same process reuses those strings regardless of what$localize.TRANSLATIONScurrently holds. In a single-build multi-locale SSR setup, the first locale to render wins for the lifetime of the process.Angular's canonical i18n story is build-time
--localize, which avoids this by producing one bundle per locale — the cache-forever behavior is correct in that model. RuntimeloadTranslations()was introduced primarily for early-bootstrap test fixtures and for whole-app locale switches where a page reload is acceptable (which is whyinjectSwitchLocale()does awindow.location.hrefassignment — the reload resets the process and the tView caches along with it). The missing quadrant is "one build, shared Node process, locale per request," which is the workflowprovideI18n()invites.Nulling
def.tViewbefore each render forcesgetOrCreateComponentTView()to rebuild — runningconsts()again, picking up whichever translations are currently loaded. The reset is safe across requests in this codebase because it runs beforebootstrapApplication(), by which point the previous request's LViews have been destroyed; there are no in-flight references to the oldtViewto invalidate. Component IDs come fromdef.id, nottView, so they remain stable across rebuilds — SSR → client hydration keeps working as long as the same translations are loaded on both sides (they are; the client runs the same APP_INITIALIZER).The trade-offs worth documenting:
tView.blueprint,tView.data,directiveRegistry, andpipeRegistryfor every rendered component rather than amortizing those allocations across the process lifetime. At high QPS this is measurable GC pressure. Users serving public traffic should still prefer build-time--localizeif their deployment model allows per-locale bundles.firstCreatePassruns every request. Directive/pipe registry extraction, host binding setup, content query setup, node-level DI, and component feature callbacks (ɵɵNgOnChangesFeature,ɵɵHostDirectivesFeature) fire each render instead of once. The contract for these is idempotent population of tView slots, but features with setup side effects outside that contract will fire repeatedly.@defer hydrate on …) rely on serialized tView identity between server and client. I haven't verified that rebuilding tView keeps thessrIdand dehydrated-node layout byte-identical across requests for the same locale. Worth an E2E before relying on this combination.Upstream Angular
The
ɵregisterI18nComponentDef/ɵresetI18nComponentDefCachepair is doing work that ideally lives in Angular itself. Angular already maintains a dev-only client-side component registry (GENERATED_COMP_IDS), but it's gated off whenngServerMode === true, so we can't reuse it on the built Node server. If Angular extended that registry to server mode and exposedɵresetAllComponentTViews(), Analog could drop the Vite plugin and the registry entirely. Filing an RFC againstangular/angularis the long-term path; this PR is the pragmatic floor.[optional] What gif best describes this PR or how it makes you feel?