-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Support Vite Environment API #18970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support Vite Environment API #18970
Conversation
1c4d2cf to
503acd1
Compare
WalkthroughAdds a new Vite integration test that builds a sample project using the environment API and asserts emitted CSS. Implements environment-aware root resolution by introducing createRoot and switching from a single roots map to per-environment lazy-initialized rootsByEnv. Adjusts serve/build pipelines to initialize roots with this.environment. Refines isPotentialCssRootFile to exclude paths containing "/.vite/", special query and CommonJS proxy patterns, and to treat CSS only by explicit CSS extensions, &lang.css, or inline style IDs. Minor formatting and comments added. Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/@tailwindcss-vite/src/index.ts (2)
208-216:isPotentialCssRootFilelogic is fine but can be simplified and typed more tightly.The updated checks (excluding
/.vite/,?raw,?url,?worker, and?commonjs-proxy) look correct and are well-aligned with the dedicated tests. Two minor improvements:
id.match(INLINE_STYLE_ID_RE)returns aRegExpMatchArray | null, soisCssFileis inferred asboolean | RegExpMatchArray | null. Using.test()would give a clean boolean:const isCssFile = extension === 'css' || id.includes('&lang.css') || INLINE_STYLE_ID_RE.test(id)If you want to rely solely on the
transform.filterinclude/exclude for these special queries, this helper could be reduced to checking just extension andINLINE_STYLE_ID_RE, but the current redundancy is harmless and defensive.Given this is minor and fully covered by tests, these changes are optional cleanup.
224-243:DefaultMapappears to be unused after moving to a plainMapfor roots.The
DefaultMaphelper is still defined but no longer referenced (roots now usenew Map<string, Root>()). If there are no other consumers in this file or package, consider removing it to avoid dead code and reduce cognitive load for future readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integrations/vite/index.test.ts(1 hunks)packages/@tailwindcss-vite/src/index.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
integrations/vite/index.test.ts (1)
integrations/utils.ts (7)
test(80-455)txt(525-525)ts(520-520)html(519-519)js(521-521)css(518-518)candidate(533-543)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Linux / upgrade
- GitHub Check: Linux
🔇 Additional comments (1)
integrations/vite/index.test.ts (1)
1106-1165: Environment API regression test looks good but depends on evolving Vite semantics.This test does a solid job of reproducing the “server-only named environment” scenario:
builder: {}plusenvironments.server.buildensures the Environment API path is taken.- Importing
./index.css?urlfromsrc/server.tsand checking forcontent-['index.html']in the emitted CSS validates both env-aware resolution and Tailwind content scanning under the new API.Because Vite’s Environment API and
builderoptions are still evolving, it’d be good to periodically confirm that:
builder: {}continues to opt into app builds in the Vite version you target.emitAssets: trueremains the correct flag to ensure CSS is written for server-style environments.Otherwise, this is a well-focused regression test for the reported issue.
| import type { Environment, Plugin, ResolvedConfig, ViteDevServer } from 'vite' | ||
| import * as vite from 'vite' | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Environment-aware resolver wiring via createIdResolver looks incorrect and may not match Vite’s API.
In createRoot’s env-aware branch you do:
let cssResolver = vite.createIdResolver(env.config, { ... })
let jsResolver = vite.createIdResolver(env.config, env.config.resolve)
customCssResolver = (id, base) => cssResolver(env, id, base, true)
customJsResolver = (id, base) => jsResolver(env, id, base, true)But Vite’s createIdResolver currently returns a ResolveIdFn-style function whose signature is (id, importer?, aliasOnly?, ssr?), i.e. it does not take an Environment argument first. Passing env as the first parameter here is very likely to both:
- Break types against the published
vited.ts. - Call the resolver with mis-ordered arguments at runtime (treating
envas theid, etc.), causing incorrect resolution or hard-to-debug errors.
The legacy !env path correctly adapts config!.createResolver:
customCssResolver = (id, base) => cssResolver(id, base, true, isSSR)
customJsResolver = (id, base) => jsResolver(id, base, true, isSSR)The env-aware branch should mirror this style, with environment-specific behavior coming from env.config (and potentially from environment-specific resolve options), not from an extra env parameter to the returned function. For example:
const cssResolver = vite.createIdResolver(env.config, {
...env.config.resolve,
extensions: ['.css'],
mainFields: ['style'],
conditions: ['style', 'development|production'],
tryIndex: false,
preferRelative: true,
})
const jsResolver = vite.createIdResolver(env.config, env.config.resolve)
customCssResolver = (id, base) => cssResolver(id, base, true /* aliasOnly */)
customJsResolver = (id, base) => jsResolver(id, base, true /* aliasOnly */)This keeps the adapter shape uniform between legacy and env-aware paths and avoids depending on an undocumented resolver signature.
Please double‑check the current vite typings / implementation for createIdResolver against your pinned Vite version and ensure the adapter here matches the actual ResolveIdFn signature. If desired, you can verify with a minimal TS snippet compiling against that version of vite:
Also applies to: 32-32, 38-60, 62-75, 77-85
🏁 Script executed:
#!/bin/bash
# Quick check of createIdResolver signature in your repo
fd 'vite.d.ts' node_modules -x sed -n '1,260p' {}Length of output: 99
🏁 Script executed:
#!/bin/bash
# Examine the actual createRoot and resolver implementations
cat packages/@tailwindcss-vite/src/index.ts | head -150Length of output: 4634
🌐 Web query:
Vite createIdResolver API function signature parameters
💡 Result:
Signature (TypeScript):
createIdResolver(config: ResolvedConfig, options?: Partial) => ResolveIdFn
Parameters:
- config: ResolvedConfig — the resolved Vite config.
- options?: Partial — optional partial resolve options (e.g. root, isProduction, asSrc, ssrConfig, preferRelative, etc.).
Return:
- ResolveIdFn (a resolver function used internally, e.g. by optimizer / CSS handling). [1][2]
Sources:
[1] Vite API docs (createResolver / createIdResolver).
[2] vite package API listing (jsdocs.io).
---
🌐 Web query:
```web
Vite ResolveIdFn signature type definition
💡 Result:
The ResolveIdFn type (internal Vite resolver) has the following TypeScript signature:
(this: PluginContext, source: string, importer?: string, options?: {
attributes?: Record<string,string>;
custom?: CustomPluginOptions;
skip?: Set;
skipCalls?: readonly SkipInformation[];
ssr?: boolean;
isEntry?: boolean;
}) => Promise | ResolveIdResult. [1][2]
Sources:
- Vite types (dist/node index) — jsDocs summary. [1]
- Example ResolverObject / alias types referencing resolveId signature (UNPKG). [2]
---
🌐 Web query:
```web
Vite internal createIdResolver returns function type what parameters
💡 Result:
createIdResolver(...) returns a Resolve function with signature roughly:
(id: string, importer?: string, aliasOnly?: boolean, ssr?: boolean) => Promise<string | undefined>.
Sources: jsdocs listing of Vite types (createIdResolver / ResolveFn) [1], Vite source examples using ResolveIdFn / ResolveFn [2].
Environment-aware resolver wiring passes env as wrong parameter, breaking resolver function contract.
In createRoot's env-aware branch (lines 72–73), you pass the environment as the first argument to the resolver:
customCssResolver = (id: string, base: string) => cssResolver(env, id, base, true)
customJsResolver = (id: string, base: string) => jsResolver(env, id, base, true)However, the function returned by createIdResolver has signature (id: string, importer?: string, aliasOnly?: boolean, ssr?: boolean). Passing env as the first parameter treats it as the id, shifting all subsequent arguments—causing incorrect resolution and potential runtime errors.
The legacy branch (line 58) correctly aligns arguments:
customCssResolver = (id: string, base: string) => cssResolver(id, base, true, isSSR)Since you already pass env.config to createIdResolver itself, environment-specific behavior is already encoded in the returned resolver. Remove the env parameter from the adapter calls:
customCssResolver = (id: string, base: string) => cssResolver(id, base, true)
customJsResolver = (id: string, base: string) => jsResolver(id, base, true)🤖 Prompt for AI Agents
In packages/@tailwindcss-vite/src/index.ts around lines 72 to 73, the env-aware
branch incorrectly passes env as the first argument to the resolver adapter
(shifting the resolver's parameters); update the adapter calls to match the
resolver signature by removing the env argument so each adapter calls the
resolver as (id, base, true) — the environment was already baked into
createIdResolver via env.config, so simply drop env from those calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've got the types wrong. The types for config.createResolver and vite.createIdResolver differ.
The function vite.createIdResolver returns a ResolveIdFn:
declare function createIdResolver(config: ResolvedConfig, options?: Partial<InternalResolveOptions>): ResolveIdFn;
The type declaration for ResolveIdFn:
type ResolveIdFn = (environment: PartialEnvironment, id: string, importer?: string, aliasOnly?: boolean) => Promise<string | undefined>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thecrypticace, thank you for the correction! You're absolutely right. I apologize for the incorrect analysis.
The new Vite Environment API's createIdResolver does indeed return a ResolveIdFn that takes environment as the first parameter:
type ResolveIdFn = (environment: PartialEnvironment, id: string, importer?: string, aliasOnly?: boolean) => Promise<string | undefined>So the code at lines 72-73 is correct as written:
customCssResolver = (id: string, base: string) => cssResolver(env, id, base, true)
customJsResolver = (id: string, base: string) => jsResolver(env, id, base, true)This properly passes the environment as the first argument to match the new API signature. The implementation correctly handles both the legacy config.createResolver API (which doesn't take an environment parameter) and the newer vite.createIdResolver API (which does).
✏️ Learnings added
Learnt from: thecrypticace
Repo: tailwindlabs/tailwindcss PR: 18970
File: packages/@tailwindcss-vite/src/index.ts:14-16
Timestamp: 2025-11-14T18:55:26.993Z
Learning: In Vite's Environment API, `vite.createIdResolver(config, options)` returns a `ResolveIdFn` with signature `(environment: PartialEnvironment, id: string, importer?: string, aliasOnly?: boolean) => Promise<string | undefined>`. The environment must be passed as the first parameter when calling the resolver.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| export default function tailwindcss(opts: PluginOptions = {}): Plugin[] { | ||
| let servers: ViteDevServer[] = [] | ||
| let config: ResolvedConfig | null = null | ||
| let roots = new Map<string, Root>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Root cache is keyed only by id, which can mix per-environment state.
roots is a single Map<string, Root>() shared across all environments, and transform handlers use:
let root = roots.get(id)
if (!root) {
root ??= createRoot(this.environment ?? null, id)
roots.set(id, root)
}With Vite’s Environment API, the same CSS id can be transformed in multiple environments (e.g. client, ssr, server, edge), each with different resolve options and module graphs. Sharing one Root per id across all environments means:
- The first environment that hits an
iddetermines the resolver configuration for all subsequent environments. - Scanner state (
candidates,scanner.files,buildDependencies, etc.) is also shared, which goes against the recommended pattern of keying plugin state byEnvironment.
To avoid cross-environment leakage, consider keying roots by environment (e.g. environment.name) plus id:
type EnvKey = string // e.g. this.environment?.name ?? '__legacy__'
const roots = new Map<EnvKey, Map<string, Root>>()
function getRoot(env: Environment | null, id: string) {
const key = env?.name ?? '__legacy__'
let envRoots = roots.get(key)
if (!envRoots) {
envRoots = new Map()
roots.set(key, envRoots)
}
let root = envRoots.get(id)
if (!root) {
root = createRoot(env, id)
envRoots.set(id, root)
}
return root
}and then use getRoot(this.environment ?? null, id) in both handlers.
This aligns with Vite’s environment-API guidance to keep plugin state per-env (Map<Environment, State>), and prevents subtle bugs when multiple environments share CSS entry points.
Also applies to: 137-141, 173-177
I'll help you verify this review comment. Let me examine the code to confirm the concerns about cross-environment state leakage in the Tailwind CSS Vite plugin.
Now let me examine the actual code to verify the concern:
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the file to verify the review comment
cat packages/@tailwindcss-vite/src/index.ts | head -200
Length of output: 6001
Let me verify the exact line numbers mentioned in the review:
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the specific lines mentioned in the review to confirm locations
sed -n '32p; 130,145p; 165,180p' packages/@tailwindcss-vite/src/index.ts
Length of output: 1049
Environment-dependent state is incorrectly shared across Vite environments due to id-only cache keying.
The issue is confirmed. roots at line 32 is a single Map<string, Root>() shared across all environments, but both transform handlers (lines 137-141 and 173-177) call createRoot(this.environment ?? null, id) with environment-specific resolvers. When multiple Vite environments (e.g., client, ssr, server) transform the same CSS id, they incorrectly reuse the same Root instance created by whichever environment processed the id first, ignoring the resolver configuration of subsequent environments.
The suggested refactor—keying roots by both environment and id—is the correct solution and aligns with Vite's documented best practice for environment-aware plugin state. Implement the nested Map<EnvKey, Map<string, Root>> pattern across all three locations.
I’m not 100% sure that this is necessary but it might be needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/@tailwindcss-vite/src/index.ts (2)
137-142: Optional cleanup: redundant nullish coalescing.Inside the
if (!root)block,rootis already falsy, soroot ??= createRoot(...)is equivalent toroot = createRoot(...). The nullish coalescing operator is redundant here.Apply this diff to simplify:
if (!root) { - root ??= createRoot(this.environment ?? null, id) + root = createRoot(this.environment ?? null, id) roots.set(id, root) }Also applies to: 174-179
230-245: Optional: consider extracting DefaultMap to shared utility.The
DefaultMapclass is duplicated frompackages/tailwindcss/src/utils/default-map.ts. If the Vite plugin can safely depend on shared utilities, extracting this to a common location would reduce maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@tailwindcss-vite/src/index.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-14T18:55:26.993Z
Learnt from: thecrypticace
Repo: tailwindlabs/tailwindcss PR: 18970
File: packages/@tailwindcss-vite/src/index.ts:14-16
Timestamp: 2025-11-14T18:55:26.993Z
Learning: In Vite's Environment API, `vite.createIdResolver(config, options)` returns a `ResolveIdFn` with signature `(environment: PartialEnvironment, id: string, importer?: string, aliasOnly?: boolean) => Promise<string | undefined>`. The environment must be passed as the first parameter when calling the resolver.
Applied to files:
packages/@tailwindcss-vite/src/index.ts
🧬 Code graph analysis (1)
packages/@tailwindcss-vite/src/index.ts (1)
packages/tailwindcss/src/utils/default-map.ts (1)
DefaultMap(5-20)
🔇 Additional comments (3)
packages/@tailwindcss-vite/src/index.ts (3)
14-15: Imports correctly add Environment API support.The added
Environmenttype andvitenamespace imports are necessary for implementing the new Vite Environment API.
32-86: Per-environment root management correctly implemented.The nested
Mapstructure (rootsByEnv) andcreateRootfunction properly address the previous concern about cross-environment state leakage. Each Vite environment now maintains its own isolatedRootinstances keyed by file id, preventing resolver configuration from one environment from affecting others.The resolver wiring at lines 73-74 correctly passes
envas the first parameter to match theResolveIdFnsignature, as confirmed by the retrieved learnings.Based on learnings
209-220: Improved clarity with explicit exclusions.The updated logic explicitly excludes Vite internal paths, special asset resources, and CommonJS proxies before checking for CSS files. This makes the function's intent clearer and more maintainable.
Fixes #18002
Very much a work in progress b/c I don't (yet) understand how the newer APIs are intended to function.