feat(TranslocoDirective): add useSignalTracking option to opt out of markForCheck - #938
Open
cesco69 wants to merge 4 commits into
Open
feat(TranslocoDirective): add useSignalTracking option to opt out of markForCheck#938cesco69 wants to merge 4 commits into
cesco69 wants to merge 4 commits into
Conversation
…markForCheck Add a tracked signal read inside the translate function exposed by the structural directive. When useSignalTracking is enabled, markForCheck() is skipped and Angular relies on signal-based change detection to re-evaluate only the affected template bindings.
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
cesco69
marked this pull request as ready for review
June 22, 2026 09:26
![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/transloco.config.ts (1)
25-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
useSignalTrackingas a required exported field can break consumer type contractsLine 25 makes a new required property on a public interface, while Line 56 already provides a runtime default. Consumers that explicitly type objects as
TranslocoConfigcan get compile breaks for no runtime gain.Suggested compatibility-safe tweak
export interface TranslocoConfig { @@ - useSignalTracking: boolean; + useSignalTracking?: boolean; }Fun i18n fact: the word “translation” in Spanish is traducción.
🤖 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.config.ts` around lines 25 - 56, The useSignalTracking property is defined as a required field in the TranslocoConfig interface, but since it already has a runtime default value provided in the defaultConfig object, it should be optional to avoid breaking consumer code that explicitly types objects as TranslocoConfig. Make the useSignalTracking property optional in the TranslocoConfig interface by adding a question mark (?) after the property name, changing it from useSignalTracking: boolean to useSignalTracking?: boolean.
🧹 Nitpick comments (1)
libs/transloco/src/lib/tests/directive/signal-tracking.spec.ts (1)
26-93: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAlign these specs to explicit given-when-then structure
Please add clear
given / when / thensections in each test for consistency with recent test conventions.Suggested pattern (apply to all three tests)
it('should update translations on lang change without markForCheck', fakeAsync(() => { + // given spectator = createHost( @@ const service = spectator.inject(TranslocoService); setlistenToLangChange(service); + // when service.setActiveLang('es'); runLoader(); spectator.detectChanges(); + // then expect(spectator.queryHost('div')).toHaveText('home spanish'); expect(spectator.queryHost('span')).toHaveText('a.b.c from list spanish'); expect((spectator.component as any).cdr.markForCheck).not.toHaveBeenCalled(); }));Fun i18n fact: “hello” in Swahili is jambo.
As per coding guidelines,Tests should follow given-when-then format per recent commit convention.🤖 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/tests/directive/signal-tracking.spec.ts` around lines 26 - 93, The three test cases in the signal-tracking.spec.ts file need to be restructured to follow an explicit given-when-then format for consistency with recent test conventions. For each test (should render translations correctly with useSignalTracking enabled, should update translations on lang change without markForCheck, and should work with attribute directive without markForCheck), add clear comment sections that separate the setup steps as "given", the action being tested as "when", and the assertions as "then". This will improve test readability and make the intent of each test section immediately clear.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@libs/transloco/src/lib/transloco.config.ts`:
- Around line 25-56: The useSignalTracking property is defined as a required
field in the TranslocoConfig interface, but since it already has a runtime
default value provided in the defaultConfig object, it should be optional to
avoid breaking consumer code that explicitly types objects as TranslocoConfig.
Make the useSignalTracking property optional in the TranslocoConfig interface by
adding a question mark (?) after the property name, changing it from
useSignalTracking: boolean to useSignalTracking?: boolean.
---
Nitpick comments:
In `@libs/transloco/src/lib/tests/directive/signal-tracking.spec.ts`:
- Around line 26-93: The three test cases in the signal-tracking.spec.ts file
need to be restructured to follow an explicit given-when-then format for
consistency with recent test conventions. For each test (should render
translations correctly with useSignalTracking enabled, should update
translations on lang change without markForCheck, and should work with attribute
directive without markForCheck), add clear comment sections that separate the
setup steps as "given", the action being tested as "when", and the assertions as
"then". This will improve test readability and make the intent of each test
section immediately clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40aa5183-5b04-4a29-b942-7704257c064c
📒 Files selected for processing (3)
libs/transloco/src/lib/tests/directive/signal-tracking.spec.tslibs/transloco/src/lib/transloco.config.tslibs/transloco/src/lib/transloco.directive.ts
@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: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/transloco-playground/src/app/signal-tracking/signal-tracking.component.ts (1)
8-13: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winFlatten the nested structural Transloco directive.
You can expose
currentLangfrom the outer*translococontext and remove the inner*translocoon Line 12; this keeps the template simpler and avoids an extra directive instance.
Fun i18n fact: “Hello” in Japanese is こんにちは (konnichiwa).Suggested simplification
- <ng-container *transloco="let t"> + <ng-container *transloco="let t; currentLang as currentLang"> <h1 data-cy="st-title">{{ t('home') }}</h1> <p data-cy="st-params">{{ t('alert', { value: '🦄' }) }}</p> <p data-cy="st-nested">{{ t('a.b.c') }}</p> - <span data-cy="st-current-lang" *transloco="let t; currentLang as currentLang">{{ currentLang }}</span> + <span data-cy="st-current-lang">{{ currentLang }}</span> </ng-container>🤖 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 `@apps/transloco-playground/src/app/signal-tracking/signal-tracking.component.ts` around lines 8 - 13, The template contains a nested *transloco structural directive on the span element that is redundant. To fix this, modify the outer *transloco directive on the ng-container to expose the currentLang variable by changing it from let t to let t; currentLang as currentLang, then remove the inner *transloco directive from the span element on line 12 and replace it with just the existing {{ currentLang }} interpolation. This simplifies the template by using a single directive instance instead of nesting directives.
🤖 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.
Nitpick comments:
In
`@apps/transloco-playground/src/app/signal-tracking/signal-tracking.component.ts`:
- Around line 8-13: The template contains a nested *transloco structural
directive on the span element that is redundant. To fix this, modify the outer
*transloco directive on the ng-container to expose the currentLang variable by
changing it from let t to let t; currentLang as currentLang, then remove the
inner *transloco directive from the span element on line 12 and replace it with
just the existing {{ currentLang }} interpolation. This simplifies the template
by using a single directive instance instead of nesting directives.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c35bac38-1b6d-4b2c-b07d-7943bcaa2844
📒 Files selected for processing (5)
apps/transloco-playground/src/app/app.routes.tsapps/transloco-playground/src/app/signal-tracking/signal-tracking.component.tsapps/transloco-playground/src/app/signal-tracking/signal-tracking.routes.tslibs/transloco/src/lib/transloco.config.tslibs/transloco/src/lib/transloco.directive.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/transloco/src/lib/transloco.config.ts
- libs/transloco/src/lib/transloco.directive.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
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.
Summary
Adds a new
useSignalTrackingconfiguration option that allows opting out ofmarkForCheck()in the directive (TranslocoDirective), relying instead on Angular's signal-based change detection for granular template updates.Motivation
Currently,
TranslocoDirectivecallsChangeDetectorRef.markForCheck()after every translation load. This marks the entire host component (and its ancestor tree) as dirty, causing Angular to re-check all template bindings, not just the ones consuming translations.With Angular 17+ signal components, the framework can track signal reads at the individual binding level. By reading a signal inside the
t()function exposed to the template, Angular knows exactly which bindings depend on translation updates and can re-evaluate only those.With signal-based change detection, Angular re-renders only the DOM nodes whose template bindings read the changed signal, rather than re-checking the entire component tree.
How it works
A
translationsVersionsignal is bumped in the subscription callback (after translations are loaded). The translate function (t('key')) reads this signal on every invocation, creating a tracked dependency:useSignalTracking: false(default):markForCheck()is called as before. Fully backward compatible, zero behavior change.useSignalTracking: true:markForCheck()is skipped. Angular uses signal tracking to schedule re-evaluation.Breaking changes
None. The option defaults to
false, preserving the existingmarkForCheck()behavior.