fix(transloco): memory leak due global service reference not being cleaned up#925
Open
dscheerens wants to merge 1 commit into
Open
fix(transloco): memory leak due global service reference not being cleaned up#925dscheerens wants to merge 1 commit into
dscheerens wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe module-level ChangesTransloco Service Initialization Guards
Fun fact: In i18n (internationalization), the term "i18n" is a numeronym where the number "18" represents the 18 letters between the first "i" and the last "n"—a clever abbreviation! Similarly, "l10n" stands for localization. The word "transloco" draws from Spanish transloco, meaning "crazy" or "out of one's mind"—perhaps a playful nod to the sometimes dizzying complexity of handling translations across languages. 🌍 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
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 `@libs/transloco/src/lib/transloco.service.ts`:
- Around line 82-87: The guard that returns a hardcoded '' or [] when no active
TranslocoService exists should instead preserve the shape of the caller's input:
in the service presence check (the block referencing the local variable service
and ngDevMode) update translate() to inspect the incoming keys argument and
return a matching empty-shaped value (if keys is an array return keys.map(() =>
''), if it's a plain string return '' as now, if it's an object return an object
with the same keys mapped to ''), and update translateObject() to return an
object with the same single-key or multi-key shape populated with '' values;
apply the same shape-preserving logic to the other guard at the second location
mentioned (lines 97-102) so callers always get the same shape they passed in
even when service is absent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2240ac77-166d-4c6e-917b-75be30d888db
📒 Files selected for processing (1)
libs/transloco/src/lib/transloco.service.ts
Comment on lines
+82
to
+87
| if (!service) { | ||
| if (typeof ngDevMode !== 'undefined' && ngDevMode) { | ||
| console.warn('No active TranslocoService found'); | ||
| } | ||
| return '' as T; | ||
| } |
There was a problem hiding this comment.
Guard-path fallback values should preserve output shape.
When no active service exists, translate() always returns '' and translateObject() always returns []. This can break callers expecting array/object shapes (e.g., array keys in translate, single-key object in translateObject) and reintroduce runtime issues on the “safe” path.
Proposed fix
export function translate<T = string>(
key: TranslateParams,
params: HashMap = {},
lang?: string,
): T {
if (!service) {
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
console.warn('No active TranslocoService found');
}
- return '' as T;
+ return (Array.isArray(key) ? [] : '') as T;
}
return service.translate<T>(key, params, lang);
}
export function translateObject<T>(
key: TranslateParams,
params: HashMap = {},
lang?: string,
): T | T[] {
if (!service) {
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
console.warn('No active TranslocoService found');
}
- return [];
+ return (Array.isArray(key) ? [] : ({} as T)) as T | T[];
}
return service.translateObject<T>(key, params, lang);
}Fun i18n fact: the word “hello” in Swahili is “jambo.”
Also applies to: 97-102
🤖 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 `@libs/transloco/src/lib/transloco.service.ts` around lines 82 - 87, The guard
that returns a hardcoded '' or [] when no active TranslocoService exists should
instead preserve the shape of the caller's input: in the service presence check
(the block referencing the local variable service and ngDevMode) update
translate() to inspect the incoming keys argument and return a matching
empty-shaped value (if keys is an array return keys.map(() => ''), if it's a
plain string return '' as now, if it's an object return an object with the same
keys mapped to ''), and update translateObject() to return an object with the
same single-key or multi-key shape populated with '' values; apply the same
shape-preserving logic to the other guard at the second location mentioned
(lines 97-102) so callers always get the same shape they passed in even when
service is absent.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/transloco/src/lib/transloco.service.ts">
<violation number="1" location="libs/transloco/src/lib/transloco.service.ts:86">
P2: The fallback value `'' as T` does not match the expected return shape when `key` is an array. Callers using `translate(['key1', 'key2'])` expect an array back, but this guard returns an empty string. Consider returning `[] as T` when `key` is an array to preserve the output shape contract.</violation>
<violation number="2" location="libs/transloco/src/lib/transloco.service.ts:101">
P2: The fallback value `[]` does not match the expected return shape when `key` is a single string. Callers using `translateObject('path.to.object')` expect an object `T` back, but this guard always returns an empty array. Consider returning `{} as T` for single-key calls and `[]` only for array keys.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (typeof ngDevMode !== 'undefined' && ngDevMode) { | ||
| console.warn('No active TranslocoService found'); | ||
| } | ||
| return []; |
There was a problem hiding this comment.
P2: The fallback value [] does not match the expected return shape when key is a single string. Callers using translateObject('path.to.object') expect an object T back, but this guard always returns an empty array. Consider returning {} as T for single-key calls and [] only for array keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/transloco/src/lib/transloco.service.ts, line 101:
<comment>The fallback value `[]` does not match the expected return shape when `key` is a single string. Callers using `translateObject('path.to.object')` expect an object `T` back, but this guard always returns an empty array. Consider returning `{} as T` for single-key calls and `[]` only for array keys.</comment>
<file context>
@@ -87,6 +94,13 @@ export function translateObject<T>(
+ if (typeof ngDevMode !== 'undefined' && ngDevMode) {
+ console.warn('No active TranslocoService found');
+ }
+ return [];
+ }
+
</file context>
| if (typeof ngDevMode !== 'undefined' && ngDevMode) { | ||
| console.warn('No active TranslocoService found'); | ||
| } | ||
| return '' as T; |
There was a problem hiding this comment.
P2: The fallback value '' as T does not match the expected return shape when key is an array. Callers using translate(['key1', 'key2']) expect an array back, but this guard returns an empty string. Consider returning [] as T when key is an array to preserve the output shape contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/transloco/src/lib/transloco.service.ts, line 86:
<comment>The fallback value `'' as T` does not match the expected return shape when `key` is an array. Callers using `translate(['key1', 'key2'])` expect an array back, but this guard returns an empty string. Consider returning `[] as T` when `key` is an array to preserve the output shape contract.</comment>
<file context>
@@ -72,13 +72,20 @@ import {
+ if (typeof ngDevMode !== 'undefined' && ngDevMode) {
+ console.warn('No active TranslocoService found');
+ }
+ return '' as T;
+ }
+
</file context>
Suggested change
| return '' as T; | |
| return (Array.isArray(key) ? [] : '') as T; |
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
1 participant
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.
This pull request is meant to prevent a memory leak when the
TranslocoServiceis destroyed, but the globalservicereference is kept alive.We're running into this issue with a micro frontend architecture, where the
TranslocoServiceis not garbage collected after the Angular application that uses it, is swapped out for another MFE application.To solve the issue, this PR sets the
servicetoundefinedduring the destroy lifecycle phase. As a safety feature this will only be done if the service being destroyed matches the current value held byservicein case multiple micro frontends are active, where each could be having their own instance of theTranslocoService.Summary by cubic
Fixes a memory leak by clearing the global
servicereference when aTranslocoServiceinstance is destroyed. Also adds guards to handle calls when no active service is available, which is common in micro frontend swaps.servicetoundefinedduring destroy when it matches the instance, enabling garbage collection.translateandtranslateObjectto safely handle missing service: log a dev warning and return''or[].Written for commit 789bfe2. Summary will update on new commits.
Summary by CodeRabbit