Skip to content

Conversation

@thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Sep 19, 2025

Fixes #18002

Very much a work in progress b/c I don't (yet) understand how the newer APIs are intended to function.

  • Needs env specific tests that verify the environment API is being used

@thecrypticace thecrypticace marked this pull request as ready for review November 14, 2025 18:34
@thecrypticace thecrypticace requested a review from a team as a code owner November 14, 2025 18:34
@coderabbitai
Copy link

coderabbitai bot commented Nov 14, 2025

Walkthrough

Adds 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)
Check name Status Explanation Resolution
Out of Scope Changes check ❓ Inconclusive While the main changes align with the issue, the PR includes test coverage for environment API usage but the implementation may not fully address all aspects of the createIdResolver migration mentioned in Vite PR #20031. Verify that the implementation fully adopts createIdResolver and test that the plugin works correctly with non-'ssr' environment names as mentioned in the original issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Support Vite Environment API' clearly and concisely describes the main objective of the PR—adding support for Vite's Environment API to the Tailwind CSS Vite plugin.
Description check ✅ Passed The description references issue #18002 and acknowledges the work-in-progress status with a checklist for environment-specific tests, which relates to the changeset of adding environment API support.
Linked Issues check ✅ Passed The code changes directly address issue #18002 by implementing environment-aware Root resolution with createRoot(env, id), per-environment roots map (rootsByEnv), and updated resolver logic compatible with Vite's newer Environment API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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: isPotentialCssRootFile logic 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 a RegExpMatchArray | null, so isCssFile is inferred as boolean | 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.filter include/exclude for these special queries, this helper could be reduced to checking just extension and INLINE_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: DefaultMap appears to be unused after moving to a plain Map for roots.

The DefaultMap helper is still defined but no longer referenced (roots now use new 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

📥 Commits

Reviewing files that changed from the base of the PR and between 21f2bf9 and 503acd1.

📒 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: {} plus environments.server.build ensures the Environment API path is taken.
  • Importing ./index.css?url from src/server.ts and checking for content-['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 builder options 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: true remains the correct flag to ensure CSS is written for server-style environments.

Otherwise, this is a well-focused regression test for the reported issue.

Comment on lines +14 to 16
import type { Environment, Plugin, ResolvedConfig, ViteDevServer } from 'vite'
import * as vite from 'vite'

Copy link

@coderabbitai coderabbitai bot Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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 vite d.ts.
  • Call the resolver with mis-ordered arguments at runtime (treating env as the id, 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 -150

Length 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.

Copy link
Contributor Author

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>

Copy link

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>()
Copy link

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 id determines 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 by Environment.

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
Copy link

@coderabbitai coderabbitai bot left a 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, root is already falsy, so root ??= createRoot(...) is equivalent to root = 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 DefaultMap class is duplicated from packages/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

📥 Commits

Reviewing files that changed from the base of the PR and between 503acd1 and e42513c.

📒 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 Environment type and vite namespace imports are necessary for implementing the new Vite Environment API.


32-86: Per-environment root management correctly implemented.

The nested Map structure (rootsByEnv) and createRoot function properly address the previous concern about cross-environment state leakage. Each Vite environment now maintains its own isolated Root instances keyed by file id, preventing resolver configuration from one environment from affecting others.

The resolver wiring at lines 73-74 correctly passes env as the first parameter to match the ResolveIdFn signature, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v4 vite plugin is not compatible with the vite environment API

2 participants