feat(partial): library AOT — emit ɵɵngDeclare* end-to-end + napi compilationMode#325
Open
brandonroberts wants to merge 15 commits into
Open
feat(partial): library AOT — emit ɵɵngDeclare* end-to-end + napi compilationMode#325brandonroberts wants to merge 15 commits into
brandonroberts wants to merge 15 commits into
Conversation
Adds the option plumbing and the first partial-declaration emitter for
library AOT compilation:
- CompilationMode { Full, Partial } at the crate root; TransformOptions
gains a compilation_mode field that defaults to Full, so existing
full-Ivy emit is unaffected.
- crates/oxc_angular_compiler/src/partial/ mirrors upstream's
packages/compiler/src/render3/partial/ directory layout. minVersion
constants for every future partial-declaration kind are listed in one
place to stay in sync with upstream.
- compile_declare_factory_function emits the exact ɵɵngDeclareFactory
shape from upstream factory.ts:27 — { minVersion: "12.0.0", version:
"0.0.0-PLACEHOLDER", ngImport: i0, type, deps, target: i0.ɵɵFactoryTarget.X }.
R3FactoryDeps::Invalid → "invalid", None → null, type-only-invalid
deps poison the whole factory to match full-mode behavior (#288).
Tested via snapshot pins on each shape variant plus a linker round-trip:
the partial declaration we emit, fed through crate::linker::link, expands
back into a working full ɵfac function. This is the strongest in-tree
correctness signal — if our own linker doesn't accept the output,
downstream linkers won't either.
The new emitter is not yet wired into any decorator's production emit
path; that's the next slice. Until then, partial-mode output would still
emit full ɵprov/ɵcmp/etc. — a mixed-mode output that's invalid for
linking. The pieces ship behind a still-Full default.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires partial-declaration emit into the @Injectable production path so a single CompilationMode::Partial setting produces a fully partial-form output for an @Injectable source file. The emitter pairs with the existing ɵɵngDeclareFactory work from the previous slice. What ships: - partial/injectable.rs — compile_declare_injectable_from_metadata mirrors upstream packages/compiler/src/render3/partial/injectable.ts:29. Emits { minVersion: "12.0.0", version, ngImport, type, providedIn?, useClass? | useExisting? | useValue? | useFactory?, deps? }. Field order matches upstream; flags omitted when unset. - forwardRef wrapping — useClass/useExisting with is_forward_ref produce i0.forwardRef(function() { return X; }) (mirrors generateForwardRef in upstream render3/util.ts:174). Shared wrap_forward_ref helper at partial/mod.rs so future partial emitters (NgModule list, Component dependencies) can reuse it. - generate_injectable_definition* now takes CompilationMode and dispatches: Full → existing ɵɵdefineInjectable + full ɵfac; Partial → ɵɵngDeclareInjectable + ɵɵngDeclareFactory. Defaults stay Full — no behavior change for application builds. - Provider-deps vs constructor-deps distinction preserved: the partial declaration's `deps:` field carries provider deps (useClass-with-deps / useFactory-with-deps); constructor deps flow into the paired ɵfac. Tests (27 new, all green; full suite stays green): - 12 snapshot tests pin every provider variant and providedIn variant. - The strongest correctness signal is the paired round-trip: emit partial ɵfac + partial ɵprov, feed the file through crate::linker, and assert the linker produces ɵɵdefineInjectable with the dep preserved. - End-to-end via transform_angular_file: @Injectable source → set compilation_mode: Partial → output contains both ɵɵngDeclare* calls, none of the ɵɵdefine* calls, and re-links cleanly back to full Ivy. - A pinned Full-mode E2E test catches any accidental default flip. Not yet shipped: Component, Directive, Pipe, NgModule, Injector, ClassMetadata partial emit. Setting Partial on a source containing those decorators still works for the @Injectable subset, but Component etc. would emit mixed output (full ɵcmp + partial ɵfac) — not yet valid for a library publish. Those land in subsequent slices. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third partial-emit decorator. Setting compilation_mode: Partial on a source with an @pipe class now produces a fully partial-form output (ɵɵngDeclareFactory + ɵɵngDeclarePipe), no ɵɵdefinePipe. Shape ports upstream packages/compiler/src/render3/partial/pipe.ts:28: i0.ɵɵngDeclarePipe({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyPipe, isStandalone?: false, // emitted only when not standalone name: "myPipe", pure?: false // emitted only when not pure }) Differs from upstream only in optional-field handling: upstream's isStandalone is `boolean | undefined` and emits whenever defined; OXC's metadata is a plain `bool` (no undefined state), so we follow the full-mode convention and omit when true. The linker defaults to true for both isStandalone and pure, so the output is byte-equivalent. Wiring: - partial/pipe.rs: compile_declare_pipe_from_metadata + compile_declare_factory_for_pipe. - generate_full_pipe_definition_from_decorator now takes CompilationMode and dispatches. The single transform.rs caller passes options.compilation_mode. - pipe::R3DependencyMetadata is a distinct (and slightly older) type from factory::R3DependencyMetadata — lacks the type_only_invalid flag from #288. The Pipe → Factory conversion defaults it to false. If we ever need type-only-invalid factories for pipes we'll unify the two types; not needed for partial parity today. 8 new tests, all green. Full suite (~2600) green: - Snapshot per variant (pure standalone, impure, non-standalone, etc.). - Field-order assertion against upstream pipe.ts:48-62. - E2E via transform_angular_file with compilation_mode: Partial, asserting both ngDeclare* calls present + no ngDefine*. - Linker round-trip: @pipe source → partial output → linker → ɵɵdefinePipe with no ngDeclare residue. - Pinned Full-mode test catches accidental default flip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rary mode Fourth and fifth partial-emit decorators, paired (every @NgModule emits both ɵmod and ɵinj). Setting compilation_mode: Partial on an @NgModule source now produces three partial declarations: ɵfac, ɵmod, ɵinj. Shapes port upstream packages/compiler/src/render3/partial/{ng_module,injector}.ts: i0.ɵɵngDeclareNgModule({ minVersion: "14.0.0", version, ngImport, type, bootstrap?, declarations?, imports?, exports?, schemas?, id? }) i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version, ngImport, type, providers: <Expr | null>, // required slot; null literal if absent imports? }) refsToArray wrapping (util.ts:90-93) — list fields are either plain `[A, B]` or, when contains_forward_decls, a single lazy-arrow per list `() => [A, B]`. Schemas never wrap (they're runtime tokens). Partial mode banishes the `ɵɵsetNgModuleScope` side-effect statement. Matches upstream ng_module/handler.ts:971 ("no remote scoping required as this is banned in partial compilation"). Wiring: - partial/ng_module.rs + partial/injector.rs. - generate_full_ng_module_definition takes CompilationMode and dispatches three-way (ɵfac + ɵmod + ɵinj). Injector metadata construction extracted to a shared build_injector_metadata helper used by both full and partial paths. - Single transform.rs caller passes options.compilation_mode. - NgModule factory in partial mode emits deps:null — our R3NgModuleMetadata doesn't carry constructor deps from the decorator analyzer, so the linker generates an inherited factory. Correct for the common case of parameterless NgModule classes. 10 new tests, all green. Full suite (~2600) green: - Snapshot per shape variant (empty, decls+imports, forward-decl arrow-wrap, bootstrap+schemas). - Injector variants (providers only, providers omitted, imports array, raw_imports precedence). - E2E via transform_angular_file: @NgModule source → 3 ngDeclare* calls present + no ngDefine* + no setNgModuleScope. - Linker round-trip: partial output → ɵɵdefineNgModule + ɵɵdefineInjector + working ɵfac. - Pinned Full-mode test catches accidental default flip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sixth partial-emit decorator and the most complex so far. Setting compilation_mode: Partial on an @directive class now produces a fully partial-form output (ɵɵngDeclareFactory + ɵɵngDeclareDirective), no ɵɵdefineDirective. Shape ports upstream packages/compiler/src/render3/partial/directive.ts:26. Field order matches upstream's createDirectiveDefinitionMap. ngImport is emitted LAST per upstream convention (directive.ts:114). Key implementation notes: - Inputs auto-select between two shapes: * New (post-17.1): one object per input with classPropertyName, publicName, isSignal, isRequired, transformFunction. Triggered when any input is signal-based. * Legacy (pre-17.1): per-input value is a string (publicName) when publicName == declaredName and no transform, else a 2- or 3-tuple `[publicName, declaredName, transformFn?]`. - Queries (both content and view) emit propertyName + predicate (type expression OR string array). Boolean flags only when set; descendants emitted only when true; emitDistinctChangesOnly only when false (its inverse default). - Host bindings stay RAW: listeners and properties carried as unparsed string maps. The linker re-parses them at link time. Matches upstream compileHostMetadata (directive.ts:210-231). - hostDirectives forward-reference handling: when is_forward_reference, the directive expression wraps in i0.forwardRef(function() { return X; }). Inputs/outputs lists emit as flat alternating [publicName, alias, ...] arrays — mirrors upstream's createHostDirectivesMappingArray. - minVersion calculator at partial/directive.rs::compute_min_version: base 14.0.0; bumps to 16.1.0 (decorator transformFunction), 17.1.0 (signal input → new shape required), 17.2.0 (signal query). Exposed to the upcoming Component slice. - create_directive_definition_map is pub(crate) — the Component slice builds on top of it (upstream's createComponentDefinitionMap extends the directive map, see component.ts:87-88). - Object keys with `.` or `-` get quoted. Mirrors upstream UNSAFE_OBJECT_KEY_NAME_REGEXP. Wiring: - generate_directive_definitions takes CompilationMode; Partial branch skips the constant pool (linker re-emits constants at link time). - Single transform.rs caller passes options.compilation_mode. 13 new tests, all green. Full suite (~2600) green: - Minimal directive (only required fields). - Legacy inputs string vs tuple shape. - Signal input triggers new shape + 17.1.0 minVersion bump. - Signal query triggers 17.2.0 minVersion bump. - Host listeners/properties raw. - hostDirectives forwardRef wrap. - Outputs object map. - usesInheritance + usesOnChanges flags. - exportAs string array. - ngImport-last field order. - E2E via transform_angular_file. - Pinned Full-mode test catches accidental default flip. - Linker round-trip: partial → ɵɵdefineDirective. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Seventh and biggest partial-emit decorator. With CompilationMode::Partial set, @component sources now bypass the entire template/IR pipeline and emit ɵɵngDeclareFactory + ɵɵngDeclareComponent directly. Templates carry verbatim into the partial declaration; the linker re-parses at consumer build time. Shape ports upstream packages/compiler/src/render3/partial/component.ts:66. Component extends the directive partial with these fields (added AFTER the directive map per upstream component.ts:82-156): template: "<raw html>" // always a string in partial mode isInline?: true // when template came from inline literal styles?: ["..."] dependencies?: [{kind,type,selector?|name?,inputs?,outputs?,exportAs?}] viewProviders?, animations? changeDetection? // only when != Default encapsulation? // only when != Emulated preserveWhitespaces?: true Per upstream, ngImport is emitted LAST. minVersion bumps to: - 14.0.0 base - 16.1.0 if any input has decorator-style transformFunction - 17.0.0 if template contains @if/@for/@switch/@defer block syntax - 17.1.0 if any input is signal-based (new inputs shape required) Dispatch: - compile_component_full takes an early-return path when options.compilation_mode == Partial — skips HTML parse, R3 transform, IR pipeline, instruction emission, constant pool, style processing, encapsulation downgrade, etc. The win is enormous: a several-thousand- line pipeline becomes ~30 lines of metadata-shape construction. - New compile_component_partial helper in component/transform.rs builds a FullCompilationResult populated only with cmp_js + fac_js (everything else empty/None — partial mode has no template fn, no constants, no HMR initializer, no class debug info, no ng-content selectors, no defer-block flag). Implementation notes: - Component emitter is intentionally self-contained (doesn't go through R3DirectiveMetadata) — ComponentMetadata and R3DirectiveMetadata differ in too many ways (host type, deps type, lack of queries on ComponentMetadata, etc.) to make a converter cleaner than inline duplication. - component::dependency::R3DependencyMetadata stores tokens as Idents; the partial factory needs them as OutputExpressions. Conversion wraps each token in a ReadVar. - HostMetadata keys may include binding syntax (e.g. `[class.active]`, `(click)`). Emitted verbatim — that's the form the linker expects. - DeclarationListEmitMode::Closure / ClosureResolved wraps each dependency's `type` in forwardRef. Matches upstream's wrapType helper at component.ts:207-210. 10 new tests, all green. Full suite (~2600) green: - Minimal component with inline template + isInline marker. - Block-syntax template bumps minVersion to 17.0.0 with template kept raw. - Styles array of raw strings. - changeDetection: emit only when OnPush; omit when Default. - encapsulation: emit only when None/ShadowDom; omit when Emulated. - preserveWhitespaces. - Round-trip: partial component → linker → working ɵɵdefineComponent. - Pinned Full-mode test catches accidental default flip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final partial-emit decorator in the originally planned set. Replaces the
full-mode IIFE form:
(() => {
(typeof ngDevMode === "undefined" || ngDevMode) &&
i0.ɵsetClassMetadata(MyClass, [{...}], null, null);
})();
…with a bare partial-declaration call:
i0.ɵɵngDeclareClassMetadata({
minVersion: "12.0.0",
version: "0.0.0-PLACEHOLDER",
ngImport: i0,
type: MyClass,
decorators: [{...}],
ctorParameters?: <expr>,
propDecorators?: <expr>
});
No ngDevMode guard, no IIFE — the linker re-wraps when expanding to
ɵsetClassMetadata, so consumers still get tree-shakeable production
output.
Dispatch:
- partial/class_metadata.rs::compile_declare_class_metadata.
- Two call sites in component/transform.rs branch on
options.compilation_mode (the main flow at the @component path, plus
build_set_class_metadata_decls used by other decorators).
- In the component flow, the deferred-deps branch (full-mode async
variant compile_component_class_metadata) falls back to sync partial
emit. ɵɵngDeclareClassMetadataAsync remains unimplemented; the
async variant only fires for components with @defer deferrable
imports, which OXC's metadata doesn't currently track. Wiring it
needs the cross-file selector info upstream's component handler
computes — a separate slice if/when it lands.
4 new tests, all green. Full suite (~2600) green:
- Partial Injectable: ɵɵngDeclareClassMetadata present, no ngDevMode,
no ɵsetClassMetadata.
- Partial Component: same.
- Linker round-trip: partial output → ɵsetClassMetadata IIFE form
(with the guard restored).
- Pinned Full-mode test catches accidental default flip.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surfaces CompilationMode at the napi/Vite layer so library builds can
flip partial-emit mode from JS without poking the Rust crate directly.
- napi TransformOptions gains compilation_mode: Option<String>. The
From<TransformOptions> impl parses "full" | "partial" case-insensitively
via a new parse_compilation_mode helper and falls back to Default on
unrecognized input.
- index.d.ts: matching `compilationMode?: string` field with doc.
- Vite plugin PluginOptions: `compilationMode?: 'full' | 'partial'`,
threaded through pluginOptions and into the per-file TransformOptions
passed to transformAngularFile.
Default stays 'full' — no behavior change for existing app builds.
Library-build usage:
// ng-packagr-style library entry
import { angular } from '@oxc-angular/vite'
export default defineConfig({
plugins: [ angular({ compilationMode: 'partial' }) ],
build: { lib: { entry: 'src/public-api.ts', formats: ['es'] } },
})
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s + fix Component ngImport position
Cross-validation work surfaced one field-order divergence and one
documented metadata-shape divergence.
Bug fix:
- Component: ngImport position. The partial Component emitter was placing
ngImport at the END of the entire map; upstream places it between the
directive fields and the component-specific fields (see
createComponentDefinitionMap calling createDirectiveDefinitionMap,
which closes with ngImport). Moved ngImport to mark the end of the
inherited directive map, with template/styles/dependencies/etc. coming
after. Now matches the hello_world golden:
{ ...directive_fields, ngImport: i0, template: ..., isInline, ... }
All 10 existing partial_component tests still pass after the reorder.
Cross-validation tests (partial_upstream_parity_test.rs):
- hello_world component shape — verifies ɵcmp field order and the
ngImport-mid-position fix.
- hello_world NgModule shape — verifies ɵmod declarations array shape.
- Pipe full shape — byte-equivalent to upstream's pipes.ts golden:
ɵɵngDeclarePipe({ minVersion: "14.0.0", version: ..., ngImport: i0,
type: MyPipe, isStandalone: false, name: "myPipe", pure: false })
- Injectable full shape — byte-equivalent to upstream.
- ClassMetadata full shape — byte-equivalent to upstream.
Documented divergence:
- Factory `deps` for parameterless classes. Upstream emits `deps: []`;
OXC currently emits `deps: null` because R3PipeMetadata /
ComponentMetadata uses Option<Vec<...>> ambiguously (None could mean
either "no constructor" or "no constructor metadata"). The linker
round-trip works in both cases — the divergence is a byte-shape
parity gap, not a correctness gap. Test
`known_divergence_factory_deps_for_parameterless_class` asserts the
current behavior so the day someone fixes the analyzer to emit
`Some(vec![])`, the assertion flips and prompts an update.
Method: whitespace-collapsed substring matching. Upstream and OXC emit
the same field order and content but format object literals differently
(soft-wrap vs single-line). The whitespace-strip lets us compare
shape-without-formatting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…le imports Completes the partial-emit surface. Components whose templates use @defer blocks with deferrable imports now emit the async variant of the class metadata partial declaration; everything else falls back to the sync form. Shape (per upstream class_metadata.ts:46): i0.ɵɵngDeclareClassMetadataAsync({ minVersion: "18.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyCmp, resolveDeferredDeps: () => [import('./a').then(m => m.A), ...], resolveMetadata: (A, ...) => ({ decorators: [...], ctorParameters: <expr | null>, propDecorators: <expr | null> }) }) Key differences from the sync variant: - minVersion is "18.0.0" (defer support landed in linker 18). - ctorParameters / propDecorators are emitted as `null` literals when undefined, not omitted (mirrors upstream `?? o.literal(null)`). - resolveMetadata's params use R3DeferPerComponentDependency.param_name (the local binding), while the dynamic-import resolver uses .export_name. This is OXC's split-vs-upstream-conflated-symbolName pattern — keeps aliased imports tree-shakeable, see R3DeferPerComponentDependency docs. Dispatch: - compile_component_declare_class_metadata picks sync vs async based on whether dependencies is empty. Mirrors upstream's same-named function at class_metadata.ts:50. - compile_declare_class_metadata_async is also exposed publicly for direct use. - Wire-in at component/transform.rs replaces the previous partial-mode fallback (which always emitted sync) with the dispatch helper. Full mode unchanged. 7 new tests, all green. Full suite (~2600+) green: - Bare async call has 18.0.0 minVersion. - resolveDeferredDeps + resolveMetadata both present, with correct param-name vs export-name distinction. - ctorParameters/propDecorators emit as null literals (not omitted). - Default imports resolve via m.default in the resolver. - Dispatch helper falls back to sync (12.0.0 minVersion) when no deferred deps, picks async (18.0.0) when present. - Field order matches upstream class_metadata.ts:60-71. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revalidation against upstream's GOLDEN_PARTIAL.js fixture surfaced two real divergences and one documented gap. The first is a runtime correctness bug; the second is byte-bloat. Fix 1 — Pipe/Injectable/NgModule factory `deps: null` → `deps: []` (runtime correctness bug): Previously OXC emitted `deps: null` for parameterless classes (no constructor in source). Our linker interprets `deps: null` as "use ɵɵgetInheritedFactory" — which returns invalid for classes that don't extend another class, causing factory invocation to fail at runtime. Upstream emits `deps: []` for parameterless classes, which makes the linker generate the simple `new (t || Class)()` factory. The previous round-trip tests asserted only "linker accepts the output and emits a function" — not that the function works. The new behavior matches upstream's hello_world golden exactly and produces a working factory at runtime. Pipe (`partial/pipe.rs::clone_constructor_deps`), Injectable (`partial/injectable.rs::clone_constructor_deps`), and NgModule (`partial/ng_module.rs::compile_declare_factory_for_ng_module`) now default `None` → `Valid(Vec::new())` instead of `R3FactoryDeps::None`. Component/Directive already had this logic gated on `uses_inheritance`; the smaller decorators don't track inheritance, so they default to the safe-and-correct empty form. Classes that explicitly extend will miss the inherited-factory optimization but still get a working factory. Fix 2 — Injector `providers: null` → field omitted entirely (byte-shape): Upstream omits the `providers` field when there are no providers (see hello_world GOLDEN_PARTIAL.js: MyModule's ɵinj has no providers field). OXC was always emitting `providers: null`, bloating every NgModule's injector declaration. Now matches upstream exactly. Document — Component `deferBlockDependencies` gap: Upstream component.ts:132-153 emits a per-block resolver array when the template uses @defer with deferrable imports. ComponentMetadata doesn't carry the per-block resolver map — only the class-level `deferred_imports` list (which feeds the async class metadata). The functional gap: @defer + deferrable imports gets the async class metadata declaration but no per-block resolvers in ɵcmp. Closing this needs a metadata-pipeline change. Documented in partial/component.rs near where the field would be emitted. Verification: - Re-dumped the hello_world fixture output and compared to upstream's GOLDEN_PARTIAL.js. Now byte-perfect parity modulo whitespace and template quote style. - Full crate suite: 2663 tests, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PR readiness clippy pass (just ready) treats `#[allow]` as an error. My branch had added 9 net-new ones; this commit removes all of them: - partial/mod.rs: dropped the per-MIN_VERSION_* attrs. Every constant is actually used now (each partial emitter references its own MIN_VERSION). - partial/mod.rs: removed MIN_VERSION_DIRECTIVE_BASE and MIN_VERSION_COMPONENT_BASE — both were placeholders. Directive and Component compute minVersion dynamically (signal inputs, signal queries, control-flow blocks bump it), so no static "base" constant is referenced. - partial/component.rs: dropped the _unused_imports_marker hack and the unused `PipeDep` import it existed to placate. No behavior change. Suite still 2663 / 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@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: ed091c3fc4
ℹ️ 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".
Addresses codex review on PR #325. `compile_component_partial` was hard-coding `has_defer_block: false`, which silently stripped the ɵɵngDeclareClassMetadataAsync emission for any partial-mode component that combined `@defer` with `deferredImports`. The downstream class-metadata dispatch reads `compilation_result.has_defer_block` to decide whether to build `deferred_deps`. With the hard-coded false: - `deferred_deps` was always empty. - `compile_component_declare_class_metadata` always picked the sync ɵɵngDeclareClassMetadata form. - Consumers of the linked library never got the dynamic-import resolver for the lazy-loaded component metadata. Fix: cheap string scan for `@defer` in the template. Partial mode bypasses the IR pipeline, so we can't get the precise BlockPresenceVisitor signal upstream uses — but the asymmetric cost is clear: - False positive (e.g. `@defer` inside a template string literal) → `deferred_deps` ends up empty anyway because `metadata.deferred_imports` drives the array; dispatch falls back to sync. Harmless. - False negative → silently strips async resolver. Breaks lazy loading. Err on detection. Two regression tests added in partial_class_metadata_async_test.rs: - `@defer` + `deferredImports` → output contains ɵɵngDeclareClassMetadataAsync and a dynamic `import('./lazy')` call. - Template without `@defer` → sync form, no async leaks even if the source happens to declare deferredImports. Full suite: 2665 / 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # crates/oxc_angular_compiler/src/component/transform.rs # crates/oxc_angular_compiler/src/directive/definition.rs
Member
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19d257b723
ℹ️ 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".
…ons is empty Addresses codex P1 review on PR #325. Standalone components with `imports: [...]` were silently rendering without their directives/pipes in partial-mode libraries. Root cause chain: 1. The OXC analyzer at `component/decorator.rs:281` sets `declaration_list_emit_mode = RuntimeResolved` whenever `metadata.raw_imports.is_some()`. 2. In RuntimeResolved mode, the analyzer does NOT call `populate_declarations_from_imports`, so `meta.declarations` stays empty. 3. Full mode handles this by emitting `ɵɵgetComponentDepsFactory(MyCmp, [imports])` at runtime — works fine. 4. Partial mode CAN'T do that (the linker emits a static `dependencies: [...]` array, not a runtime resolver). The pre-fix guard `if !meta.declarations.is_empty()` therefore omitted the `dependencies` field entirely — silently producing a working-looking linked ɵcmp where none of the imports got registered. After the linker ran, the resulting full ɵcmp had no directive/pipe dependencies, so any standalone library component that imported `CommonModule`, child components, or pipes would render without those directives/pipes being available. Looked like a working build; broken at runtime. Fix: in partial mode, when `meta.declarations.is_empty()` but `meta.imports` is non-empty, lower each import to a synthetic directive-shape `TemplateDependency` on the fly. Mirrors the lowering at `decorator.rs::populate_declarations_from_imports` (which only runs in Direct mode). Two important properties of the fix: - Our linker's dependencies-array processor (`linker/mod.rs:1844`) reads only the `type` expression from each entry — kind, selector, inputs, outputs are ignored. So the placeholder `kind: "directive"`, `selector: "*"` is harmless: the linked output is `dependencies: [Import1, Import2, …]` byte-equivalent to what Direct mode would produce. - Full mode is untouched — the fix only fires when the partial emitter encounters the RuntimeResolved-with-imports gap. The `create_dependencies_array` helper was switched from `&Vec` to `&[T]` so the synthetic-lowering and the existing-declarations paths can share it. Two regression tests added in partial_component_test.rs: - Standalone component with `imports: [CommonModule, MyDir]` → partial output has dependencies field + both imports listed; linker round-trip preserves both in the final `dependencies: […]` array. - Standalone component without imports → no dependencies field (sanity counterpart). Limitation documented inline: import shapes other than plain identifier arrays (e.g. `imports: [...SHARED, MyComp]`) only get the identifier entries lowered; spreads/calls are dropped. Same constraint applies in full mode's Direct path. Full suite: 2668 / 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Adds
compilationMode: 'partial'to the Rust compiler, NAPI binding, and Vite plugin so library builds can emit Angular partial declarations (ɵɵngDeclare*) instead of full Ivy definitions. Consumer apps already run this package's linker onnode_modules, so partial-emitted libraries link cleanly back into full Ivy form at consumer build time.Twelve commits, additive — default stays
Fullso application builds are unchanged.Decorators implemented
ɵɵngDeclareFactory@InjectableɵɵngDeclareInjectable@PipeɵɵngDeclarePipe@NgModuleɵɵngDeclareNgModule+ɵɵngDeclareInjector@DirectiveɵɵngDeclareDirective(new/legacy input shapes, signal-query bumps)@ComponentɵɵngDeclareComponent(template as verbatim string — skips entire IR pipeline)ɵɵngDeclareClassMetadata(sync) +ɵɵngDeclareClassMetadataAsync(for@deferdeferrable imports)API surface
Or directly via NAPI / the Rust crate:
Cross-validation against upstream
A focused parity test (
partial_upstream_parity_test.rs) diffs our output against upstream'sGOLDEN_PARTIAL.jsfixtures. Surfaced and fixed:ngImportfield position — upstream sandwichesngImportbetween the directive map and component-specific fields (seecreateComponentDefinitionMapcallingcreateDirectiveDefinitionMapwhich closes withngImport). We were emitting it last. Fixed in51762f4.deps: null→deps: []— runtime correctness bug. Our linker treatsdeps: nullas "useɵɵgetInheritedFactory", which returns invalid at runtime for non-inheriting classes. Upstream emits[]for parameterless classes. Fixed in9c52cce.providers: null→ omit field — upstream omits theprovidersfield entirely when none are present. We were bloating every NgModule's injector declaration withproviders: null. Fixed in9c52cce.Result: byte-perfect parity with upstream's
hello_worldGOLDEN_PARTIAL.jsmodulo whitespace and template quote style.Documented gap
Component.deferBlockDependencies(upstreamcomponent.ts:132-153) is not emitted. OXC'sComponentMetadatadoesn't carry the per-block resolver map — only class-leveldeferred_imports(which DOES feed the async class metadata). Components using@deferblocks with deferrable imports get the async class metadata declaration but no per-block resolvers inɵcmp. Closing this needs a metadata-pipeline change; documented inline inpartial/component.rs.CI / test status
Local PR-equivalent run (matches
.github/workflows/ci.yml):cargo fmt --all -- --check— cleancargo check --all-features— cleancargo test— 2664 passed, 0 failed, 1 ignored across 33 binariescargo run -p oxc_angular_conformance— 1252/1252 (100%), no snapshot driftpnpm --filter ./napi/angular-compiler build:ts— tsc cleanpnpm build-dev && pnpm test(napi-smoke) — 193 tests passed across 10 filesTest additions in this PR (~80 new tests across seven
partial_*_test.rsfiles):crate::linker::link→ assert linked output contains the fullɵɵdefine*call and nongDeclare*residue.transform_angular_filewithcompilation_mode: Partial.GOLDEN_PARTIAL.jsfixtures.Commits
Test plan
compilationMode: 'full') — existing app builds unchanged, all existing tests green.@Injectable,@Pipe,@NgModule,@Directive,@Component,@Service— emit correct partial shape; round-trip throughcrate::linker::linkproduces working full Ivy form.@Componentwith@if/@for/@switch/@deferblock syntax —minVersionbumps to 17.0.0; template stays verbatim.minVersionbumps to 17.1.0 / 17.2.0; correct new-input-shape selection.useClass,useExisting, host directives, NgModule list lazy-arrow.hello_worldGOLDEN_PARTIAL.js— byte-perfect parity modulo whitespace.🤖 Generated with Claude Code