fix(vite): resolve HMR resources from the extractor, not a text scan - #462
Conversation
The `@ng/component` endpoint read a class's own `templateUrl` / `template`
/ `styleUrl(s)` / `styles` by scanning decorator TEXT. The Rust extractor
folds same-file constants and interpolates template literals; the scan
cannot. Every shape it could not read fell back to the FILE-LEVEL union,
which in a multi-component file served a class its siblings' template and
stylesheets.
const DIR = './themes'
@component({ styleUrls: [`${DIR}/a.css`, SHARED_STYLE] })
The endpoint now asks the extractor. `extract_component_metadata_sync`
was already written and correct but carried no `#[napi]`, so it was not
on the JS surface; adding the attribute is the whole Rust change.
That answer is definitive, because it is the SAME one the compiler uses.
`transform.rs:2557` and the extractor both call `extract_component_metadata`
with the same `collect_string_consts` table. The transform's only extra
step, `resolve_styles`, turns URLs into content and never re-derives the
URL list:
compiled styles = metadata.styles ++ concat(content(u) for u in style_urls)
└── inline, first ──┘
Eight fixtures were compared against the real compile and all matched,
including the one that looks like a partial read: `[OK, IMPORTED, './lit.css']`
resolves to two entries in BOTH. The dropped import is genuinely not a
stylesheet this component gets, so serving two is exact, not partial.
A class MISSING from the metadata is equally definitive — measured, the
compiler skips it too, leaving the decorator intact with no `ɵcmp`. So
both fallbacks are deleted rather than narrowed.
This retires the `unreadable` state, which existed only because the text
scan disagreed with Rust. What does NOT change is `readStyles`, its
`complete` flag, and the three-valued `styles` argument from #457/#461:
those guard a resolved file that cannot be READ, which is a filesystem
question and still real.
Three existing assertions flipped, deliberately. `styles: SOME_ARRAY_CONST`
asserted no `styles:` key on the theory that Rust folded the constant.
It does not — OXC folds string-valued consts, not array-valued ones, so
that component's `ɵcmp` never had a `styles` property. `styles: []` there
clears nothing. Verified against the compiler output, not reasoned.
Closes #456
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx
…uses The previous commit moved HMR resource resolution to the extractor, which left the per-class text locators reachable only from their own tests. Gone from the plugin: `extractTemplateUrlFor`, `extractClassStylesFor`, `extractInlineTemplate`, `extractInlineStyles`, and the two caches the last pair existed to fill — `inlineTemplateCache` and `inlineStylesCache`, both of which were written, pruned, refreshed on hot update, and never read by anything, from before this PR. Gone from the scanner: the `*For` locator family, `readStringLiterals`, `StringLiteralsRead`, `ClassStyleFields`, `locateStyleFieldsFor`, `hasUnreadableKey`, `hasInterpolation`. Its exports drop from 15 to 6. `stripComponentMetadata` and its closure stay untouched. It decides full reload versus hot update, so it still parses decorator text and still has to get comments, decoys and escapes right. Most of what looked dead was not. Nine of ten candidate helpers turned out live via `locateStylesInArgs` / `locateTemplateInArgs` -> `locateFieldInsideArgs` -> `findFieldInArgs`; only `hasInterpolation` was genuinely unreachable. `FieldValue` stays as `findFieldInArgs`'s return type, with its `export` dropped. So the tests were re-pointed rather than dropped: of 206, 22 are unchanged, 78 keep their title with the call swapped to a surviving locator, 26 are renamed because the old title named a concept that is gone, and 76 are deleted — the `readStringLiterals` block and the url-locator blocks, which test functions that no longer exist. Two new cases cover strip-path behaviour the salvage exposed: a `]` inside a comment must not close the array early, and an array holding only a comment must still strip to `[]`. Behaviour is proven unchanged beyond the e2e suite: `stripComponentMetadata` was run against both the old and new scanner over 744 generated sources — 62 decorator shapes by 12 file wrappers, including phantom decorators in comments and strings, CRLF, Unicode class names, malformed escapes, spreads and elisions. Zero differences. 343 unit tests pass, e2e stays at 37. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_886d9e28-3c9e-4690-913d-9d8e7651cc5b) |
The endpoint re-parses `resolvedId` from disk, but the component was
compiled from the `code` Vite handed `transform`. Those are different
byte streams whenever another plugin exposes a `load` hook or a
pre-ordered `transform`, and — with no third-party plugin at all —
whenever `fileReplacements` points `actualId` at a different file.
On a disk source the compiler never saw, every `styles` shape the
extractor cannot fold resolves to nothing: an array constant, an imported
one, a `.concat(...)`. Reading that as definitive emitted `styles: []`
and wiped CSS the running component genuinely had — from a template edit
that never touched the styles.
Measured on a real dev server, disk holding `styles: STYLE_ARRAY` with an
upstream `load` expanding it, editing only the external `.html`:
this branch styles: [], <- wipes the compiled style
main (no styles: key) <- CSS survives
So this PR introduced it. It fires on the external-resource branch with
no `.ts` change at all.
The evidence needed was already cached. `componentMetadataCache` holds
the transform-time source with the `template:` / `styles:` VALUES
blanked, and blanking only ever empties a delimited range — so an
expression the strip cannot open survives verbatim, and the two stripped
forms disagree exactly when the two sources disagree outside those
fields. Matching strips is proof that the styles read here are the styles
the component compiled with.
This gates the destructive answer ONLY. Content that WAS read is still
served on a mismatch, which is no worse than main, since main scanned the
same disk source. `merged.length > 0` short-circuits, so the strip runs
only when `[]` is on the table.
Not done here, deliberately: the reviewer suggested caching metadata from
each successful transform. `transform` does not re-run before the
endpoint serves an inline `.ts` edit, so that cache is stale on the most
common path.
The four existing array-constant tests keep clearing, because disk and
transform source are identical there. The gate separates "the compiler
also saw this constant and resolved nothing", where clearing is exact,
from "the compiler saw something else", where it is a guess. A new test
pins the other side: identical sources, external-resource path, still
clears.
347 unit tests pass, e2e stays at 37.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e840fb58-37aa-4c8b-a8e0-1461aebdd277) |
|
Confirmed and fixed in 873d190. You were right, and the trigger is wider than the finding says. Reproduced, and it is this PR's regressionReal dev server, disk holding It fires on the external-resource branch with no It does not need a third-party plugin. This plugin's own One correction to the finding's phrasing: The fix, and why not the one you recommendedI did not cache metadata from each successful transform. The evidence was already cached. The two disagree exactly when the two sources disagree outside those fields. That is the proof needed, and it was simply never consulted on the path where the clearing fires. const styles: string[] | null =
merged.length > 0 || (external.complete && compiledFromThisSource()) ? merged : nullThis gates the destructive answer ONLY. Content that WAS read is still served on a mismatch — no worse than main, which scanned the same disk source. Key alignment checked two ways, since VerificationThree tests red-checked, failing with The four existing array-constant tests keep clearing, because disk and transform source are identical there. The gate separates "the compiler also saw this constant and resolved nothing", where clearing is exact, from "the compiler saw something else", where it is a guess. 343 -> 347 unit tests; e2e stays at 37. Two neighbouring hazards, left aloneBoth measured in the same scenario, both identical on
Worth their own issue rather than a fix folded into this one. |
Closes #456. Two commits: the functional swap, then the deletion it enables.
The problem
The
@ng/componentHMR endpoint read a class's owntemplateUrl/template/styleUrl(s)/stylesby scanning decorator TEXT. The Rust extractor folds same-file constants and interpolates template literals; the text scan cannot. Every shape it could not read fell back to the FILE-LEVEL union, which in a multi-component file served a class its siblings' template and stylesheets.Five review rounds on #455 each patched one syntactic form and revealed the next. This removes the text locators from the resolution path instead of teaching them to fold constants.
Two things the issue got wrong, both found by measurement
1. The metadata was not reachable. The issue assumed
extract_component_metadata_syncwas available. It is fully written and correct but carries no#[napi], so it was not on the JS surface at all. Adding the attribute is the entire Rust change.2. There is nothing to cache. The issue suggested the data might already be in hand at transform time. It is not:
result.styleUpdatesis always empty andtemplateUpdates' values are discarded, only the keys kept. And a probe showedtransformdoes NOT re-run before the endpoint serves an inline edit —transformCallsstays at 1 — so a transform-time cache would have served stale resources.That turned out not to matter. The endpoint already does this per request:
So this is a one-for-one swap of one call for another. No cache, no staleness, no ordering hazard. ~0.016 ms of Rust against the ~0.30 ms async hop it paid before.
Why the per-class answer is definitive
transform.rs:2557and the extractor both callextract_component_metadatawith the samecollect_string_conststable. The transform's only extra step,resolve_styles(transform.rs:4061), turns URLs into content and never re-derives the URL list:Eight fixtures were compared against the real compile. All matched — including the one that looks like a partial read:
Two of three in both. The dropped import genuinely is not a stylesheet this component gets, so serving two is exact, not partial. The same held for spreads,
letbindings, concatenation, and cross-file imports.A class MISSING from the metadata is equally definitive.
decorator.rs:90's?drops a whole class when one key will not resolve — and measured, the real compile skips it too: decorator left intact, noɵfac, noɵcmp, its stylesheet not even independencies. Same forimport { Component as Cmp }, a class nested below top level, and a@Directive.So both fallbacks are deleted rather than narrowed.
What this retires, and what it does not
The
unreadablestate existed only because the text scan disagreed with Rust. With Rust as the source, that disagreement cannot occur.What does NOT change is
readStyles, itscompleteflag, and the three-valuedstylesargument from #457/#461. Those guard a RESOLVED file that cannot be READ — a filesystem question, still real, and still the thing that stops a truncate window during an atomic write from wiping live CSS. That code is byte-identical.Three assertions flipped, deliberately
Three tests used
styles: SOME_ARRAY_CONSTand asserted nostyles:key, on the stated theory that "the Rust extractor folded the CSS out of the constant". It does not. OXC'scollect_string_constsfolds STRING-valued consts;extract_string_valuenever walks an array. Verified against compiler output rather than reasoned:That component compiles with no styles, so
styles: []clears nothing that ever existed. All the #457/#461 read-failure guards are untouched and green.Commit 2: the deletion was far smaller than expected
I predicted ~139 tests would delete and roughly half the scanner would go. Wrong, in the safe direction — nine of ten candidate helpers turned out to be live, all reachable from
locateStylesInArgs/locateTemplateInArgsthroughfindFieldInArgs. OnlyhasInterpolationwas genuinely dead.decorator-fields.tsOf 206 tests: 22 unchanged, 78 keep their title with the call swapped to a surviving locator, 26 renamed because the old title named a concept that is gone, 76 deleted. Comment handling, decoy tokens and Unicode identifiers were all salvaged — the strip path still parses decorator text and still has to get them right.
Also deleted:
inlineTemplateCacheandinlineStylesCache, written and pruned and refreshed on hot update, and read by nothing — true before this PR.stripComponentMetadataand its closure are untouched. It decides full reload versus hot update.Verification
cargo test -p oxc_angular_compilerpnpm testpnpm test:e2eThe six new tests are the const-folded cases the text scan could never read, each with a styled sibling in the same file to prove the contamination is gone. All six fail on
ab79282; red-checked by restoring the old plugin file. One of them pins the invariant the whole design rests on: it runs the plugin's owntransform, slices the emittedɵcmp, and asserts the endpoint serves exactly what the compiler put there. If extractor and compiler ever diverge, that test moves.Commit 2's behaviour is proven beyond e2e:
stripComponentMetadatawas run against both the old and new scanner over 744 generated sources — 62 decorator shapes by 12 file wrappers, including phantom decorators in comments and strings, CRLF, Unicode class names, malformed escapes, spreads and elisions. Zero differences.Notes for the reviewer
styles: CONST_ARRAY. ngtsc's partial evaluator would fold it; OXC does not. The endpoint is now exactly faithful to OXC, which is the right invariant — the browser runs OXC-compiled code. Making OXC match ngtsc is a separate extractor change, and those three tests would move again.styleUrlplusstyleUrlstogether is order-dependent (decorator.rs:115assigns for the array form and pushes for the scalar). The compile reproduces it exactly, so it is faithful and out of scope here.e2e/compare/src/discovery/finder.ts:93carries a TODO waiting for exactly this binding, and hand-rolls a TypeScript-based extractor as a fallback. Untouched — a separate follow-up.findFieldInArgs'sshorthandMeansparameter is now never passed a non-default value, making one branch unreachable. Left in place, since it is still referenced from a live function.Note
Medium Risk
Changes dev-server HMR resolution and style-update semantics; behavior is heavily tested but wrong per-class updates would show up only in multi-component or non-literal decorator setups.
Overview
Fixes per-class HMR (#456) by having the
@ng/componentendpoint load each class’stemplateUrl/template/styleUrls/stylesfromextractComponentMetadataSync(same Rust path as compile) instead of scanning decorator text and falling back to the file-level URL union—which could serve a sibling’s template or CSS in multi-component files.The Rust fn is now
#[napi]-exported; the plugin dropsinlineTemplateCache/inlineStylesCache, file-level fallbacks, and the old text-based extractors.decorator-fieldsis trimmed to whatstripComponentMetadatastill needs for HMR vs full-reload; URL/style literal parsing helpers are removed.Style clearing only treats an empty merged list as definitive when filesystem reads are complete and disk source matches the transform-time stripped cache—so disk/transform mismatches (array consts, imports, etc.) omit
stylesinstead of wiping live CSS.Tests and e2e comments are updated; large
decorator-fieldsandhmr-hot-updatecoverage adds const-folded / sibling-contamination cases.Reviewed by Cursor Bugbot for commit 873d190. Bugbot is set up for automated code reviews on this repo. Configure here.