fix(vite): stop requesting a full reload on component template HMR - #444
fix(vite): stop requesting a full reload on component template HMR#444Brooooooklyn wants to merge 4 commits into
Conversation
) Editing a component `templateUrl` `.html` put a `full-reload` on the HMR socket on top of the `angular:component-update` that already hot-swapped the template. `handleHotUpdate` returned `ctx.modules` for the template, and that array is empty: the file was watched but never a module. Vite full-reloads any changed `.html` whose module list is empty or holds no `js` module. Vite's client usually drops that payload, because an `.html` path only reloads when it matches `location.pathname`. That guard is gone when the path is `*`, which is what Vite sends in `middlewareMode` — so the page reloaded right after the update was applied. Reproduced at the reporter's versions (0.0.35 / vite 8.2.1 / angular 21.2.21): `middlewareMode` reloads, plain SPA does not, `.css` never does. Either way the terminal printed `page reload …` and cleared the screen on every save. Register each template with `addWatchFile`, so it enters the module graph as a `js` module with the component `.ts` as its importer, and return it as its own HMR boundary. The browser never imported the template, so the resulting `js-update` is a client no-op; the DOM change still comes from `angular:component-update`. Templates only. An import edge on a style makes Vite propagate the change up to the component `.ts` and re-execute it, which defines a duplicate class (NG0912) and leaves `angular:component-update` patching a class that is no longer mounted — styles then go stale from the second edit onward. Styles never needed it: Vite only force-reloads on `.html`. The e2e sentinel tests could not catch this. They assert the browser did not act on a reload, which stayed true while the server kept asking for one. Added `captureWirePayloads`, which records the HMR socket across navigations, and two tests: a template edit sends no `full-reload`, and `index.html` still does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUvBViRkyuDvqUMkuT5XnH
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_6491c4da-d714-4760-8ecf-0f808478dec6) |
![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: ccaa2dd7e1
ℹ️ 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".
| if (TEMPLATE_REGEX.test(normalizedDep)) { | ||
| this.addWatchFile(dep) |
There was a problem hiding this comment.
When two component files reference the same external template, each transform adds this watch-file edge, but resourceToComponent.set() retains only the last component file. An edit therefore dispatches angular:component-update only for that last owner, while the new self-accepting template boundary suppresses the reload or propagation that would refresh the other component; any mounted component from the earlier owner keeps rendering the stale template. Track template owners as a set and dispatch the update to each owner before suppressing Vite's reload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified on a real Angular app (21.2.21 / vite 8.2.1), 11 runs across four configurations. Not acting on this here — the defect is real but pre-existing, and the causal mechanism described does not hold.
Two components sharing one templateUrl, editing it:
| main (0.0.37) | this PR | |
|---|---|---|
plain SPA / |
only one updates, no reload | only one updates, no reload |
| middlewareMode | only one updates, page does reload | only one updates, no reload |
Never both, never neither — this PR changes no cell.
The claim is that the self-accepting boundary suppresses a reload that would have refreshed the other component. Measured on main in middlewareMode, the reload fires and the loser is still stale:
full-reload path="*" -> page reloaded
after reload: alpha=MARKER_V1_EDITED beta=MARKER_V0 <- still stale
Vite re-serves the loser's .ts from its transform cache with the old template inlined. The reload never rescued anything.
The PR actually improves this case. addWatchFile gives the template an importer edge to both owners, so both .ts modules get invalidated. A user-initiated refresh now rescues the stale component, where on main it stayed stale until its .ts was touched or the server restarted.
Root cause is resourceToComponent being single-valued, documented at index.ts:264-267 — untouched by this PR. Worth its own issue, with templateComponentOwners: Map<string, Set<string>> mirroring the existing styleComponentOwners.
| if (handled && TEMPLATE_REGEX.test(normalizedFile)) { | ||
| for (const mod of ctx.modules) { | ||
| if (isModuleForFile(mod, normalizedFile)) markModuleSelfAccepting(mod) |
There was a problem hiding this comment.
If a component template is also imported by application code through an HTML variant such as ./template.html?raw, ctx.modules contains that variant and its file is still the normalized template path. This predicate consequently marks the genuinely browser-imported variant self-accepting too; Vite then stops propagation at a generated module that has no import.meta.hot.accept handler, so importers retain the old raw value. Only the synthetic no-query module created by addWatchFile should be marked as the boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b52bc85. Thank you — this was a real regression introduced by this PR.
Verified the whole chain: _ensureEntryFromUrl sets mod.file = cleanUrl(resolvedId) (node.js:34844), and cleanUrl strips [?#], so ./app.html?raw is filed under the template's path. Measured in a plain Vite app with no plugin at all:
id = <abs>/src/tpl.html?raw
url = /src/tpl.html?raw
file = <abs>/src/tpl.html <- query-stripped
Both nodes appear in ctx.modules, and the old mod.file ?? mod.id predicate marked both. Measured consequence on an Angular app whose non-component module imports the same file as ?raw:
before raw value after a template edit: MARKER_V0 (stale, consumer never re-evaluated)
after raw value after a template edit: MARKER_V1_EDITED
Fixed by matching on id instead of file. An exact match means same file with no postfix, so it also covers #fragment, which an includes('?') check would miss. Checked that no other query form reaches the bare node: ?t=/?import are stripped by removeImportQuery(removeTimestampQuery(...)) before resolution, and ?v= is dep-optimizer only.
#443 stays fixed — no full-reload in SPA or middlewareMode when nothing imports the template. With a ?raw importer present the page reloads again, which is Vite's native semantics for a chain with no accept boundary, and is what keeps the raw importer correct. Regression test added.
`{ id, _clientModule: { isSelfAccepting: false } }` is not assignable to
`Partial<ModuleNode>` — `_clientModule` is an `EnvironmentModuleNode`.
CI's Lint step failed on it, before the e2e suite ran.
Extract `createMockTemplateModule`, which casts once and hands back the
client node so the assertion stays readable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUvBViRkyuDvqUMkuT5XnH
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_e4ee3073-b717-4d85-9270-aa76cbbb3763) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 045049e3b9
ℹ️ 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".
| // that is no longer mounted. Styles never needed it — Vite only | ||
| // force-reloads on `.html`. | ||
| if (TEMPLATE_REGEX.test(normalizedDep)) { | ||
| this.addWatchFile(dep) |
There was a problem hiding this comment.
When liveReload: false, this still adds every external template as an importer dependency even though the generated component has HMR disabled and handleHotUpdate immediately returns. A template edit therefore enters Vite's default propagation through the component .ts; in a typical Angular app with no accepting boundary, Vite issues a full reload, contradicting the option's documented purpose of disabling both HMR and reloads. Gate addWatchFile on pluginOptions.liveReload so disabled applications retain the previous behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 07ed3ec. Thank you — a real regression from this PR, and the suggested one-line gate is exactly right.
Measured at liveReload: false, editing a component template:
| build | mode | payload | navs | reloaded |
|---|---|---|---|---|
| main | SPA / |
full-reload path=/src/app/app.html |
0 | no |
| this PR | SPA / |
full-reload path="*" |
1 | yes |
| gated | SPA / |
full-reload path=/src/app/app.html |
0 | no |
| main | middlewareMode | full-reload path="*" |
1 | yes |
| gated | middlewareMode | full-reload path="*" |
1 | yes |
The gate restores main's payloads string-identically, and liveReload: true is unaffected: only angular:component-update + a no-op js-update, no full-reload, 0 navigations, in both modes.
The reasoning also holds on principle. The graph edge exists solely to keep Vite off its .html reload branch during HMR. With liveReload: false, handleHotUpdate returns on its first line, so the edge has no consumer inside the plugin — it only feeds Vite's default propagation, which is the bug. Unit tests added for both states.
One thing surfaced while verifying, which I am deliberately not changing here: at liveReload: false the reload was already useless on main — refetching the component .ts after a template edit returns the old template, because every resourceCache.delete and invalidateModule call sits inside handleHotUpdate past that early return. So main does not honour the documented "disables both HMR and reloads" contract in middlewareMode either. Pre-existing, and a product call rather than something to settle inside a bug fix.
Vite sets `mod.file` to `cleanUrl(mod.id)`, so a template that application code also imports as `./tpl.html?raw` is filed under the same path and lands in `ctx.modules` next to the node `addWatchFile` created. Matching on `file` marked that browser-imported variant as a boundary too, and Vite then stopped propagation at a module with no `import.meta.hot.accept` handler — its importers kept the stale raw string. Match on `id` instead. An exact match means same file, no postfix, so it covers `#fragment` as well as `?query`. Measured on an Angular app whose non-component module imports the same file as `./app.html?raw`: before raw value after a template edit: MARKER_V0 (stale) after raw value after a template edit: MARKER_V1_EDITED #443 stays fixed — no `full-reload` in SPA or middlewareMode when nothing imports the template. With a `?raw` importer present the page reloads again, which is Vite's native semantics for a chain with no accept boundary, and is what keeps the raw importer correct. Reported by Codex review on #444. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUvBViRkyuDvqUMkuT5XnH
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_f22e4b49-4658-4a4d-b1b0-8fc58e66f071) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b52bc850e5
ℹ️ 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".
| const clientModule = (mod as { _clientModule?: { isSelfAccepting?: boolean } })._clientModule | ||
| if (clientModule) { | ||
| clientModule.isSelfAccepting = true |
There was a problem hiding this comment.
When a component later changes or removes its templateUrl, the pruning logic deletes the reverse mapping but this mutation remains on the existing Vite module node. A subsequent edit to that now-unowned HTML file passes through the normal branch with handled === false, yet Vite still sees the stale isSelfAccepting flag and stops propagation instead of performing its normal HTML reload, so the edit can be ignored. Reset this flag when an old template dependency is pruned, or avoid storing permanent acceptance state on the shared module node.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Mechanism confirmed, but not acting on this — the consequence is unobservable, and the suggested fix is a net regression.
The stale flag is real. vite:prune fires and the importer edge is dropped, but the node stays in fileToModulesMap, so ctx.modules is non-empty and the flag is genuinely read:
S2 edit (owned) inMap=true type=js isSelfAccepting=TRUE importers=[app.component.ts]
S3 switch to b inMap=true type=js isSelfAccepting=TRUE importers=[] <- stale + orphaned
The claim has one factual error, though: "stops propagation instead of performing its normal HTML reload". Clearing the flag does not restore Vite's HTML reload. The node's type is js, so isClientHtmlChange is false; propagation dead-ends on the empty importer set and Vite sends full-reload path="*" — an unconditional reload in both modes. The droppable .html-path payload requires no js node, i.e. main. It is the addWatchFile hunk that sets the payload shape, not markModuleSelfAccepting.
Measured, editing an orphaned a.html:
| build | mode | payload | reloads? |
|---|---|---|---|
| main | SPA | full-reload /src/app/a.html |
no — client drops it |
| main | middlewareMode | full-reload path="*" |
yes, but DOM byte-identical before/after |
| this PR | both | js-update /src/app/a.html |
no |
| suggested fix | SPA | full-reload path="*" |
yes — worse than main |
So the fix converts a payload the SPA client dropped into an unconditional reload, which is the same regression class the sibling liveReload finding just had me eliminate in 07ed3ec. Restoring main's exact payload would require removing the node, and there is no public API for that — moduleGraph.onFileDelete only detaches importedModules.
Two harm probes, both negative: a shared template re-registers during the reload the .ts change triggers (DOM matches hard-reload ground truth), and a ?raw co-consumer is a separate node whose flag is untouched, so it still propagates and stays fresh.
Net: no user-visible difference in SPA, and in middlewareMode the PR removes a spurious reload of a file nothing references. Leaving as is.
With `liveReload: false`, `handleHotUpdate` returns on its first line, so the graph edge has no consumer inside the plugin. It still fed Vite's default propagation, which found no accepting boundary and issued a full reload — turning a payload the client used to drop into an unconditional one with path `*`. Measured on an Angular app at `liveReload: false`, editing a template: build mode payload navs main SPA full-reload path=/src/app/app.html 0 PR SPA full-reload path=* 1 <- regression gated SPA full-reload path=/src/app/app.html 0 main MW full-reload path=* 1 gated MW full-reload path=* 1 The gate restores main's payloads exactly, and `liveReload: true` is unaffected — no `full-reload` in either mode, template still self-accepting. Also collapse the duplicated plugin lookup in the test file into `getAngularPlugin(options)` rather than add two more copies. Reported by Codex review on #444. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUvBViRkyuDvqUMkuT5XnH
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_2dd1fb91-c7d4-4420-aaa0-bc7b6138f735) |
|
Nice! I tried to fix it myself but failed 😅. I can confirm this branch solves the issue i expereinced. |
2 participants
Fixes #443.
The bug
Editing a component
templateUrl.htmlputs afull-reloadon the HMR socket, on top of theangular:component-updatethat already hot-swapped the template.Whether the browser obeys depends on the payload path. Vite's client only path-matches when the path ends in
.html; a*path skips that test and reloads at once.//src/app/app.htmlbase: '/app/'/src/app/app.htmlserver.middlewareMode: true"*""*".cssstyleUrl editReproduced at the reporter's versions —
@oxc-angular/vite@0.0.35,vite@8.2.1,@angular/*@21.2.21. Console order in the failing case:Even where the browser ignores the payload, Vite prints
page reload src/app/app.htmland clears the terminal on every save.The fix
Register each template with
addWatchFile, so it enters the module graph as ajsmodule with the component.tsas its importer, and return it as its own HMR boundary. Both of Vite's.htmlreload branches are then unreachable.The browser never imported the template, so Vite's client finds no
hotModulesMapentry and the resultingjs-updateis a no-op there. The DOM change still comes entirely fromangular:component-update. Verified:moduleEvalCount: 0, no console errors, and the browser never requests the.html.Templates only. An import edge on a style makes Vite propagate the change up to the component
.tsand re-execute it, which defines a duplicate class (NG0912) and leavesangular:component-updatepatching a class that is no longer mounted. Styles then go stale from the second edit onward — this showed up as three red e2e tests while developing. Styles never needed the edge: Vite only force-reloads on.html.Two other candidates were measured and rejected, because both look correct on the first edit and serve a stale template from the second onward:
middlewareMode?[]*thereaddWatchFilealone.tsmoduleaddWatchFile+ self-acceptingWhy no test caught it
The e2e sentinel tests assert the browser did not act on a reload. That stayed true while the server kept asking for one.
setupEventListenerswas meant to cover the gap, but an injected<script type="module">has noimport.meta.hot, so it never registered a listener — and no test ever read its output.Added
HmrDetector.captureWirePayloads, which wrapsWebSocketbefore page load and persists payloads insessionStorageso afull-reloadis still observable after the reload it caused. Two new tests:full-reloadpayload (fails onmainwith["connected","full-reload","custom","full-reload"])index.htmlstill sends one — it is not hot-swappable and must keep reloadingVerification
oxfmt --checkclean.middlewareModerepro at the reporter's versions:navs: 0, sentinel survives, second edit rendersMARKER_V2_EDITED,moduleEvalCount: 0.middlewareMode:index.htmlstill reloads, component.cssstill hot-swaps, a global stylesheet still flows through Vite's CSS pipeline (Plugin swallows HMR updates that it doesn't handle #185), plain.tsstill full-reloads.Review round (Codex)
Four P2 findings, each verified empirically before acting. Two were real regressions from this PR and are fixed; two are declined with measurements.
templateUrlonly updates one owner?rawvariant marked self-acceptingb52bc85liveReload: false07ed3ecOn #4: clearing the flag does not restore Vite's droppable
.htmlreload — the node istype: 'js', so propagation dead-ends and Vite sendspath="*", an unconditional reload in both modes. That is the same regression class #3 just removed. Full measurements are in the review threads.Known limits
templateUrlshared by two components still hot-updates only one of them — pre-existing, tracked in A templateUrl shared by two components only hot-updates one of them #445.liveReload: false, a reload serves the old template, because everyresourceCache.deletesits insidehandleHotUpdatepast its early return. Pre-existing onmain; a product call, not settled here.🤖 Generated with Claude Code
https://claude.ai/code/session_01FUvBViRkyuDvqUMkuT5XnH
Note
Medium Risk
Touches Vite HMR module-graph and self-accept behavior; a mistake can cause full reloads, stale templates, or duplicate component classes (NG0912). Scoped to templates with unit and e2e coverage.
Overview
Stops Vite from putting a
full-reloadon the HMR socket when a componenttemplateUrl.htmlis edited, which stacked on top ofangular:component-updateand always reloaded inmiddlewareMode(path*) — #443.The plugin now
addWatchFiles templates (HMR on only, never styles) so they exist asjsmodules in the graph, then marks the template’s own node self-accepting so the update does not re-execute the component.ts.?rawand other postfixed variants stay un-marked;index.htmlstill full-reloads.E2E now records HMR websocket payloads (not just DOM sentinels) and asserts a template edit sends no
full-reloadwhileindex.htmlstill does.Reviewed by Cursor Bugbot for commit 07ed3ec. Bugbot is set up for automated code reviews on this repo. Configure here.