Skip to content

fix(compiler): emit queries/viewQueries in ɵɵngDeclareComponent - #554

Merged
Brooooooklyn merged 2 commits into
mainfrom
fix/issue-513-partial-component-queries
Oct 7, 2026
Merged

Brooooooklyn merged 2 commits into
mainfrom
fix/issue-513-partial-component-queries

Conversation

@Brooooooklyn

Copy link
Copy Markdown
Member

Problem

With compilationMode: 'partial', ɵɵngDeclareComponent dropped queries/viewQueries — @ViewChild/@ContentChild, signal queries (viewChild()/contentChild()), and the queries: decorator field all vanished from the declaration. ɵɵngDeclareDirective already emitted them. The linker then had nothing to turn into contentQueries:/viewQuery: on ɵɵdefineComponent, so queries silently resolved to nothing at runtime.

Fix

Upstream builds the component map on top of the directive map (createComponentDefinitionMap → createDirectiveDefinitionMap), so this reuses the directive path wholesale:

  • extract_class_queries output (already computed in compile_component_full) is threaded into compile_component_partial via two new PartialComponentInputs slices.
  • queries/viewQueries are emitted with the directive emitter's compile_queries_array (now pub(crate), takes a slice), positioned between providers and exportAs — the upstream order in directive.ts:74-80, confirmed against the vendored GOLDEN_PARTIAL.js.
  • minVersion bumps to 17.2.0 when any query is signal-based — the same rule, version, and plain-assignment style the directive path applies (directive.ts:146-149).

No changes to ComponentMetadata — queries only matter for partial emit, so they ride on PartialComponentInputs.

Verification

  • 4 new tests: member-decorator queries, queries: decorator field, signal queries (+ minVersion bump), and a full link() round-trip asserting viewQuery:/contentQueries: in the linked ɵɵdefineComponent.
  • cargo test --all-features green, cargo check + --all-features clean, cargo fmt clean, conformance 1264/1264 (100%).
  • Codex adversarial review: one finding (forwardRef predicates emit unwrapped). Verified pre-existing — the directive path has emitted bare predicates since before this change (partial/directive.rs:468-470 documents it; property_decorators.rs unwraps in parse_query_predicate). Same behavior now inherited by components; fix belongs in shared R3QueryMetadata/compile_query and is a follow-up, not scope for this PR.

Fixes #513

With compilationMode: 'partial', components lost @ViewChild/
@ContentChild, signal queries, and `queries:` decorator metadata —
the declaration map simply never carried them, so the linker had
nothing to put into contentQueries/viewQuery on ɵɵdefineComponent.

Upstream builds the component definition map on top of the directive
map (partial/component.ts createComponentDefinitionMap), so the fix
reuses the directive emitter's compile_queries_array: extracted view
and content queries are threaded through PartialComponentInputs and
emitted between providers and exportAs, matching upstream field order
(directive.ts:74-80, verified against GOLDEN_PARTIAL.js).

Signal queries bump minVersion to 17.2.0, the same rule and version
the directive path already applies (directive.ts:146-149).

Fixes #513
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T04:39:48.972243Z d7e17c2 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb57c39a1f

ℹ️ 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".

Comment thread crates/oxc_angular_compiler/src/partial/component.rs
`forwardRef(() => X)` query predicates were unwrapped at extraction and
emitted bare — for components a new divergence, for directives a
pre-existing one the compile_query comment already documented. A bare
predicate is a TDZ hazard: ɵɵngDeclareComponent evaluates at
class-definition time, before a later-declared `X` is initialized.

Track `is_forward_ref` on R3QueryMetadata (set at the three
try_unwrap_forward_ref sites: member decorators, signal queries, and
decorator `queries:`/`member_query`), and re-wrap in compile_query via
wrap_forward_ref — upstream's convertFromMaybeForwardRefExpression.
Full-mode emit is untouched: its query calls run at first render, after
class init, so the bare predicate is correct there.

Covers the P1 review finding on #554.
@Brooooooklyn
Brooooooklyn merged commit 8a29746 into main Oct 7, 2026
10 checks passed
@Brooooooklyn
Brooooooklyn deleted the fix/issue-513-partial-component-queries branch October 7, 2026 06:07
Brooooooklyn added a commit that referenced this pull request Oct 7, 2026
…#555)

* fix(compiler): emit queries/viewQueries in ɵɵngDeclareComponent

With compilationMode: 'partial', components lost @ViewChild/
@ContentChild, signal queries, and `queries:` decorator metadata —
the declaration map simply never carried them, so the linker had
nothing to put into contentQueries/viewQuery on ɵɵdefineComponent.

Upstream builds the component definition map on top of the directive
map (partial/component.ts createComponentDefinitionMap), so the fix
reuses the directive emitter's compile_queries_array: extracted view
and content queries are threaded through PartialComponentInputs and
emitted between providers and exportAs, matching upstream field order
(directive.ts:74-80, verified against GOLDEN_PARTIAL.js).

Signal queries bump minVersion to 17.2.0, the same rule and version
the directive path already applies (directive.ts:146-149).

Fixes #513

* fix(compiler): preserve forwardRef on query predicates in partial emit

`forwardRef(() => X)` query predicates were unwrapped at extraction and
emitted bare — for components a new divergence, for directives a
pre-existing one the compile_query comment already documented. A bare
predicate is a TDZ hazard: ɵɵngDeclareComponent evaluates at
class-definition time, before a later-declared `X` is initialized.

Track `is_forward_ref` on R3QueryMetadata (set at the three
try_unwrap_forward_ref sites: member decorators, signal queries, and
decorator `queries:`/`member_query`), and re-wrap in compile_query via
wrap_forward_ref — upstream's convertFromMaybeForwardRefExpression.
Full-mode emit is untouched: its query calls run at first render, after
class init, so the bare predicate is correct there.

Covers the P1 review finding on #554.

* fix(compiler): keep defaults, async, rest in metadata arrow functions

Arrows copied into decorator metadata lost what the output AST can't
express: `FormalParameter.initializer` was never read (defaults dropped),
`arrow.r#async` and `params.rest` were ignored. `(v: string = 'a') => v`
emitted as `(v) => v` — an async factory then returned a value, not a
Promise.

Fall back to `make_raw_source` (types-stripped verbatim) when the arrow
is async, has a rest param, or any param has an initializer — the same
fallback destructured params and non-arrow function expressions already
take.

Also thread `source_text` through `extract_pipe_metadata_in` →
`extract_constructor_deps` → `extract_param_dependency`: the pipe
constructor's `@Inject(...)` arg conversion previously passed `None`, so
the new fallback would have returned `None` there and degraded the dep
to `ɵɵinvalidFactoryDep`.

Fixes #512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(partial): ɵɵngDeclareComponent omits queries / viewQueries

1 participant