Repository navigation
fix(compiler): ngtsc parity batch — 10 issues (#546, #538, #519, #517, #515, #514, #516, #521, #520, #547) - #548
Merged
Merged
Conversation
ngtsc's `reflectClassMember` reports a TS `accessor` field as a PropertyDeclaration, so `filterToMembersWithDecorator` collects a `@HostListener` on one (typescript.ts, used by directive/src/shared.ts:693). OXC's `extract_host_listeners_in` only matched `MethodDefinition` and `PropertyDefinition`, so an `AccessorProperty` member was silently dropped. It now matches `ClassElement::AccessorProperty` too, mirroring `extract_host_bindings_in` which already handled it. Statics stay excluded via `r#static`, per `filterToMembersWithDecorator`'s `!member.isStatic` filter (#544). Fixes #546
ngtsc normalizes `selector: ''` to the default selector (`resolved === '' ? defaultSelector : resolved` in annotations/directive/src/shared.ts). For @component the default is 'ng-component', identical to a missing selector. OXC kept '' as an explicit selector, producing `selectors: []` in ɵcmp and `""` in the .d.ts ComponentDeclaration type parameter — and the runtime reads `componentDef.selectors[0][0]` when creating a component without a host element. Normalize the empty selector to absent at metadata extraction so every emit path (ɵcmp selectors, partial ɵɵngDeclareComponent, .d.ts) applies the same 'ng-component' fallback used for a missing selector. @directive is unchanged: upstream resolves '' to the `null` default, which raises NG2004 — not the component fallback. Directive selector diagnostics are tracked separately. Fixes #514
The NgModule ɵfac path matched the Injectable one for the flag decorators (@Optional/@Self/@SkipSelf/@host) and @Attribute, but never matched 'Inject', so an @Inject(TOKEN) ctor param fell back to the type annotation (tokenless type -> ɵɵinvalidFactoryDep -> NG0202 at runtime; class type -> injects the class instead of the token). Extract the decorator's argument like injectable/decorator.rs does, under any alias ('import {Inject as Inj}') or 'ng.Inject', since angular_param_decorator resolves the imported name. The partial-declaration path also hardcoded 'deps: []' for every NgModule factory; it now carries the extracted deps so the linker can build the factory. Unskips ctor-mod-named/-alias/-ns in the ngtsc ctor-param fixture. Fixes #519
Like ngtsc's WrappedNodeExpr, a query predicate that evaluates to a reference or a value that can't be statically evaluated is emitted verbatim, so @ViewChild(class Foo {}), new ViewChild(class Foo {}) in queries: metadata, and viewChild(class Foo {}) now emit the class expression in place instead of erroring with "predicate cannot be interpreted" or dropping the query. Fixes #516
…angular/core import alone ngtsc's `isAngularDecorator` checks `decorator.import.from === '@angular/core'` with no name check. The JIT `ctorParameters` extraction and `setClassMetadata`'s `ctorParameters`/`propDecorators` filters still name-checked against the DI/member decorator lists, so `@angular/core` decorators whose names aren't listed (`@Component()` or any custom name on a constructor parameter or member) were dropped — or, in JIT, wrongly kept as `__param(index, dec)` decorators. All three paths now filter with `is_angular_core_decorator` when the file's imports are known (`Some(consts)`), matching `isAngularDecorator` in `downlevel_decorators_transform.ts` and `metadata.ts`. The `consts: None` fallbacks keep the known-name lists: without the file's imports OXC can't prove provenance, so a bare name-match is all that's safe. Foreign decorators on ctor params still lower to `__param` in JIT and stay dropped in AOT metadata as before. Fixes #538
An identifier this file doesn't declare or import (a lib global like `window`, `atob` or `document`, or a name declared nowhere) was treated as dynamic wherever it appeared, so `transform: window ? atob : btoa` errored with "Input transform must be a function". ngtsc resolves such a name to a `Reference` to its declaration, which is truthy like any reference, and picks the branch: the transform compiles to `atob`. `RefKind::Ambient` now stands as that truthy reference where only truthiness or the name matters (`?:`, `&&`, `||` and unary operators) while staying dynamic where a concrete value would be read from it (member access, calls, spreads), which is also what ngtsc's `accessHelper` and call path do for a reference to a declaration in another file. Fixes #517
ɵsetClassMetadata's ctorParameters callback names ctor-param decorators
({type: Optional}), @Inject(TOKEN) tokens (args: [TOKEN]) and member
decorators ({type: Input}) as bare identifiers, but import elision
dropped them unconditionally. TestBed.overrideComponent and friends then
throw ReferenceError when they invoke ctorParameters (issue #520).
Pass whether metadata will be emitted (emit_class_metadata &&
!advanced_optimizations) into ImportElisionAnalyzer::analyze and skip the
ctor-param-decorator/@Inject-arg/declare-prop elision set in that case —
matching ngtsc, which keeps these imports. Elision still applies when no
setClassMetadata is emitted.
Fixes #520
…adata Two @component decorators on one class: ngtsc gives no diagnostic — it compiles the first, leaves the second on the class (downleveled by TS into __decorate), and lists BOTH in ɵsetClassMetadata's decorators array, since extractClassMetadata filters class decorators on isAngularDecorator (imported from @angular/core), not on which one the component handler took. OXC already compiled the first decorator and kept the second's text on the emitted class, but passed only the compiled decorator to build_decorator_metadata_array, so setClassMetadata listed just one entry. - transform_angular_file (@component branch) and build_set_class_metadata_decls (@Directive/@Pipe/@Injectable/ @NgModule/@service) now collect every class decorator that is_angular_core_decorator accepts, in source order — the same filter extractClassMetadata uses — instead of a single primary decorator. - build_decorator_metadata_array takes the compiled @component decorator so the resource-inlining gate mirrors transformDecoratorResources: when the compiled decorator's config references external resources, the args of EVERY decorator literally named `Component` are replaced by the transformed map (upstream `{...dec, args: [literal]}`); the unresolvable template-literal field drop now also applies to the verbatim args of duplicate @component decorators. Fixes #521
ngtsc's extractDirectiveMetadata rejects inputs, outputs and queries on static members with INCORRECTLY_DECLARED_ON_STATIC_MEMBER (shared.ts parseInputFields/parseOutputFields/parseQueriesOfClassFields), while @HostBinding/@HostListener on statics are ignored (filterToMembersWithDecorator drops static members upstream, so they never reach the io/query parsers either). decorator_io_errors now mirrors those checks for @Component/@directive — and @pipe, whose upstream handler runs the same extraction: - `Input "x" is incorrectly declared as static member of "D".` for an @input decorator or an input()/model() initializer on a static member, after the decorator's own checks, like upstream. - `Output is incorrectly declared on a static class member.` for an @output decorator (methods included, which upstream also accepts) or an output()/outputFromObservable()/model() initializer on a static member, on the decorator or the call. tryParseInitializerBasedOutput's rejection of output.required() is mirrored too, before the static check. - `Query is incorrectly declared on a static class member.` for a @ViewChild/@ViewChildren/@ContentChild/@ContentChildren decorator or a viewChild()/viewChildren()/contentChild()/contentChildren() initializer on a static member, and the signal-query errors that come first upstream ('No locator specified.', 'Argument needs to be an object literal.', and the @ViewChild + signal query collision). Fixes #547
`@Component({jit: true})`, `@Directive({jit: true})` and
`@NgModule({jit: true})` opt the class out of AOT compilation. ngtsc
returns jitForced from metadata extraction, produces no analysis (no
ɵcmp/ɵdir/ɵmod/ɵfac/ɵsetClassMetadata) and registers the class in
jitDeclarationRegistry so the JIT application transform downlevels its
decorators.
The AOT path now detects the jit-forced primary decorator and lowers the
class exactly like the JIT mode pipeline: decorators are collected into a
tslib __decorate() call (templateUrl/styleUrl(s) rewritten to
angular:jit:file; imports), constructor parameters become static
ctorParameters, member decorators become static propDecorators or
__decorate() member calls, and the class is restructured as a class
expression with a trailing re-export. The per-class collection/emission
is shared with transform_angular_file_jit via collect_jit_class_info and
jit_class_edits.
Details handled to match ngtsc + TypeScript emit:
- A co-located @Injectable still compiles (upstream's InjectableHandler
is independent of the jitForced short-circuit): ɵfac/ɵprov statics and
the injectable setClassMetadata/d.ts survive.
- @Pipe/@Injectable/@service have no upstream jit opt-out and are
unaffected; dispatch order (Component→Directive→Pipe→NgModule) is
respected so a jit property on a decorator that doesn't own the class
can't skip compilation.
- Import elision preserves symbols that only appear in constructor
parameters of jit-forced classes, since they are re-emitted inside
ctorParameters/__param — the same result TypeScript's elision produces
on ngtsc's rewritten AST.
- tslib __decorate/__param imports are only added when the file doesn't
already import them, and import * as i0 is only emitted when the
jit-forced class actually references the @angular/core namespace
(synthesized signal-API propDecorators or injectable statics).
Fixes #515
…to fix/issue-batch-1
… into fix/issue-batch-1
…into fix/issue-batch-1
…nto fix/issue-batch-1
…nto fix/issue-batch-1
…mports' into fix/issue-batch-1
…t' into fix/issue-batch-1
…ators' into fix/issue-batch-1
…sor' into fix/issue-batch-1 # Conflicts: # crates/oxc_angular_compiler/src/directive/property_decorators.rs # crates/oxc_angular_compiler/tests/integration_test.rs
…nostics' into fix/issue-batch-1 # Conflicts: # crates/oxc_angular_compiler/tests/integration_test.rs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
…consts in napi metadata - output_members used value? which skipped the static-member check for members without an initializer (bare @output() static fields and all decorated methods). Gate the .required() check on Some(value) instead. - napi compileClassMetadata missed the new StringConsts arg on build_decorator_metadata_array; pass the already-collected consts.
…t presence, ctorParameters null - PipeDecoratorHandler never runs extractDirectiveMetadata upstream: drop @pipe from angular_decorator_config so io/query checks and metadata fields don't apply to pipes. - Query decorator on a non-property member (method/ctor) reports 'Query decorator must go on a property-type member' before the static check; get/set accessors stay property-type. - accessor fields reflect as property members upstream: treat them in the view/content query extractors too. - jit forced on property presence (directive.has('jit')), not value. - ctorParameters emits bare null for params with no type/decorators.
Member
Author
|
Review round disposition (two adversarial review passes over the merged diff, each verified against the vendored ngtsc sources): Fixed (
Deferred (pre-existing or out of scope, recorded for follow-up):
|
This was referenced Oct 6, 2026
ngtsc emits type: Inject in setClassMetadata while TypeScript elision still drops the Inject import — our fix intentionally keeps it so the emitted code resolves. Extend KnownDifference with importDiffs entries and forgive documented import diffs in the fixture runner.
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
Batch of ten ngtsc-parity fixes, one commit per issue, merged onto
mainhere.accessorfields is silently dropped #546 —@HostListeneron TSaccessorfields now emitsɵɵlistener;static accessorstays excluded via the!member.isStaticfilter (property_decorators.rsmatchesAccessorProperty).ctorParameters/setClassMetadatadecorators are gated on the@angular/coreimport alone (ngtscisAngularDecorator), not on a name list, when string consts are available.ɵfacignores@Inject(TOKEN)on@NgModuleconstructor parameters #519 —@Injecton@NgModuleconstructor params now contributes inject tokens; partialdeclareNgModuleandɵɵInjectorDefdeps carry the real deps instead of[].@Input({transform: window ? atob : btoa})/atob || btoareports a false "must be a function" #517 — Ambient globals (declare var window) evaluate as truthyReferences in?:/||/&&/!like the ngtsc evaluator;??stays dynamic.jit: truein@Component/@Directive/@NgModule(skip AOT compile) #515 —jit: truein@Component/@Directive/@NgModulemetadata opts the class out of AOT (jitForced) and receives the whole JIT downleveling; ctor-param imports are preserved.selector: ''should fall back tong-componentlike a missing selector #514 —selector: ''on@Componentfalls back tong-componentupstream-style; directive''is kept verbatim pending NG2004.@ViewChild(class Foo {})) is rejected or dropped #516 — Query predicates that evaluate toReference/DynamicValue(e.g. class expressions) emit verbatim viaWrappedNodeExprinstead of erroring.@Componenton one class — ngtsc compiles the first with no error; OXC's HMR text scan pairs the second #521 — Duplicate/co-located@angular/coreclass decorators all appear insetClassMetadata(extractClassMetadataparity); the first@Componentstill drives AOT compilation.setClassMetadatanamesInject/Optional/ tokens whose imports OXC removed (ReferenceError under TestBed overrides) #520 — Import elision keeps identifiers referenced bysetClassMetadata(ctor-param decorators,@Attribute,declareprops) when class metadata is emitted.INCORRECTLY_DECLARED_ON_STATIC_MEMBER-style diagnostics for inputs/outputs/queries (incl.output.required()and signal-query parses) on static members across@Component/@Directive/@Pipe.Fixes #546, fixes #538, fixes #519, fixes #517, fixes #515, fixes #514, fixes #516, fixes #521, fixes #520, fixes #547
Merge notes
Two textual conflicts resolved (both append-only test blocks, kept both sides), plus one semantic fix: the jit-forced
build_set_class_metadata_declscall site was adapted to the #521 signature that scans@angular/coredecorators internally.Test plan
cargo test -p oxc_angular_compiler— 41 test binaries, all green.cargo fmt -p oxc_angular_compiler— clean.setClassMetadatanamesInject/Optional/ tokens whose imports OXC removed (ReferenceError under TestBed overrides) #520).