fix(vite-plugin-angular): scope style package-export condition to .css imports#2330
fix(vite-plugin-angular): scope style package-export condition to .css imports#2330brandonroberts merged 2 commits intoalphafrom
style package-export condition to .css imports#2330Conversation
✅ 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:
📝 WalkthroughWalkthroughThe changes remove code that globally mutated Vite’s Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Key observations
🚥 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. Review rate limit: 6/8 reviews remaining, refill in 8 minutes and 39 seconds.Comment |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…css imports
The Angular Vite plugin injected the `style` package-export condition
into Vite's *global* `resolve.conditions` (in `angular-vite-plugin`,
`fast-compile-plugin`, `compilation-api-plugin`, and the shared
`createDepOptimizerConfig`). That global injection was needed so JS-side
imports of CSS files whose package exports gate them only under `style`
— e.g. `import '@angular/material/prebuilt-themes/azure-blue.css'` —
could resolve at all.
But globalising `style` had a load-bearing side effect: Tailwind v4's
`@plugin` JS resolver inherits Vite's `resolve.conditions`. With `style`
in scope, Tailwind matched the `style` export of packages with mixed
exports (notably `tailwindcss-primeui`, whose `.` exports include both
`style: "./v4/index.css"` and `import: "./v3/index.js"`) and handed the
resolved `.css` file to Node's ESM loader, which crashed with:
Internal server error: Unknown file extension ".css" for
…/tailwindcss-primeui/v4/index.css
Removing the `style` injection outright (as PR #2326 proposes) trades
one regression for another: it breaks `@angular/material/prebuilt-themes`
and any other package that exposes its CSS only under `style`.
This commit scopes the `style` condition to `.css`-extension requests
via a new `cssExtensionStyleResolverPlugin`, registered once at the
`angular()` factory level. The plugin builds its own scoped resolver
(via Vite 8's `vite.createIdResolver` or Vite 7's
`ResolvedConfig.createResolver`) with `style` appended to the user's
configured conditions, and only fires when the requested id is a bare
specifier ending in `.css` (with optional `?inline` / `?module` query).
Net effect:
- `import '@angular/material/prebuilt-themes/azure-blue.css'` ✓
- `@import '@angular/material/prebuilt-themes/azure-blue.css'` ✓
- `import '@angular/material/button'` (non-CSS) ✓
- `@plugin 'tailwindcss-primeui'` (Tailwind JS resolver) ✓
Tests:
- `cssExtensionStyleResolverPlugin` (new, 6 cases) covers the
fire/no-fire cases, condition appending, and both Vite version paths
via a `vi.mock('vite', …)`-backed resolver factory.
- `createDepOptimizerConfig` test asserts the helper no longer
returns a global `resolve` block.
brandonroberts
force-pushed
the
fix/scoped-css-style-resolver
branch
from
253e75e to
1ede6ba
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.ts (1)
12-35: Add coverage for theconfig.createResolverfallback.Every test in this file forces the
vite.createIdResolverpath, so the fallback branch incssExtensionStyleResolverPlugin()never executes. That leaves the progressive-compatibility path unguarded by CI.As per coding guidelines,
{vite.config.ts,packages/vite-plugin-*/**/*.ts}: Maintain compatibility with Vite versions 6-8, with progressive fallbacks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.ts` around lines 12 - 35, The tests currently always exercise the Vite 8 path by mocking createIdResolver, so add a new test that forces the progressive-fallback branch by ensuring the plugin sees config.createResolver instead of vite.createIdResolver: create a test that does not mock createIdResolver (or adjusts the existing vi.mock to omit createIdResolver), instantiate cssExtensionStyleResolverPlugin() with a fake Vite config object that includes a createResolver function, call the resolver returned by cssExtensionStyleResolverPlugin and assert that your fake config.createResolver was invoked and returned the expected resolution; reference the cssExtensionStyleResolverPlugin factory, the vite.createIdResolver override in the current test file, and the config.createResolver fallback so the new test covers that branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts`:
- Around line 115-125: The early-return guard for non-bare specifiers misses
Windows drive-letter absolute paths like "C:/..." so such absolute stylesheet
ids are wrongly treated as package specifiers; update the condition in
css-extension-resolver (the block that checks
id.startsWith('.')|'/'|'\0'|'data:'|'virtual:') to also detect Windows drive
letters (e.g. id matches /^[A-Za-z]:\// after Vite's POSIX-normalization) and
return null for those ids as well so resolveCss() is not invoked for absolute
Windows paths.
---
Nitpick comments:
In `@packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.ts`:
- Around line 12-35: The tests currently always exercise the Vite 8 path by
mocking createIdResolver, so add a new test that forces the progressive-fallback
branch by ensuring the plugin sees config.createResolver instead of
vite.createIdResolver: create a test that does not mock createIdResolver (or
adjusts the existing vi.mock to omit createIdResolver), instantiate
cssExtensionStyleResolverPlugin() with a fake Vite config object that includes a
createResolver function, call the resolver returned by
cssExtensionStyleResolverPlugin and assert that your fake config.createResolver
was invoked and returned the expected resolution; reference the
cssExtensionStyleResolverPlugin factory, the vite.createIdResolver override in
the current test file, and the config.createResolver fallback so the new test
covers that branch.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c9ffc92-17f3-4d93-87a6-32ac9538c5ee
📒 Files selected for processing (7)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.tspackages/vite-plugin-angular/src/lib/compilation-api/compilation-api-plugin.tspackages/vite-plugin-angular/src/lib/fast-compile-plugin.tspackages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.tspackages/vite-plugin-angular/src/lib/utils/css-extension-resolver.tspackages/vite-plugin-angular/src/lib/utils/plugin-config.spec.tspackages/vite-plugin-angular/src/lib/utils/plugin-config.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts (1)
1555-1562: Good plugin placement; add a wiring-order regression test.
cssExtensionStyleResolverPlugin()being registered atangular()level is the right integration point. Consider adding/keeping a focused test that asserts this plugin is present and ordered before compilation plugins to prevent future regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts` around lines 1555 - 1562, Add a regression test that ensures cssExtensionStyleResolverPlugin() is registered when angular() is invoked and that it appears before compilation plugins (e.g., the plugin that handles TypeScript/compilation) to prevent wiring-order regressions; locate the angular() setup where cssExtensionStyleResolverPlugin() and replaceFiles(...) are added and write a focused test that instantiates angular() with minimal pluginOptions, inspects the resulting plugin list or resolved hook ordering, and asserts cssExtensionStyleResolverPlugin() is present and comes before the compilation plugin(s).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts`:
- Around line 1555-1562: Add a regression test that ensures
cssExtensionStyleResolverPlugin() is registered when angular() is invoked and
that it appears before compilation plugins (e.g., the plugin that handles
TypeScript/compilation) to prevent wiring-order regressions; locate the
angular() setup where cssExtensionStyleResolverPlugin() and replaceFiles(...)
are added and write a focused test that instantiates angular() with minimal
pluginOptions, inspects the resulting plugin list or resolved hook ordering, and
asserts cssExtensionStyleResolverPlugin() is present and comes before the
compilation plugin(s).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec2f959e-5758-4127-9028-23706b7c5b33
📒 Files selected for processing (7)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.tspackages/vite-plugin-angular/src/lib/compilation-api/compilation-api-plugin.tspackages/vite-plugin-angular/src/lib/fast-compile-plugin.tspackages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.tspackages/vite-plugin-angular/src/lib/utils/css-extension-resolver.tspackages/vite-plugin-angular/src/lib/utils/plugin-config.spec.tspackages/vite-plugin-angular/src/lib/utils/plugin-config.ts
✅ Files skipped from review due to trivial changes (1)
- packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/vite-plugin-angular/src/lib/utils/plugin-config.spec.ts
- packages/vite-plugin-angular/src/lib/utils/plugin-config.ts
- packages/vite-plugin-angular/src/lib/compilation-api/compilation-api-plugin.ts
- packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.ts
…css resolver Vite POSIX-normalizes resolved ids on Windows but preserves the drive volume, so an absolute stylesheet can reach `cssExtensionStyleResolverPlugin` as `C:/...` rather than `/...`. The early-return guard only checked for `/`, so Windows absolute paths slipped through and were forwarded to the package-exports resolver as if they were bare specifiers. Match Windows drive letters (`/^[A-Za-z]:\//`) and bail out early. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts`:
- Around line 100-109: Update the thrown Error message in the resolver fallback
block (where vite.createIdResolver and ResolvedConfig.createResolver are
checked) to state that Vite 6–8 are supported and that neither API being present
indicates an unsupported or misconfigured Vite version; keep references to
vite.createIdResolver and ResolvedConfig.createResolver in the message so it
clearly communicates the two APIs being checked and that Vite 6–8 are expected
to provide at least one of them.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2f4721d-3592-4950-9dcd-c7be9bdc1335
📒 Files selected for processing (2)
packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.spec.tspackages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts
| // Both APIs have been stable across the supported Vite range | ||
| // (7.x via `ResolvedConfig.createResolver`, 8.x via | ||
| // `vite.createIdResolver`). If neither is present, fail loudly | ||
| // rather than silently regressing CSS imports of `style`-only | ||
| // package exports such as `@angular/material/prebuilt-themes/*`. | ||
| throw new Error( | ||
| '[@analogjs/vite-plugin-angular]: neither vite.createIdResolver ' + | ||
| '(Vite 8) nor ResolvedConfig.createResolver (Vite 7) is available. ' + | ||
| 'Unsupported Vite version.', | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Vite 6 expose createResolver on ResolvedConfig or createIdResolver?
💡 Result:
In Vite 6, ResolvedConfig exposes createResolver, but it is deprecated. Plugins should use createIdResolver(environment.config) instead, where createIdResolver is imported from 'vite'. The createResolver method on ResolvedConfig only works for 'client' and 'ssr' environments and ignores the aliasOnly option.
Citations:
- 1: chore: deprecate
ResolvedConfig.createResolverand recommendcreateIdResolvervitejs/vite#20031 - 2: vitejs/vite@d101d64
- 3: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/config.ts
🏁 Script executed:
cat -n packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts | sed -n '90,115p'Repository: analogjs/analog
Length of output: 1236
🏁 Script executed:
cat -n packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts | sed -n '75,110p'Repository: analogjs/analog
Length of output: 1534
🏁 Script executed:
cat -n packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts | sed -n '60,110p'Repository: analogjs/analog
Length of output: 2108
Update error message to reflect full Vite 6-8 support.
The code correctly supports Vite 6-8 via progressive fallback: first attempting vite.createIdResolver (Vite 8+), then falling back to ResolvedConfig.createResolver (Vite 6-7+). However, the error message at line 105-109 states support only for "Vite 7 and 8" and omits Vite 6. Since Vite 6 exposes ResolvedConfig.createResolver per the web search, update the message to accurately reflect that all three versions (6, 7, 8) are expected to provide at least one of these APIs, clarifying that the error indicates an unexpectedly misconfigured or unsupported Vite version altogether.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vite-plugin-angular/src/lib/utils/css-extension-resolver.ts` around
lines 100 - 109, Update the thrown Error message in the resolver fallback block
(where vite.createIdResolver and ResolvedConfig.createResolver are checked) to
state that Vite 6–8 are supported and that neither API being present indicates
an unsupported or misconfigured Vite version; keep references to
vite.createIdResolver and ResolvedConfig.createResolver in the message so it
clearly communicates the two APIs being checked and that Vite 6–8 are expected
to provide at least one of them.
1 participant
PR Checklist
The Angular Vite plugin injected the
stylepackage-export condition into Vite's globalresolve.conditions(inangular-vite-plugin,fast-compile-plugin,compilation-api-plugin, and the sharedcreateDepOptimizerConfig). That global injection was needed so JS-side imports of CSS whose package exports gate them only understyle— e.g.import '@angular/material/prebuilt-themes/azure-blue.css'— could resolve at all.But globalising
stylehad a load-bearing side effect: Tailwind v4's@pluginJS resolver inherits Vite'sresolve.conditions. Withstylein scope, Tailwind matched thestyleexport of packages with mixed exports (notablytailwindcss-primeui, whose.exports include bothstyle: "./v4/index.css"andimport: "./v3/index.js") and handed the resolved.cssfile to Node's ESM loader, which crashed with:Removing the
styleinjection outright (as analogjs/analog#2326 proposes) trades one regression for another: it breaks@angular/material/prebuilt-themesand any other package that exposes its CSS only understyle.Closes #
Affected scope
vite-plugin-angularRecommended merge strategy for maintainer [optional]
Commit preservation note [optional]
N/A — single focused change.
What is the new behavior?
styleis no longer added to Vite's globalresolve.conditions. Instead, a newcssExtensionStyleResolverPluginis registered once at theangular()factory level. It builds a scoped resolver (via Vite 8'svite.createIdResolveror Vite 7'sResolvedConfig.createResolver) withstyleappended to the user's configured conditions, and only fires when the requested id is a bare specifier ending in.css(with optional?inline/?modulequery).Net effect:
import '@angular/material/prebuilt-themes/azure-blue.css'✓@import '@angular/material/prebuilt-themes/azure-blue.css'✓import '@angular/material/button'(non-CSS) ✓@plugin 'tailwindcss-primeui'(Tailwind JS resolver) ✓The shared
createDepOptimizerConfigno longer returns a globalresolveblock, and the per-plugin globalresolve.conditions: ['style', ...]injection is removed fromangular-vite-plugin,fast-compile-plugin, andcompilation-api-plugin.Test plan
cssExtensionStyleResolverPluginunit tests (new, 6 cases) cover fire/no-fire cases, condition appending, and both Vite version paths via avi.mock('vite', …)-backed resolver factorycreateDepOptimizerConfigtest asserts the helper no longer returns a globalresolveblocknx format:checkpnpm buildpnpm test@angular/material/prebuilt-themes/azure-blue.cssresolves and a Tailwind v4 app using@plugin 'tailwindcss-primeui'no longer crashes the ESM loaderDoes this PR introduce a breaking change?
Other information
Alternative to analogjs/analog#2326, which removes the
styleinjection outright. This PR keeps Material-style packages working by scopingstyleto.cssrequests rather than dropping it.