fix(vite-plugin-angular): @Service decorator support and conformance coverage#2338
Merged
Merged
Conversation
…compiler
Angular 22 introduces the `@Service` decorator as a lighter-weight
alternative to `@Injectable`. The fast compiler recognised only the five
classic decorators, so a `@Service` class fell through uncompiled — no
`ɵprov`/`ɵfac` emitted.
Wire it through the AOT path:
- add `Service` to `ANGULAR_DECORATORS` (it self-registers via `ɵprov`,
so it stays out of `COMPILABLE_DECORATORS`, same as `@Injectable`)
- emit `ɵprov` via `compileService` / `compileDeclareServiceFromMetadata`,
gated on `angularVersionAtLeast(22)` since the compiler API and the
decorator only exist on Angular 22+
- parse the `autoProvided` and `factory` decorator options in metadata
- keep `@Service` on the class in the JIT transform, like `@Injectable` —
its runtime decorator self-registers `ɵprov`/`ɵfac` lazily
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover the four @service shapes the fast compiler now handles: a bare service, `autoProvided: false`, an explicit `factory`, and constructor dependency resolution. The suite is gated on `angularVersionAtLeast(22)` so it skips cleanly on the workspace-pinned Angular 21 and runs on the compiler-compat `next` matrix slot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gory
Wire Angular's `service_decorator` compliance fixtures into the
conformance sweep. The category is gated through `CATEGORY_MIN_MAJOR`
so it runs only when the installed `@angular/compiler` is 22+, where
`@Service` exists; on older peers it is skipped rather than failed.
Also extend `expectEmit` with a cosmetic-insensitive per-call fallback
reusing the existing `aggressiveNorm`. Previously that normalization
ran only on the ellipsis path, so the ellipsis-free `service_decorator`
fixtures failed purely on object-literal spacing (`{token: …}` vs
`{ token: … }`) despite semantically identical output.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gories Add a drift detector to the conformance suite: it enumerates every directory under Angular's compliance `test_cases` and asserts each is consciously triaged into either `CATEGORIES` (run) or the new `UNSUPPORTED_CATEGORIES` allowlist (skipped, with a reason). When a future Angular release ships compiler fixtures for a new feature — a new decorator, a new template construct — the directory lands in neither list and this test fails, forcing a maintainer to either implement support or record a deliberate skip. Without it, new Angular surface is silently ignored: the suite only ever tests what it already knows about. This runs across the conformance matrix, including the `next` (Angular prerelease) slot, so the warning lands early. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ft detection Add `@Service` to the supported-decorators table and describe the conformance category drift detector under Conformance Testing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fast compiler delegates template expression compilation to `@angular/compiler`, so template arrow functions (`@let`, `$event`, host bindings, pipes, nested contexts) already compile correctly. Wire the `r3_view_compiler_arrow_functions` category into the conformance sweep — all 20 fixtures pass — and drop it from UNSUPPORTED_CATEGORIES. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`providers` and `viewProviders` on `@Component`/`@Directive` are parsed in metadata and forwarded into the component/directive definition, so `@angular/compiler` already emits the `ɵɵProvidersFeature`. Wire the `r3_view_compiler_providers` category into the conformance sweep — all 4 fixtures pass — and drop it from UNSUPPORTED_CATEGORIES. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signal-based queries (`viewChild`/`contentChild` and their plural and required variants) are extracted by `detectSignals` and emitted as `R3QueryReference`s, so the directive/component definitions already match. Wire the `signal_queries` category into the conformance sweep — all 3 fixtures pass — and drop it from UNSUPPORTED_CATEGORIES. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire `r3_view_compiler_i18n` into the conformance sweep. Unlike the flat categories, Angular splits its i18n fixtures across 11 subdirectories (ICU logic, nested nodes, namespaces, blocks, …), each with its own TEST_CASES.json — a flat read of the category's top-level file would miss all 87 nested cases. Teach the runner to descend into subdirectories, opt-in per category via `NESTED_CATEGORIES` so the existing flat categories are unaffected. Test cases now carry the directory they were discovered in, and nested cases are labelled with their subpath to keep descriptions unique. Also harden the runner against fixtures that omit `expectations` or `inputFiles` (e.g. the `icu_and_i18n` case), which the i18n fixtures are the first to exercise. i18n template compilation is handled by `@angular/compiler`, so the fast compiler already emits correct `$localize`/`ɵɵi18n` output: +30 conformance checks pass with zero new failures. Four external-template fixtures in `line_ending_normalization` record tolerated compile errors — the runner compiles `.ts` directly without the resource inliner, so `templateUrl` stays unresolved (a pre-existing harness limitation, not i18n-specific). Other wired categories also have nested fixture folders that remain unswept; sweeping those is tracked as separate follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…egories The subdirectory-recursion machinery added for i18n is now applied to the other six wired categories that also nest fixtures into subfolders: `r3_view_compiler_bindings`, `_styling`, `_di`, `_directives`, `r3_view_compiler`, and `r3_compiler_compliance` — 38 nested TEST_CASES.json files that the flat top-level read was silently missing. This surfaces +103 passing conformance checks. It also surfaces 11 new soft-failures (`value_composition`, `di`, `directives/matching` + `host_directives`, `template_variables`) — codegen-formatting differences of the same class as the pre-existing soft-failures, now tracked rather than invisible. Overall pass rate 91.3%, comfortably above the 0.75 gate; no hard failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
the
scope:vite-plugin-angular
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Angular 22+ ChangesService Decorator Implementation and Testing
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/compiler/metadata.ts (1)
320-328: 💤 Low valueConsider storing only explicit
falseforautoProvided.The comment states "only an explicit
falseis meaningful" sinceautoProvideddefaults totruein Angular. The current implementation stores bothtrueandfalse, but compile.ts only checksmeta.autoProvided === falsebefore forwarding it. Storingtrueis redundant.For clarity, consider:
case 'autoProvided': - meta.autoProvided = valText === 'true'; + if (valText === 'false') { + meta.autoProvided = false; + } break;This makes the intent explicit: only preserve the non-default value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vite-plugin-angular/src/lib/compiler/metadata.ts` around lines 320 - 328, The code currently sets meta.autoProvided = valText === 'true' in the 'autoProvided' case and thus stores both true and false; change it to only record the explicit non-default value by setting meta.autoProvided = false when valText === 'false' (leave it undefined for true/default) so that meta.autoProvided only exists when explicitly disabling autoProvided; update the 'autoProvided' switch case handling in metadata.ts (and ensure compileService/compile.ts continues to check meta.autoProvided === false) to reflect this intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vite-plugin-angular/src/lib/compiler/compile.ts`:
- Around line 975-1001: When encountering a 'Service' decorator on Angular <22
the code falls through leaving targetType as FactoryTarget.Injectable and later
unconditionally emits an Injectable factory; change the handling so Service only
sets targetType to FactoryTarget.Service when angularVersionAtLeast(22) is true
and otherwise prevents factory emission for that class (either by
continuing/returning early or marking a flag to skip factory generation).
Concretely, update the 'Service' case around
FactoryTarget/compileService/compileDeclareServiceFromMetadata to not fall
through on older Angular peers (do not leave targetType set to Injectable), and
update the factory-generation logic (the block that emits the factory into
ivyCode) to check the resolved targetType or the new skip flag before calling
factory emit so a Service on <22 will not produce an incorrect Injectable
factory. Ensure references to FactoryTarget, targetType, compileService,
compileDeclareServiceFromMetadata, and ivyCode are used to locate and adjust the
logic.
In `@packages/vite-plugin-angular/src/lib/compiler/conformance.spec.ts`:
- Around line 423-448: The drift detector test "has no untriaged Angular
compliance categories" is failing because the on-disk category "isolated" is not
listed in either CATEGORIES or UNSUPPORTED_CATEGORIES; fix by adding "isolated"
to the CATEGORIES array if the compiler should run those fixtures, or add an
entry in UNSUPPORTED_CATEGORIES with a clear reason string (so
Object.keys(UNSUPPORTED_CATEGORIES) will include it), then re-run the test to
ensure the untriaged set is empty.
---
Nitpick comments:
In `@packages/vite-plugin-angular/src/lib/compiler/metadata.ts`:
- Around line 320-328: The code currently sets meta.autoProvided = valText ===
'true' in the 'autoProvided' case and thus stores both true and false; change it
to only record the explicit non-default value by setting meta.autoProvided =
false when valText === 'false' (leave it undefined for true/default) so that
meta.autoProvided only exists when explicitly disabling autoProvided; update the
'autoProvided' switch case handling in metadata.ts (and ensure
compileService/compile.ts continues to check meta.autoProvided === false) to
reflect this intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f58a5597-6c00-44f8-b366-475da6ac8f40
📒 Files selected for processing (7)
packages/vite-plugin-angular/src/lib/compiler/COMPILER.mdpackages/vite-plugin-angular/src/lib/compiler/compile.tspackages/vite-plugin-angular/src/lib/compiler/conformance.spec.tspackages/vite-plugin-angular/src/lib/compiler/constants.tspackages/vite-plugin-angular/src/lib/compiler/jit-transform.tspackages/vite-plugin-angular/src/lib/compiler/metadata.tspackages/vite-plugin-angular/src/lib/compiler/service.spec.ts
…tegory The drift detector flagged `isolated` on the Angular 21 / latest conformance jobs: it exists in Angular 21.x's compliance fixtures but not in 22/main, so it was never triaged when the allowlist was built against `main`. Working as designed — a category in neither list fails the suite. `isolated` tests Angular's source-to-source "isolated" transform mode; its expected outputs are transformed `.ts` and `.ngtypecheck.ts` shims, not Ivy JS. That is out of scope for the fast compiler (which emits Ivy JS only), so it joins `source_mapping` in UNSUPPORTED_CATEGORIES. Verified by running the conformance suite against downloaded Angular 21.2.13 fixtures — drift detector green, 538 passed / 19 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses CodeRabbit review feedback. When a `@Service` class was compiled against Angular <22, the version gate did a bare `break`, leaving `targetType` at its `FactoryTarget.Injectable` default — the unconditional factory block then emitted an `Injectable`-target `ɵfac` with no `ɵprov`, a silently wrong half-compiled class. `@Service` does not exist before Angular 22, so this is a broken configuration. Set `classCompileError` instead, surfacing a clear `@Service requires Angular 22 or later` error before factory emission rather than degrading silently. 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
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.
PR Checklist
Adds Angular 22
@Servicedecorator support to thevite-plugin-angularfast compiler and expands its Angular conformance-suite coverage.Closes #
Affected scope
vite-plugin-angularRecommended merge strategy for maintainer [optional]
Commit preservation note [optional]
The 10 commits are deliberately focused — one
fixfor the@Servicecompiler change, one commit per conformance category wired in, the drift-detector test, and a docs commit. Squashing collapses independently-reviewable, independently-bisectable units (each category can be reverted on its own) into one opaque change. Rebase preserves those boundaries.What is the new behavior?
@Servicedecorator — the fast compiler now compiles Angular 22's@Servicedecorator; previously a@Serviceclass fell through uncompiled, emitting noɵprov/ɵfac. EmitsɵprovviacompileService, parsesautoProvided/factory, gated on Angular 22+.arrow_functions,providers,signal_queries,i18n, andservice_decoratorcategories.TEST_CASES.jsonfiles across 7 categories that the flat top-level read was missing.expectEmitgained a cosmetic-insensitive per-call fallback (tolerates object-literal whitespace differences).COMPILER.mdupdated for@Serviceand category drift detection.Net conformance: +133 passing checks, pass rate ~91%, no hard failures.
Test plan
Ran
pnpm exec vitest run packages/vite-plugin-angular/src/lib/compiler/against both the workspace-pinned Angular 21 and Angular 22 (@angular/compiler@nextviapnpm.overrides):Angular 21 — 23 test files pass;
@Servicepaths skip as version-gated.Angular 22 — 24 test files pass; all
@Serviceunit and conformance cases green.prettier --checkon all changed files.nx format:checkpnpm buildpnpm testDoes this PR introduce a breaking change?
Other information
source_mappingremains the soleUNSUPPORTED_CATEGORIESentry — template source-map emission is deferred (theJSEmitterwould need to become source-map-aware, and the conformance harness would need a map-decoding check).