fix(directive): chain query instructions on Angular ≥ 21.0.4, align _c<N> ordering#323
Merged
brandonroberts merged 2 commits intoMay 30, 2026
Conversation
8a6ecc4 to
9bf5c52
Compare
9bf5c52 to
63701a9
Compare
Collaborator
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
Collaborator
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63701a9b1b
ℹ️ 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".
`ɵɵviewQuery`, `ɵɵcontentQuery`, `ɵɵviewQuerySignal`, and
`ɵɵcontentQuerySignal` switched from returning `void` to returning
`typeof <fn>` in `angular/angular@ae1c0dc4` ("perf(compiler): chain
query creation instructions"), cherry-picked into v21.0.4 as
`f901cc9e`. From that release on, upstream ngtsc chains consecutive
same-instruction calls via `instructionChainAfter()` (every
compliance golden under `signal_queries/` and
`r3_compiler_compliance/.../queries/` shows the chained form). This
compiler emitted separate statements unconditionally — a known
runtime-safe form that disagrees with the upstream emit on supported
modern Angular.
This change adds a new `AngularVersion::supports_chained_queries()`
predicate (`major > 21 || (major == 21 && (minor > 0 || patch >= 4))`)
and gates chained emit in both `create_view_queries_function` and
`create_content_queries_function`. When the predicate returns `false`
— including when the consumer doesn't pass a version (`None`) — both
functions fall back to the existing safe form, one expression
statement per query. v19, v20, and v21.0.0–v21.0.3 stay on that
fallback path; chaining them would throw `TypeError: not a function`
at runtime.
Mixed signal + decorator queries on a single directive: the chain
breaks at the signal-ness boundary (different runtime symbols can't
chain), matching upstream behavior.
`compile_directive`, `compile_directive_from_metadata`,
`build_base_directive_fields`, `generate_directive_definitions`, and
`generate_dir_definition` gain an `Option<AngularVersion>` parameter
so the new version-aware emit can be threaded from `TransformOptions`.
Tests:
- Three unit tests in `directive/query` rewritten to pass v22 and
assert the chained form.
- Three integration tests rewritten with v22 and the corresponding
assertions.
- New `test_query_chaining_falls_back_to_separate_statements_pre_v21_0_4`
exercises the safe fallback for `None`, v19.2.0, v20.0.0, and
v21.0.3, plus a sanity check that v21.0.4 chains.
Linker (`linker/mod.rs`) is intentionally left unchanged. The linker
processes partial declarations into final form without knowing the
target runtime version, so emitting chained calls there would be
unsafe on pre-v21.0.4 runtimes. Worth revisiting once the linker
gains version awareness.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cates Upstream ngtsc's `compileComponent` emits the `contentQueries:` field before `viewQuery:` in the component definition object literal, and pools predicates in that order. The resulting `_c<N>` constant-table indices end up content-first (`_c0`, `_c1` for content; `_c2`, `_c3` for view) — the form pinned by every compliance golden under `compiler-cli/test/compliance/test_cases/signal_queries/`. This compiler had the order reversed (view queries pooled first), so the same source produced `_c0`/`_c1` for view queries and `_c2`/`_c3` for content — runtime-equivalent but breaks emit-anchored tooling and diff-based comparisons against Angular's own goldens. Fix: swap the call order in `compile_component_full` so content queries are pooled and emitted before view queries. Runtime behavior is unaffected on every supported Angular version — only the const-table indices shift back into upstream alignment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
63701a9 to
2da0379
Compare
Brooooooklyn
approved these changes
May 30, 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
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
ɵɵviewQuery(p1)(p2),ɵɵcontentQuerySignal(...)(...)) when the target Angular runtime supports it — matches upstream ngtsc'sinstructionChainAfter()emit pattern.AngularVersion::supports_chained_queries()predicate so consumers explicitly targeting v19/v20/v21.0.0–v21.0.3 can opt out and keep the safe separate-statement form. (The runtime functions returnedvoiduntil v21.0.4; chaining on those versions throws at runtime.)_c<N>constant-table indices with upstream emit. Runtime-safe on every supported version.Compatibility (v19+)
Behavior across the currently-supported version range:
ɵɵviewQuery/ɵɵcontentQuery/ɵɵ*Signalreturnvoidtypeof <fn>typeof <fn>None)The runtime contract changed in
angular/angular@ae1c0dc4("perf(compiler): chain query creation instructions", cherry-picked into v21.0.4 asf901cc9e). The existing comments in this codebase saying "Angular 20'sɵɵviewQueryreturns void, so chaining is not supported" were correct on v20 and remain correct on v19/v20/v21.0.0–3 — this PR keeps that fallback path available to consumers who pass an explicitangular_version.The first revision of this PR silently changed emit for every supported version, which would have broken v19/v20/v21.0.0–3 at runtime. This revision adds the version gate so the older fallback is reachable.
None= assume latestWhen
angular_versionisNonethe compiler emits the chained form, matching this crate's existing convention (supports_implicit_standalone,supports_service_decorator, etc. all usemap_or(true, …)and thetransform.rscomment readsangular_version: None // assume latest). Consumers targeting v19/v20/v21.0.0–3 already need to setangular_versionexplicitly to get the right emit for several other instructions (ɵɵconditionalCreatevs.ɵɵtemplate,ɵɵdomPropertyvs.ɵɵhostProperty, etc.); the chained-query gate slots into the same opt-out pattern.Why
The Angular runtime source at v22 / v21.0.4+ shows all four query instructions return
typeof <fn>:core/src/render3/instructions/queries.ts:40,58(decorator queries)core/src/render3/instructions/queries_signals.ts:32,53(signal queries)Upstream ngtsc chains consecutive same-instruction calls on those versions via
instructionChainAfter(). Every compliance golden undersignal_queries/andr3_compiler_compliance/components_and_directives/queries/emits the chained form.The pool-ordering swap is a separate, runtime-safe fix: predicate pooling was running view queries first, so
_c0/_c1ended up on view queries while Angular's goldens have them on content queries. One-line swap incompile_component_fullputs content first, aligning const-table indices with every supported Angular version.What changes per file
crates/oxc_angular_compiler/src/component/metadata.rsAngularVersion::supports_chained_queries(). Matches the existingsupports_*pattern. v22+ unconditionally, v21.1.0+ on the v21 line, v21.0.4+ on the v21.0 cherry-pick line. v19/v20 explicitly false.crates/oxc_angular_compiler/src/directive/query.rscreate_view_queries_functionandcreate_content_queries_functiongain anOption<AngularVersion>parameter. Chained-emit guard:angular_version.map_or(true, |v| v.supports_chained_queries())—Noneassumes latest (chained), explicit pre-v21.0.4 falls back to one expression statement per query.is_signalqueries fold viacall_fn(chain, params). The chain flushes when signal-ness flips (different runtime symbol — can't bridge the boundary) and at end-of-loop.crates/oxc_angular_compiler/src/directive/compiler.rsanddefinition.rscompile_directive,compile_directive_from_metadata,build_base_directive_fields,generate_directive_definitions, andgenerate_dir_definitionthread the newOption<AngularVersion>parameter through.crates/oxc_angular_compiler/src/component/transform.rsoptions.angular_versionto the query builders and togenerate_directive_definitions.crates/oxc_angular_compiler/tests/integration_test.rstest_query_chaining_obeys_angular_version_gateexercises both contracts in one place: explicit v19.2.0, v20.0.0, v21.0.3 each fall back to separate statements;None, v21.0.4, v22.0.0 all chain.Linker (intentionally untouched)
The linker (
linker/mod.rs) processes partial declarations into final form without knowing the target runtime version, so chained emit there would be unsafe when the linked library ends up running on the v19/v20/v21.0.0–3 path. Worth revisiting oncebuild_queriesand its callers gain version awareness. The earlier revision of this PR did patch the linker; it's reverted here.Mixed signal + decorator behavior
Worth calling out:
ɵɵviewQueryandɵɵviewQuerySignalare different runtime symbols, so a chain can't cross that boundary. The implementation flushes the chain whenis_signalflips between consecutive queries — same for content queries.test_mixed_signal_and_decorator_view_queries_break_chain_on_boundarycovers this: two signal queries chain into one expression, then the single decorator query produces a separate root call.Test plan
cargo test -p oxc_angular_compiler— all ~2500 tests pass (1047 lib + 398 integration + remainder across other test files).cargo fmt --all -- --checkpasses (clean rustfmt).pnpm --filter ./napi/angular-compiler build) succeeds.angularVersion: { major: 22, minor: 0, patch: 0 }via the adapter. Thesignal_queries/query_in_componentfixture — previously the one known OXC divergence — now matches Angular v22.1.0-next.0 byte-for-byte.test_query_chaining_obeys_angular_version_gateasserts both contracts: explicit pre-v21.0.4 falls back,None/v21.0.4+/v22 chain.