fix(vite): rebuild component styles when a transitive preprocessor dependency changes - #425
fix(vite): rebuild component styles when a transitive preprocessor dependency changes#425Alexays wants to merge 11 commits into
Conversation
…pendency changes resolveResources runs each styleUrl through Vite's preprocessCSS but discarded the deps it reports (Sass partials pulled in through @use, @import or meta.load-css, Less imports, ...). Editing such a shared partial therefore never invalidated the compiled style nor dispatched component HMR: the dev server kept serving the stale CSS until the styleUrl file itself was touched. Track the reported deps per compiled style, map each dep back to every style built on top of it, and on a hot update of a dep invalidate those styles and dispatch HMR for each owning component. A dep shared by several components (a design-system partial, typically) updates all of them, not just the last registered owner.
02e6a57 to
0850aea
Compare
|
@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: 0850aeaff7
ℹ️ 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".
When a component stylesheet was edited so its `@use`/`@import` list changed, handleHotUpdate dropped the style's cached dependency list but nothing re-recorded the fresh processed.deps: the `@ng/component` HMR endpoint recompiles the CSS without touching the dep maps, and returning [] skips the component transform. The newly imported partial was therefore never registered in styleDepOwners, so subsequent edits to it produced no component update until a full reload or another transform. Refactor dep registration into registerStyleDeps (idempotent: prunes stale owner entries and records fresh ones) and use it from both the initial transform and a new refreshStyleDeps in handleHotUpdate's style branch, which re-reads and re-preprocesses the edited style before dispatching HMR. Adds a regression test that switches a style's Sass `@use` import via HMR and asserts the newly imported partial then dispatches component updates.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fbb8bb5a1
ℹ️ 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".
…mports The shared-dependency branch of handleHotUpdate dropped each owning style's cached dependency list when an edited partial changed, but did not re-register the fresh processed.deps the way the direct-resource branch now does. If the partial itself switched a nested `@use`/`@import`, the newly loaded nested file never entered styleDepOwners, so its next edit fell through to Vite and produced no component update. Call refreshStyleDeps for each owning style during the rebuild, iterating over a snapshot of the owners: refreshStyleDeps mutates the owner sets via registerStyleDeps, and a re-added style would otherwise be visited again during live Set iteration. Adds a regression test where a partial switches its own nested Sass `@use` via HMR and asserts the newly imported nested file then dispatches component updates.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ff38e50a9
ℹ️ 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".
…rd refreshes Two related gaps in the external-resource HMR branch: - A changed file can be both a shared preprocessor dep of one component's style and another component's direct templateUrl/styleUrl. The shared-dep branch returned early after updating the importing styles, so the direct owner's compiled-CSS cache was never dropped and no update was dispatched for it — its rendered CSS stayed stale. Process both roles before returning: fall through to the direct-resource branch and skip only files that are purely transitive deps (already handled, avoiding a duplicate update for the same component). - refreshStyleDeps ran unconditionally in the direct-resource branch, feeding HTML template contents and filenames to preprocessCSS. With PostCSS plugins configured this runs the CSS pipeline on HTML during template HMR (parse errors or side-effectful processing). Guard the refresh so it runs only for actual styles (files compiled as styles during a transform). Adds a regression test where shared.component.scss is both imported by component A and used directly by component B, asserting both receive component-update events when it changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134986a55d
ℹ️ 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".
…ntly Two gaps in style-dependency HMR: - Vite's watcher only observes the dev-server root (plus config/env files and publicDir). Preprocessor deps that resolve outside it (shared monorepo packages, configured include paths) were added to resourceToComponent but never to the watcher, so their edits never reached handleHotUpdate and the component CSS stayed stale. Register every discovered dependency (and any dep picked up by an HMR-driven style refresh) with the watcher via server.watcher.add. - The direct-style marker used styleDepsCache presence, which is absent for a direct styleUrl whose initial preprocessing failed (e.g. a temporarily missing import), so fixing the style refreshed nothing and the now-valid imports were never registered. It also missed on Windows, where cache keys keep native separators while handleHotUpdate lookups are normalized. Track direct styleUrls in a dedicated normalized set that is populated regardless of preprocessing outcome. Adds regression tests: deps registered with the watcher on transform, and a broken-then-fixed direct style whose imports become trackable.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d5a757d3b
ℹ️ 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".
- Transitive style deps were written into resourceToComponent, which is single-owner per resource. A dep of one component's style could therefore overwrite another component's direct styleUrl mapping (and a later prune pass could delete it outright), leaving the direct owner without updates when the shared file changed. Watch the deps but never register them as owners; the direct-owner test now transforms the importing component last, the order that previously reproduced the bug. - The watcher regression test compared raw paths, which differ on Windows CI: the temp dir uses 8.3 short names (RUNNER~1) while Sass canonicalizes loaded URLs to long names. Compare realpath-canonicalized paths instead.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebbd4ba5a0
ℹ️ 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".
…esheets When a Sass/Less partial is consumed both by a component stylesheet and by Vite's ordinary CSS graph (e.g. a global styles.scss imports the same partial), the Angular branch returned [] after dispatching component updates. That filtered every module out of Vite's default HMR pipeline, so the component updated but the global stylesheet stayed stale. Return ctx.modules instead: component updates are dispatched via the ws endpoint, and Vite's remaining modules (the importing global stylesheet) keep flowing through its default pipeline. Update the two hmr-hot-update tests that encoded the old swallow-everything contract, and the style-deps mock context no longer fabricates a module for the changed file. Adds a regression test: a partial shared with a global stylesheet keeps that module in the returned context while still dispatching component HMR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9361c8141a
ℹ️ 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".
resourceToComponent is single-valued, so when two component files used the same styleUrl and that style imported a changed partial, the shared-dep branch looked up only the last-transformed owner and the other component kept stale CSS. The direct-resource branch had the same gap when the shared style itself was edited. Track all direct component owners per compiled style in a multi-valued map (style -> Set of component files), populated and pruned alongside resourceToComponent, and dispatch the update to every owner in both the shared-dep and direct-resource branches. Adds a regression test: two components sharing one style file that imports an edited partial both receive component-update events.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64a85fd061
ℹ️ 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".
… styles When components A and B shared a direct style and B was transformed last, resourceToComponent pointed at B. If B later switched to another style, its prune deleted that single-valued entry; styleComponentOwners correctly kept A, but the direct-resource branch was gated on resourceToComponent.has, so subsequent edits to the shared root style never dispatched an update to A. Enter the direct-resource branch when styleComponentOwners also has the file, so direct-style edits dispatch to every remaining owner. Templates still dispatch through the single-valued resourceToComponent entry. Adds a regression test: two components share a style, one switches away, and editing the shared style still updates the remaining owner.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bb16f07f5
ℹ️ 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".
On Windows, the transform registers styles under the raw path.resolve result (backslashes) while handleHotUpdate supplies a normalized ctx.file to refreshStyleDeps. registerStyleDeps therefore created a second registration instead of replacing the first: removed imports stayed tracked and current partial edits could dispatch duplicate component updates. Use the normalized style path consistently as the styleDepsCache key and as the owner value in styleDepOwners, and normalize resourceCache keys (templates and styles) to match, so every lookup from handleHotUpdate hits on all platforms.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f17b16d4a2
ℹ️ 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".
The external-resource HMR branch only matched html/css/scss/sass/less files, but preprocessCSS reports deps for every stylesheet language Vite supports (Stylus, PostCSS, SugarSS included). Edits to a .styl/.stylus component style or one of its partials therefore fell through without invalidating the compiled style or dispatching component HMR. Widen the extension filter to Vite's full CSS_LANGS_RE set. Adds a regression test using a .styl styleUrl: preprocessing fails without the stylus package, but the file is still tracked as a direct style and must dispatch HMR when edited.
2 participants
Problem
Editing a stylesheet that a component style depends on transitively (a Sass partial pulled in through
@use/@import/meta.load-css, a Less import, ...) does nothing in dev mode: the compiled component style is cached and no HMR update is dispatched. The stale CSS is served until thestyleUrlfile itself is touched (or the dev server restarted).The Angular CLI's builder handles this case (it watches the Sass
loadedUrls), so projects migrating to this plugin lose partial watching silently, which is easy to misdiagnose as an application bug. In our app the shared partial is included by a dozen component stylesheets, and edits to it were invisible.Root cause
resolveResourcesruns eachstyleUrlthrough Vite'spreprocessCSS, which already reports every file the preprocessor loaded inprocessed.deps, but the plugin discarded that field. Transitive deps were therefore never registered inresourceToComponent, sohandleHotUpdatefell through to Vite's default pipeline (which has no module for them either), and theresourceCacheentry for the compiled style was never invalidated.Fix
processed.deps(normalized) per compiled style, cached alongside the compiled CSS so cache hits re-register them and the transform's prune loop does not drop them.resourceToComponentkeeps a single owner per resource, and a shared partial belongs to many components; with a single owner only the last transformed component would receive the update.handleHotUpdate, when a changed file is a known style dep: drop the compiled-CSS cache of every owning style and dispatch component HMR for each owner (the@ng/componentendpoint already re-reads and re-preprocesses styles from disk, so the served update is fresh).Tests
New
style-deps-hmr.test.ts, reusing the harness style ofhmr-hot-update.test.tswith a real resolved config so Sass actually runs and reports its deps:angular:component-updatefor both and returns[];ctx.modules).Adds
sassas a devDependency for that test.pnpm test: 202 passed (12 files).oxfmt --checkclean; theoxlint --type-awarefailure one2e/app/vite.config.tsis pre-existing onmain.Also validated end-to-end against a real application (large Angular admin, Sass partial shared by ~10 components): before the patch edits to the partial were never picked up; with it, each edit recompiles the owning styles and hot-updates every owning component within a couple of seconds.