feat(service): support @Service decorator (Angular v22+) in AOT#324
feat(service): support @Service decorator (Angular v22+) in AOT#324brandonroberts wants to merge 8 commits into
Conversation
Foundation for the AOT @service decorator handler. Routes Service through the same ɵɵinject token resolution as Injectable — upstream's getInjectFn in r3_factory.ts falls through to inject() for unrecognized targets, and the v22 service runtime expects deps to be resolved via inject() calls in the constructor body rather than the ɵfac. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New src/service/ module mirrors src/injectable/ but slimmer to match the v22 @service decorator's scope: - No providedIn or useClass/Factory/Value/Existing provider variants. - ɵfac is generated with empty constructor deps — upstream's service.ts:178 calls `toFactoryMetadata({...meta, deps: []}, FactoryTarget.Service)`, since the v22 service runtime resolves DI via `inject()` calls in the constructor body rather than the ɵfac. - ɵprov emits `ɵɵdefineService({token, factory, autoProvided?})`. The `autoProvided` entry is only present when the user explicitly disables it, matching upstream's `if (meta.autoProvided === false)` gate. - User-supplied `factory:` from the decorator is re-parsed and wrapped in an arrow `() => factory()`, per service_compiler.ts:35-42. The decorator parser matches by identifier name only; the AOT wiring that follows will mirror the JIT path's import-map gate to ensure a non-Angular `@Service` library export doesn't shadow real decorators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a standalone-@service branch to component/transform.rs alongside the existing standalone-@Injectable branch. A @Service-decorated class produces a static ɵfac (deps-less constructor factory) and a static ɵprov initialized by ɵɵdefineService. Three gates run before emission: 1. Import-map gate. `find_angular_service_decorator` only matches when the local `Service` identifier resolves to `@angular/core`. A bare `@Service()` from a third-party library is left untouched, mirroring the JIT path's behavior. 2. Version gate. Targeting Angular < 22 surfaces a diagnostic and skips emission, since the runtime lacks ɵɵdefineService. Unknown version defaults to "supports" (matches the implicit-standalone pattern). 3. Collision diagnostic. Per upstream service.ts:101-116, @service may not coexist with another @angular/core decorator on the same class — `find_conflicting_angular_decorator` flags the offending sibling. setClassMetadata is emitted for TestBed support and a .d.ts declaration is produced via `generate_service_dts`. The .d.ts shape reuses ɵɵInjectableDeclaration<T> (consistent with upstream service_compiler.ts:55), and the ɵfac ctor-deps tuple is always `never`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Seven integration tests: - aot_service_decorator_basic — bare @service() emits static ɵfac/ɵprov with ɵɵdefineService and no autoProvided entry. - aot_service_decorator_auto_provided_false — explicit autoProvided: false surfaces in both ɵprov and setClassMetadata args. - aot_service_decorator_custom_factory — user-supplied factory is wrapped in an arrow that calls it, replacing the ɵfac delegation. - aot_service_decorator_version_gated — targeting v21 surfaces a diagnostic and emits nothing. - aot_service_decorator_collision_with_injectable — @service alongside @Injectable surfaces the upstream collision diagnostic and emits nothing. - aot_non_angular_service_decorator_is_ignored — a third-party @service is left untouched (no diagnostic, no emission). - dts_service — .d.ts emits ɵfac<T, never> + ɵprov<T> using the InjectableDeclaration type (matches createInjectableType upstream). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cross-checked emission against the upstream ngtsc compliance goldens at packages/compiler-cli/test/compliance/test_cases/service_decorator/. Output for basic, autoProvided:false, custom-factory, and generic cases matches structurally (whitespace and i0/$r3$ namespace alias aside) plus the .d.ts shape. One missing diagnostic surfaced. Upstream service.ts:278-309 surfaces SERVICE_CONSTRUCTOR_DI when an @service class declares its own constructor parameters, because the ɵfac is generated with empty deps — any constructor parameter would silently become `undefined` at runtime. `class_has_own_constructor_params` adds the local check (we skip upstream's base-class walk; that requires cross-file reflection that oxc doesn't perform, and upstream itself skips it in LOCAL mode). Three new integration tests cover this: - aot_service_decorator_constructor_di_diagnostic — own-ctor params trigger the diagnostic and emit nothing. - aot_service_decorator_explicit_auto_provided_true — matches upstream's explicitly_provided_service golden: autoProvided:true is omitted from ɵprov (matches the runtime default) but preserved in setClassMetadata args. - aot_service_decorator_inline_arrow_factory — matches upstream's service_with_factory golden: an inline arrow factory is double-wrapped (outer wrapper calls the user expression). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 021c69dd51
ℹ️ 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".
Address two Codex review findings on the @service AOT branch: 1. Aliased-import gate: the @service classification matched on the local identifier name only, so `import { Injectable as Service } from '@angular/core'` was treated as the v22 decorator. Introduce `is_angular_core_export` to verify the import's original exported name and route both `find_angular_decorator` (JIT) and `find_angular_service_decorator` (AOT) through it. 2. Collision dispatch: the @Service-with-other-Angular-decorator collision check sat inside the standalone-@service branch, which only runs after @Component/@Directive/@Pipe/@NgModule have failed to match. With @service + @component, the Component branch won the dispatch race and silently compiled the class. Hoist the collision check to a pre-flight ahead of the primary-decorator branches and drop the now-unreachable copy inside the Service branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bae2019ff7
ℹ️ 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".
Two follow-ups from Codex's review of bae2019: 1. Aliased extraction asymmetry. The previous commit taught find_angular_service_decorator to accept aliased imports (e.g. `import { Service as NgService }; @NgService()`) via the import map, but extract_service_metadata still re-searched the class's decorators by literal name "Service" and returned None — so the dispatcher would accept the class and the extractor would silently drop it. Pass the already-resolved Decorator through to extract_service_metadata and use service_decorator.span directly for removal, so the dispatcher's gate is the only place the name/alias logic lives. 2. Namespace Service gate. find_angular_decorator's Service-only import-map check covered Identifier callees but fell through for StaticMemberExpression callees, classifying any `@third.Service()` from a third-party namespace as Angular's v22 decorator. Add an is_angular_core_namespace helper and extend the Service gate to verify namespace callees too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01bd489d48
ℹ️ 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".
| let Expression::CallExpression(call) = &decorator.expression else { continue }; | ||
| let Expression::Identifier(id) = &call.callee else { continue }; |
There was a problem hiding this comment.
Recognize namespaced @service in AOT
When Angular is imported as a namespace, e.g. import * as ng from '@angular/core'; @ng.Service(), this helper returns None because it only accepts identifier callees before checking the import map. The AOT path then never enters the service branch, so the class is emitted without the new ɵfac/ɵprov fields (and the decorator is left in place), even though the JIT classifier in this same file explicitly handles StaticMemberExpression for Service.
Useful? React with 👍 / 👎.
| .get(&Ident::from(id.name.as_str())) | ||
| .map(|info| info.source_module.as_str() == "@angular/core") | ||
| .unwrap_or(false), | ||
| Expression::StaticMemberExpression(_) => true, |
There was a problem hiding this comment.
Verify namespaced conflicts before rejecting @service
For an Angular @Service class that also has a non-Angular namespace decorator with an Angular-looking property name, such as @thirdParty.Component(), this treats the namespace decorator as Angular unconditionally and the preflight at transform_angular_file_aot emits the collision diagnostic and skips service compilation. That blocks valid service classes with unrelated decorators; the namespace object should be checked against the import map the same way @ns.Service() is checked.
Useful? React with 👍 / 👎.
Summary
@Servicedecorator in the AOT pipeline. A decorated class now emitsstatic ɵfac(deps-less factory) andstatic ɵprovinitialized byɵɵdefineService, matching upstream'sservice_compiler.ts/compiler-cli/.../service.tsoutput.inject()calls in the constructor body — upstream'sservice.ts:178callstoFactoryMetadata({...meta, deps: []}, FactoryTarget.Service). Constructor params on an@Serviceclass are diagnosed (per upstreamservice.ts:278-318) instead of silently emitted asundefinedat runtime.token,factory(defaults toMyService.ɵfac; a user-suppliedfactory:is wrapped in() => factory()), andautoProvided: falseonly when the user explicitly disables it.autoProvided: trueis omitted (matches the runtime default) but preserved insetClassMetadataargs..d.tsemitsɵfac<T, never>+ɵprov<T>reusingɵɵInjectableDeclaration(upstreamservice_compiler.ts:55usescreateInjectableTypefor both decorators).Closes #310.
Upstream parity audit
Cross-checked against the ngtsc compliance goldens at
packages/compiler-cli/test/compliance/test_cases/service_decorator/:basic_service(@Service())explicitly_provided_service(autoProvided: true)not_provided_service(autoProvided: false)service_with_factory(inline arrow factory)generic_service(.d.ts)Also covered the
service_spec.tsngtsc tests: compile, multi-decorator collision, own-constructor DI. Inherited-constructor DI is not implemented — it requires cross-file reflection that OXC doesn't perform, and upstream itself skips it inCompilationMode.LOCAL.Three gates run before emission
find_angular_service_decoratoronly matches when the localServiceidentifier resolves to@angular/core. A bare@Service()from a third-party DI library is left untouched.implicit_standalonepattern).@Servicecannot coexist with another@angular/coredecorator on the same class (service.ts:101-116).Commit layout
feat(r3): add FactoryTarget::Service and ɵɵdefineService identifierfeat(service): port R3ServiceMetadata and compile_service from upstreamfeat(service): emit ɵfac/ɵprov for @Service classes in AOT pipelinetest(service): cover @Service AOT emission, version gate, and collisionfeat(service): diagnose @Service classes with constructor paramsstyle(service): apply rustfmtTest plan
cargo check --all-featurescargo test— 407 integration tests (10 new), 1052 unit tests (5 new), no regressionscargo fmt --all -- --checkcargo run -p oxc_angular_conformance+git diff --exit-code(1252/1252 passing)pnpm check(oxfmt + oxlint)🤖 Generated with Claude Code