refactor(transloco): replace isBrowser() with ngServerMode for SSR detection#909
Open
arturovt wants to merge 1 commit into
Open
refactor(transloco): replace isBrowser() with ngServerMode for SSR detection#909arturovt wants to merge 1 commit into
arturovt wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds an ambient TypeScript global Changes
Suggested reviewers
Fun fact: "Transloco" helps preserve linguistic nuance—e.g., the Welsh word "hiraeth" has no direct English equivalent and conveys a mix of longing, homesickness, and nostalgia. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
@jsverse/transloco
@jsverse/transloco-locale
@jsverse/transloco-messageformat
@jsverse/transloco-optimize
@jsverse/transloco-persist-lang
@jsverse/transloco-persist-translations
@jsverse/transloco-preload-langs
@jsverse/transloco-schematics
@jsverse/transloco-scoped-libs
@jsverse/transloco-utils
@jsverse/transloco-validator
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/transloco/src/lib/utils/browser.utils.ts (1)
24-31:⚠️ Potential issue | 🟠 MajorAdd a non-browser fallback in addition to
ngServerMode.The code only checks
ngServerModebefore accessingwindow.navigator. However,ngServerMode(introduced in Angular 19) is not guaranteed to be defined in all SSR executions—custom builders, test runners, and non-standard server runtimes may omit it. If it's undefined and the function runs outside a browser, line 29 crashes withReferenceError: window is not defined.Guard against this by also checking whether
windowis defined:Proposed fix
export function getBrowserCultureLang(): string { - if (typeof ngServerMode !== 'undefined' && ngServerMode) { + if ( + (typeof ngServerMode !== 'undefined' && ngServerMode) || + typeof window === 'undefined' + ) { return ''; } const navigator = window.navigator; return navigator.languages?.[0] ?? navigator.language; }Fun i18n fact: The term "internationalization" is often abbreviated as "i18n" because there are 18 letters between the 'i' and 'n'—this same abbreviation pattern applies to "localization" as "l10n" (10 letters).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/transloco/src/lib/utils/browser.utils.ts` around lines 24 - 31, The function getBrowserCultureLang currently only checks ngServerMode before touching window.navigator, which can still throw if window is undefined in non-browser runtimes; update getBrowserCultureLang to also guard for typeof window !== 'undefined' (preserving the existing ngServerMode check) and return '' when not in a browser environment so the code never accesses window.navigator unless window is present and ngServerMode is false; locate the check in getBrowserCultureLang and add the window typeof guard around the navigator access.
🧹 Nitpick comments (1)
libs/transloco-persist-lang/src/lib/persist-lang.service.ts (1)
26-46: Consolidate repeated SSR checks into one helper.The same guard appears three times; centralizing it will reduce drift and make future SSR policy changes safer.
Fun i18n fact: in Arabic, “language” is لغة (lugha).Refactor sketch
export class TranslocoPersistLangService implements OnDestroy { @@ + private isServerSide(): boolean { + return ( + (typeof ngServerMode !== 'undefined' && ngServerMode) || + typeof window === 'undefined' + ); + } + constructor() { - if (typeof ngServerMode !== 'undefined' && ngServerMode) { + if (this.isServerSide()) { return; } this.init(); } getCachedLang(): string | null { - if (typeof ngServerMode !== 'undefined' && ngServerMode) { + if (this.isServerSide()) { return null; - } else { - return this.storage.getItem(this.storageKey); } + return this.storage.getItem(this.storageKey); } clear() { - if (typeof ngServerMode !== 'undefined' && ngServerMode) { + if (this.isServerSide()) { return; } this.storage.removeItem(this.storageKey); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts` around lines 26 - 46, Create a single helper (e.g., isServerSide or isNgServerMode) that encapsulates the repeated typeof ngServerMode !== 'undefined' && ngServerMode check and use it in init(), getCachedLang(), and clear() to replace the three duplicated guards; update getCachedLang() to return null when the helper is true and clear() / init() to return early when the helper is true, keeping references to this.storage, this.storageKey, this.init(), and the original storage.getItem/storage.removeItem calls unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@libs/transloco/src/lib/utils/browser.utils.ts`:
- Around line 24-31: The function getBrowserCultureLang currently only checks
ngServerMode before touching window.navigator, which can still throw if window
is undefined in non-browser runtimes; update getBrowserCultureLang to also guard
for typeof window !== 'undefined' (preserving the existing ngServerMode check)
and return '' when not in a browser environment so the code never accesses
window.navigator unless window is present and ngServerMode is false; locate the
check in getBrowserCultureLang and add the window typeof guard around the
navigator access.
---
Nitpick comments:
In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts`:
- Around line 26-46: Create a single helper (e.g., isServerSide or
isNgServerMode) that encapsulates the repeated typeof ngServerMode !==
'undefined' && ngServerMode check and use it in init(), getCachedLang(), and
clear() to replace the three duplicated guards; update getCachedLang() to return
null when the helper is true and clear() / init() to return early when the
helper is true, keeping references to this.storage, this.storageKey,
this.init(), and the original storage.getItem/storage.removeItem calls
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2af7057-3ca2-4f8d-a35f-80d85097ef26
📒 Files selected for processing (5)
@types/angular-globals/index.d.tslibs/transloco-persist-lang/src/lib/persist-lang.service.tslibs/transloco-persist-lang/tsconfig.lib.jsonlibs/transloco/src/index.tslibs/transloco/src/lib/utils/browser.utils.ts
💤 Files with no reviewable changes (1)
- libs/transloco/src/index.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
shaharkazaz
reviewed
Apr 4, 2026
Comment on lines
+10
to
+6
| if (!browserLang || !isBrowser()) { | ||
| if (!browserLang) { |
Collaborator
There was a problem hiding this comment.
No need to check here as well?
Collaborator
Author
There was a problem hiding this comment.
No, because getBrowserCultureLang already guards it.
Collaborator
There was a problem hiding this comment.
For consistency I would still use the check here, I wouldn't want to depend on another function that might change
arturovt
force-pushed
the
refactor/remove-isBrowser
branch
from
f70f73d to
da04ff3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/transloco-persist-lang/src/lib/persist-lang.service.ts (1)
33-69: Consider centralizing the SSR predicate to avoid drift.The same condition is repeated three times. A single helper/readonly flag will make future SSR changes safer.
♻️ Suggested refactor
export class TranslocoPersistLangService implements OnDestroy { private injector = inject(Injector); + private readonly isServerRuntime = + (typeof ngServerMode !== 'undefined' && ngServerMode) || + isPlatformServer(this.injector.get(PLATFORM_ID)); constructor() { - if ( - (typeof ngServerMode !== 'undefined' && ngServerMode) || - isPlatformServer(inject(PLATFORM_ID)) - ) { + if (this.isServerRuntime) { return; } this.init(); } getCachedLang(): string | null { - if ( - (typeof ngServerMode !== 'undefined' && ngServerMode) || - isPlatformServer(this.injector.get(PLATFORM_ID)) - ) { + if (this.isServerRuntime) { return null; - } else { - return this.storage.getItem(this.storageKey); } + return this.storage.getItem(this.storageKey); } clear() { - if ( - (typeof ngServerMode !== 'undefined' && ngServerMode) || - isPlatformServer(this.injector.get(PLATFORM_ID)) - ) { + if (this.isServerRuntime) { return; } this.storage.removeItem(this.storageKey); }Fun i18n fact: Unicode currently supports well over 100,000 characters, enabling scripts from thousands of locales.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts` around lines 33 - 69, Extract the repeated SSR detection into a single readonly boolean (e.g. readonly isServer) computed once in the constructor using the same predicate that currently appears in constructor/getCachedLang/clear (check ngServerMode, isPlatformServer and PLATFORM_ID via inject or this.injector as used today), then replace the duplicated conditions in getCachedLang() and clear() with checks against this.isServer; keep init() call and storage access (this.storage.getItem/removeItem(this.storageKey)) behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts`:
- Around line 33-69: Extract the repeated SSR detection into a single readonly
boolean (e.g. readonly isServer) computed once in the constructor using the same
predicate that currently appears in constructor/getCachedLang/clear (check
ngServerMode, isPlatformServer and PLATFORM_ID via inject or this.injector as
used today), then replace the duplicated conditions in getCachedLang() and
clear() with checks against this.isServer; keep init() call and storage access
(this.storage.getItem/removeItem(this.storageKey)) behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97d68f34-bd15-4806-b3a5-0f0120aa7f88
📒 Files selected for processing (4)
@types/angular-globals/index.d.tslibs/transloco-persist-lang/src/lib/persist-lang.service.tslibs/transloco-persist-lang/tsconfig.lib.jsonlibs/transloco/src/lib/utils/browser.utils.ts
✅ Files skipped from review due to trivial changes (2)
@types/angular-globals/index.d.ts- libs/transloco-persist-lang/tsconfig.lib.json
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/transloco/src/lib/utils/browser.utils.ts
shaharkazaz
reviewed
Apr 5, 2026
Comment on lines
+61
to
+64
| if ( | ||
| (typeof ngServerMode !== 'undefined' && ngServerMode) || | ||
| isPlatformServer(this.injector.get(PLATFORM_ID)) | ||
| ) { |
Collaborator
There was a problem hiding this comment.
Let's introduce #isSSR
Collaborator
Author
There was a problem hiding this comment.
This would not be tree-shakable. Functions being used twice are not inlined.
Comment on lines
+10
to
+6
| if (!browserLang || !isBrowser()) { | ||
| if (!browserLang) { |
Collaborator
There was a problem hiding this comment.
For consistency I would still use the check here, I wouldn't want to depend on another function that might change
| if (!isBrowser()) { | ||
| // See SSR guard in `isBrowser`. When `ngServerMode` is defined, bundlers can inline | ||
| // the `isBrowser()` result as `false`, eliminating the runtime call entirely. | ||
| if ((typeof ngServerMode !== 'undefined' && ngServerMode) || !isBrowser()) { |
Collaborator
There was a problem hiding this comment.
If we just use isBrowser in an SSR env, won't both get shaken out?
Collaborator
Author
There was a problem hiding this comment.
Functions being used twice are not inlined.
…tection Remove the isBrowser() utility in favor of Angular's native ngServerMode global for server-side rendering detection. This aligns with Angular's recommended approach for SSR guards. - Replace isBrowser() checks in TranslocoPersistLangService with ngServerMode guards - Add ngServerMode declaration to @types/angular-globals - Include angular-globals types in transloco-persist-lang tsconfig.lib.json
arturovt
force-pushed
the
refactor/remove-isBrowser
branch
from
da04ff3 to
c1e2ebc
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/transloco-persist-lang/src/lib/persist-lang.service.ts (1)
37-40: Only fall back whenngServerModeis unavailable.With the current
||guard,isPlatformServer(...)still runs whenngServerModeexists but isfalse, so the fallback is not limited to older/non-ngServerModeruntimes andPLATFORM_IDremains on the hot path. Prefer the inline ternary form to preserve the intended precedence without introducing a non-tree-shakable helper.♻️ Proposed guard rewrite
- (typeof ngServerMode !== 'undefined' && ngServerMode) || - isPlatformServer(inject(PLATFORM_ID)) + typeof ngServerMode !== 'undefined' + ? ngServerMode + : isPlatformServer(inject(PLATFORM_ID))- (typeof ngServerMode !== 'undefined' && ngServerMode) || - isPlatformServer(this.injector.get(PLATFORM_ID)) + typeof ngServerMode !== 'undefined' + ? ngServerMode + : isPlatformServer(this.injector.get(PLATFORM_ID))Fun i18n fact: “langue de repli” is French for “fallback language.”
Also applies to: 49-52, 61-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts` around lines 37 - 40, The guard currently uses "(... && ngServerMode) || isPlatformServer(inject(PLATFORM_ID))" which evaluates isPlatformServer even when ngServerMode exists but is false; change each occurrence to use a ternary so isPlatformServer is only called when ngServerMode is undefined: replace the expression with "typeof ngServerMode !== 'undefined' ? ngServerMode : isPlatformServer(inject(PLATFORM_ID))". Apply this fix for the three occurrences referencing ngServerMode, isPlatformServer, inject, and PLATFORM_ID in persist-lang.service.ts (the blocks around lines 37-40, 49-52, and 61-64).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/transloco-persist-lang/src/lib/persist-lang.service.ts`:
- Around line 37-40: The guard currently uses "(... && ngServerMode) ||
isPlatformServer(inject(PLATFORM_ID))" which evaluates isPlatformServer even
when ngServerMode exists but is false; change each occurrence to use a ternary
so isPlatformServer is only called when ngServerMode is undefined: replace the
expression with "typeof ngServerMode !== 'undefined' ? ngServerMode :
isPlatformServer(inject(PLATFORM_ID))". Apply this fix for the three occurrences
referencing ngServerMode, isPlatformServer, inject, and PLATFORM_ID in
persist-lang.service.ts (the blocks around lines 37-40, 49-52, and 61-64).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b08a854-ecf0-4b82-97ec-0b03bacd86df
📒 Files selected for processing (4)
@types/angular-globals/index.d.tslibs/transloco-persist-lang/src/lib/persist-lang.service.tslibs/transloco-persist-lang/tsconfig.lib.jsonlibs/transloco/src/lib/utils/browser.utils.ts
✅ Files skipped from review due to trivial changes (2)
@types/angular-globals/index.d.ts- libs/transloco-persist-lang/tsconfig.lib.json
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/transloco/src/lib/utils/browser.utils.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the isBrowser() utility in favor of Angular's native ngServerMode global for server-side rendering detection. This aligns with Angular's recommended approach for SSR guards.
Summary by cubic
Switched SSR detection to Angular’s
ngServerMode, with a fallback toisPlatformServer, to avoid running browser APIs on the server and improve tree-shaking.TranslocoPersistLangService(constructor,getCachedLang,clear) withngServerModeorisPlatformServer(inject(PLATFORM_ID)); added anInjectorfor runtimePLATFORM_IDaccess.isBrowser()returns false whenngServerModeis true (falls back towindowcheck);getBrowserLang()drops the directisBrowsercheck;getBrowserCultureLang()short-circuits onngServerModeor non-browser.ngServerModein@types/angular-globalsand included it intransloco-persist-langtsconfig.lib.jsontypes.Written for commit c1e2ebc. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Refactor
Chores