fix(linker): resolve changeDetection default from the declaration version - #429
Merged
Brooooooklyn merged 2 commits intoAug 10, 2026
Conversation
…ersion The linker emitted `changeDetection` only when the partial declaration carried the field, with no default for the omitted case. Which strategy an omitted field means depends on the version the library was compiled against: v22 made OnPush the default, so anything older meant `Eager`. The TS linker resolves that with `hasOnPushByDefault = major >= 22 || version === PLACEHOLDER_VERSION` in `partial_component_linker_1.ts`. Because the runtime derives `onPush = changeDetection !== Eager`, an absent field reads as OnPush. A pre-v22 component that never declared a strategy was therefore silently switched from Eager to OnPush, and stops re-rendering on anything that is not a signal or input change. Nothing is logged, so it surfaces only as stale UI. Six components in a typical `node_modules` hit this, including `AgGridAngular` (v17.3.12), `HighchartsChartComponent` (v16.2.12), `MarkdownComponent` (v21.2.5) and `_MatDialogContainerBase` (v16.1.1). Also stops emitting `changeDetection: 0` for an explicit OnPush. OnPush is the emit-time default and the TS compiler leaves it out (`meta.changeDetection !== OnPush` in `compileComponentFromMetadata`); the runtime infers it from the absent field. This is byte-identical in behaviour, and it clears the last `changeDetection` divergence when diffing linked output against the official linker across a real `node_modules`.
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90c1a1984e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Reverts the parity tweak that dropped the field for OnPush, per review. The TS compiler omits OnPush, but it ships with the runtime it targets. This linker does not know the consumer's version: `angularLinkerPlugin()` takes no options and `linkCode` calls `linkAngularPackage(code, id)`, and the plugin's `angularVersion` option documents support back to v19. Angular 20 and 21 compute `onPush = changeDetection === OnPush`, so an absent field reads as Default there and the component would silently lose OnPush. `0` is correct on both: pre-v22 `0 === 0`, and v22 `0 !== 1`. The version-gated default for an omitted field is unaffected, since `1` is likewise correct on both.
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Brooooooklyn
approved these changes
Aug 10, 2026
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.
Summary
The linker emitted
changeDetectiononly when the partial declaration carried the field, with no default for the omitted case.Which strategy an omitted field means depends on the version the library was compiled against. Angular v22 made OnPush the default, so anything older meant
Eager. The TS linker resolves that explicitly:Impact
In v22
OnPush = 0,Eager = 1, andDefault = 1is a deprecated alias forEager. The runtime derives the flag from:So an absent field evaluates
undefined !== 1and reads as OnPush. A pre-v22 component that never declared a strategy was therefore silently switched from Eager to OnPush, and stops re-rendering on anything that is not a signal or input change. Nothing is logged, so it surfaces only as stale UI.Scanning a real
node_modules: 154ɵɵngDeclareComponentdeclarations, 139 of them pre-v22, and 6 of those omit the field. Most libraries do declare a strategy, so the blast radius is narrow, but the ones that do not are unlucky:AgGridAngularag-grid-angularHighchartsChartComponenthighcharts-chartMarkdownComponentmarkdown_MatDialogContainerBaseNgpTooltipTextContentComponentDynamicViewComponentdynamic-viewA grid or chart component quietly becoming OnPush is a rough one to track down.
Also in this change (reverted after review)
This originally stopped emitting
changeDetection: 0for an explicit OnPush, to match themeta.changeDetection !== OnPushguard incompileComponentFromMetadata. That was wrong and has been reverted in cdd3353.That compiler ships with the runtime it targets. This linker does not know the consumer's version:
angularLinkerPlugin()takes no options andlinkCodecallslinkAngularPackage(code, id), while the plugin'sangularVersionoption documents support back to v19. Angular 20 and 21 computeonPush = changeDetection === OnPush, so an absent field reads as Default there and the component would have silently lost OnPush. Emitting0is correct on both: pre-v220 === 0, v220 !== 1.So this PR is now just the version-gated default.
1is likewise correct on both runtimes (pre-v221 === 0is false, v221 !== 1is false, both Eager/Default).One existing test asserted the
changeDetection: 0emit; it is unchanged, and now carries a comment explaining why the emit has to stay so a future parity cleanup trips over it.Tests
Three tests covering the version gate: pre-v22 without the field must emit
changeDetection: 1, v22+ must not emit, and0.0.0-PLACEHOLDERmust not emit. Only the first is red before the fix; the other two are guards for behaviour that must not change.The version gate reuses the shape of the existing
get_default_standalone_valuehelper, which already does the same semver-plus-placeholder resolution forstandalone.Verification
Every CI step run locally on macOS:
cargo check --all-featurescargo testcargo fmt --all -- --checkcargo run -p oxc_angular_conformancepnpm build-dev+build:tspnpm testpnpm checkpnpm test:e2ecompare --fixturesAlso rebuilt the napi binding and confirmed
ag-grid-angular,highcharts-angularandng-primitivesnow emitchangeDetection: 1.Re-ran the full suite after the revert; all of the above still pass.
Note
This was found by diffing linked output against
@angular/compiler-cli's linker rather than by any existing test, and the same exercise found #428.compare --fixturesstays at 100% throughout, because that harness covers source to AOT compilation and has nongDeclarefixtures, so the linker currently has no differential coverage against the official linker. A partial-declaration category there would likely have caught both, but that is well outside the scope of this change.