From 53801091a0f148696dd4efb50286911da3482387 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 31 Mar 2025 18:44:06 +0200 Subject: [PATCH] Watch CSS module files for changes (#17467) This PR is a follow-up PR for: https://github.com/tailwindlabs/tailwindcss/pull/17433 In the other PR we allow scanning CSS files for extracting usages of CSS variables. This is important for `.module.css` files that reference these variables but aren't in the same big AST of the main CSS file. This PR also makes sure to watch for changes in those registered CSS files and re-extract the variables when they change. This PR took a bit longer than expected because I was trying to make sure that writing to `./dist/out.css` works without infinite-looping (e.g.: we had issues with this in Tailwind CSS v3 with webpack). But I couldn't reproduce the issue at all. I did had some code that tried to detect if the CSS file contained license headers and skip in (because then it's very likely an output CSS file) but even without it the tests were fine. I setup integration tests with `@tailwindcss/cli` itself, and with tools that use webpack. Added a test for Next.js, and a dedicated webpack test as well. Even without tests, locally, I couldn't reproduce an infinite loop due to changes in an output CSS file... Eventually dropped the code that tries to detect output CSS files. One thing to keep in mind is that if you change any of your "main" CSS files, then we will trigger a full rebuild anyway, so this change is only required for unrelated CSS files (like CSS module files) that use CSS variables. ## Test plan 1. Added integration tests for the CLI and Next.js 2. Added new dedicated test for webpack --- .github/workflows/integration-tests.yml | 1 + CHANGELOG.md | 2 +- crates/oxide/src/extractor/mod.rs | 1 - crates/oxide/src/scanner/mod.rs | 70 +++++-------- crates/oxide/tests/scanner.rs | 2 +- integrations/cli/index.test.ts | 74 ++++++++++++++ integrations/postcss/next.test.ts | 97 ++++++++++++++++++ integrations/utils.ts | 2 +- integrations/webpack/index.test.ts | 110 +++++++++++++++++++++ packages/@tailwindcss-postcss/src/index.ts | 7 +- 10 files changed, 316 insertions(+), 50 deletions(-) create mode 100644 integrations/webpack/index.test.ts diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 761b769d7..acd73ba37 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -31,6 +31,7 @@ jobs: - cli - postcss - workers + - webpack # Exclude windows and macos from being built on feature branches run-all: diff --git a/CHANGELOG.md b/CHANGELOG.md index 13f205219..4d0eec549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,8 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix negated `content` rules in legacy JavaScript configuration ([#17255](https://github.com/tailwindlabs/tailwindcss/pull/17255)) - Extract special `@("@")md:…` syntax in Razor files ([#17427](https://github.com/tailwindlabs/tailwindcss/pull/17427)) - Disallow arbitrary values with top-level braces and semicolons as well as unbalanced parentheses and brackets ([#17361](https://github.com/tailwindlabs/tailwindcss/pull/17361)) -- Extract used CSS variables from `.css` files ([#17433](https://github.com/tailwindlabs/tailwindcss/pull/17433)) - Ensure the `--theme(…)` function still resolves to the CSS variables even when legacy JS plugins are enabled +- Extract used CSS variables from `.css` files ([#17433](https://github.com/tailwindlabs/tailwindcss/pull/17433), [#17467](https://github.com/tailwindlabs/tailwindcss/pull/17467)) ### Changed diff --git a/crates/oxide/src/extractor/mod.rs b/crates/oxide/src/extractor/mod.rs index bcbe3e1ae..736deb28d 100644 --- a/crates/oxide/src/extractor/mod.rs +++ b/crates/oxide/src/extractor/mod.rs @@ -1,6 +1,5 @@ use crate::cursor; use crate::extractor::machine::Span; -use bstr::ByteSlice; use candidate_machine::CandidateMachine; use css_variable_machine::CssVariableMachine; use machine::{Machine, MachineState}; diff --git a/crates/oxide/src/scanner/mod.rs b/crates/oxide/src/scanner/mod.rs index e678f7b7f..612f5a864 100644 --- a/crates/oxide/src/scanner/mod.rs +++ b/crates/oxide/src/scanner/mod.rs @@ -16,8 +16,7 @@ use fxhash::{FxHashMap, FxHashSet}; use ignore::{gitignore::GitignoreBuilder, WalkBuilder}; use rayon::prelude::*; use std::collections::{BTreeMap, BTreeSet}; -use std::path::Path; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{self, Arc, Mutex}; use std::time::SystemTime; use tracing::event; @@ -265,18 +264,21 @@ impl Scanner { .and_then(|x| x.to_str()) .unwrap_or_default(); // In case the file has no extension - // Special handing for CSS files to extract CSS variables - if extension == "css" { - self.css_files.push(path); - continue; + match extension { + // Special handing for CSS files, we don't want to extract candidates from + // these files, but we do want to extract used CSS variables. + "css" => { + self.css_files.push(path.clone()); + } + _ => { + self.changed_content.push(ChangedContent::File( + path.to_path_buf(), + extension.to_owned(), + )); + } } self.extensions.insert(extension.to_owned()); - self.changed_content.push(ChangedContent::File( - path.to_path_buf(), - extension.to_owned(), - )); - self.files.push(path); } } @@ -427,43 +429,21 @@ fn read_all_files(changed_content: Vec) -> Vec> { #[tracing::instrument(skip_all)] fn extract_css_variables(blobs: Vec>) -> Vec { - let mut result: Vec<_> = blobs - .par_iter() - .flat_map(|blob| blob.par_split(|x| *x == b'\n')) - .filter_map(|blob| { - if blob.is_empty() { - return None; - } - - let extracted = crate::extractor::Extractor::new(blob).extract_variables_from_css(); - if extracted.is_empty() { - return None; - } - - Some(FxHashSet::from_iter(extracted.into_iter().map( - |x| match x { - Extracted::CssVariable(bytes) => bytes, - _ => &[], - }, - ))) - }) - .reduce(Default::default, |mut a, b| { - a.extend(b); - a - }) - .into_iter() - .map(|s| unsafe { String::from_utf8_unchecked(s.to_vec()) }) - .collect(); - - // SAFETY: Unstable sort is faster and in this scenario it's also safe because we are - // guaranteed to have unique candidates. - result.par_sort_unstable(); - - result + extract(blobs, |mut extractor| { + extractor.extract_variables_from_css() + }) } #[tracing::instrument(skip_all)] fn parse_all_blobs(blobs: Vec>) -> Vec { + extract(blobs, |mut extractor| extractor.extract()) +} + +#[tracing::instrument(skip_all)] +fn extract(blobs: Vec>, handle: H) -> Vec +where + H: Fn(Extractor) -> Vec + std::marker::Sync, +{ let mut result: Vec<_> = blobs .par_iter() .flat_map(|blob| blob.par_split(|x| *x == b'\n')) @@ -472,7 +452,7 @@ fn parse_all_blobs(blobs: Vec>) -> Vec { return None; } - let extracted = crate::extractor::Extractor::new(blob).extract(); + let extracted = handle(crate::extractor::Extractor::new(blob)); if extracted.is_empty() { return None; } diff --git a/crates/oxide/tests/scanner.rs b/crates/oxide/tests/scanner.rs index 70b9a8627..681d63620 100644 --- a/crates/oxide/tests/scanner.rs +++ b/crates/oxide/tests/scanner.rs @@ -325,7 +325,7 @@ mod scanner { ("c.less", ""), ]); - assert_eq!(files, vec!["index.html"]); + assert_eq!(files, vec!["a.css", "index.html"]); assert_eq!(globs, vec!["*"]); assert_eq!(normalized_sources, vec!["**/*"]); } diff --git a/integrations/cli/index.test.ts b/integrations/cli/index.test.ts index dc06ecb09..5cb689ff8 100644 --- a/integrations/cli/index.test.ts +++ b/integrations/cli/index.test.ts @@ -1468,6 +1468,80 @@ test( }, ) +test( + 'changes to CSS files should pick up new CSS variables (if any)', + { + fs: { + 'package.json': json` + { + "dependencies": { + "tailwindcss": "workspace:^", + "@tailwindcss/cli": "workspace:^" + } + } + `, + 'unrelated.module.css': css` + .module { + color: var(--color-blue-500); + } + `, + 'index.css': css` + @import 'tailwindcss/theme'; + @import 'tailwindcss/utilities'; + `, + 'index.html': html`
`, + }, + }, + async ({ spawn, exec, fs, expect }) => { + // Generate the initial build so output CSS files exist on disk + await exec('pnpm tailwindcss --input ./index.css --output ./dist/out.css') + + // NOTE: We are writing to an output CSS file which is not being ignored by + // `.gitignore` nor marked with `@source not`. This should not result in an + // infinite loop. + let process = await spawn( + 'pnpm tailwindcss --input ./index.css --output ./dist/out.css --watch', + ) + await process.onStderr((m) => m.includes('Done in')) + + expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(` + " + --- ./dist/out.css --- + :root, :host { + --color-blue-500: oklch(0.623 0.214 259.815); + } + .flex { + display: flex; + } + " + `) + + await fs.write( + 'unrelated.module.css', + css` + .module { + color: var(--color-blue-500); + background-color: var(--color-red-500); + } + `, + ) + await process.onStderr((m) => m.includes('Done in')) + + expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(` + " + --- ./dist/out.css --- + :root, :host { + --color-red-500: oklch(0.637 0.237 25.331); + --color-blue-500: oklch(0.623 0.214 259.815); + } + .flex { + display: flex; + } + " + `) + }, +) + function withBOM(text: string): string { return '\uFEFF' + text } diff --git a/integrations/postcss/next.test.ts b/integrations/postcss/next.test.ts index a6fb26f8e..acfa75130 100644 --- a/integrations/postcss/next.test.ts +++ b/integrations/postcss/next.test.ts @@ -257,3 +257,100 @@ test( ]) }, ) + +test( + 'changes to CSS files should pick up new CSS variables (if any)', + { + fs: { + 'package.json': json` + { + "dependencies": { + "react": "^18", + "react-dom": "^18", + "next": "^14" + }, + "devDependencies": { + "@tailwindcss/postcss": "workspace:^", + "tailwindcss": "workspace:^" + } + } + `, + 'postcss.config.mjs': js` + export default { + plugins: { + '@tailwindcss/postcss': {}, + }, + } + `, + 'next.config.mjs': js`export default {}`, + 'app/layout.js': js` + import './globals.css' + + export default function RootLayout({ children }) { + return ( + + {children} + + ) + } + `, + 'app/page.js': js` + export default function Page() { + return
+ } + `, + 'unrelated.module.css': css` + .module { + color: var(--color-blue-500); + } + `, + 'app/globals.css': css` + @import 'tailwindcss/theme'; + @import 'tailwindcss/utilities'; + `, + }, + }, + async ({ spawn, exec, fs, expect }) => { + // Generate the initial build so output CSS files exist on disk + await exec('pnpm next build') + + // NOTE: We are writing to an output CSS file which is not being ignored by + // `.gitignore` nor marked with `@source not`. This should not result in an + // infinite loop. + let process = await spawn(`pnpm next dev`) + + let url = '' + await process.onStdout((m) => { + let match = /Local:\s*(http.*)/.exec(m) + if (match) url = match[1] + return Boolean(url) + }) + + await process.onStdout((m) => m.includes('Ready in')) + + await retryAssertion(async () => { + let css = await fetchStyles(url) + expect(css).toContain(candidate`flex`) + expect(css).toContain('--color-blue-500:') + expect(css).not.toContain('--color-red-500:') + }) + + await fs.write( + 'unrelated.module.css', + css` + .module { + color: var(--color-blue-500); + background-color: var(--color-red-500); + } + `, + ) + await process.onStdout((m) => m.includes('Compiled in')) + + await retryAssertion(async () => { + let css = await fetchStyles(url) + expect(css).toContain(candidate`flex`) + expect(css).toContain('--color-blue-500:') + expect(css).toContain('--color-red-500:') + }) + }, +) diff --git a/integrations/utils.ts b/integrations/utils.ts index 4558edd03..f4218b176 100644 --- a/integrations/utils.ts +++ b/integrations/utils.ts @@ -321,7 +321,7 @@ export function test( return [ file, // Drop license comment - content.replace(/[\s\n]*\/\*! tailwindcss .*? \*\/[\s\n]*/g, ''), + content.replace(/[\s\n]*\/\*![\s\S]*?\*\/[\s\n]*/g, ''), ] }), ) diff --git a/integrations/webpack/index.test.ts b/integrations/webpack/index.test.ts new file mode 100644 index 000000000..3c59d0bab --- /dev/null +++ b/integrations/webpack/index.test.ts @@ -0,0 +1,110 @@ +import { css, html, js, json, test } from '../utils' + +test( + 'Webpack + PostCSS (watch)', + { + fs: { + 'package.json': json` + { + "main": "./src/index.js", + "browser": "./src/index.js", + "dependencies": { + "css-loader": "^6", + "postcss": "^8", + "postcss-loader": "^7", + "webpack": "^5", + "webpack-cli": "^5", + "mini-css-extract-plugin": "^2", + "tailwindcss": "workspace:^", + "@tailwindcss/postcss": "workspace:^" + } + } + `, + 'postcss.config.js': js` + /** @type {import('postcss-load-config').Config} */ + module.exports = { + plugins: { + '@tailwindcss/postcss': {}, + }, + } + `, + 'webpack.config.js': js` + let MiniCssExtractPlugin = require('mini-css-extract-plugin') + + module.exports = { + output: { + clean: true, + }, + plugins: [new MiniCssExtractPlugin()], + module: { + rules: [ + { + test: /.css$/i, + use: [MiniCssExtractPlugin.loader, 'css-loader', 'postcss-loader'], + }, + ], + }, + } + `, + 'src/index.js': js`import './index.css'`, + 'src/index.html': html` +
+ `, + 'src/index.css': css` + @import 'tailwindcss/theme'; + @import 'tailwindcss/utilities'; + `, + 'src/unrelated.module.css': css` + .module { + color: var(--color-blue-500); + } + `, + }, + }, + async ({ fs, spawn, exec, expect }) => { + // Generate the initial build so output CSS files exist on disk + await exec('webpack --mode=development') + + // NOTE: We are writing to an output CSS file which is not being ignored by + // `.gitignore` nor marked with `@source not`. This should not result in an + // infinite loop. + let process = await spawn('webpack --mode=development --watch') + await process.onStdout((m) => m.includes('compiled successfully in')) + + expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(` + " + --- ./dist/main.css --- + :root, :host { + --color-blue-500: oklch(0.623 0.214 259.815); + } + .flex { + display: flex; + } + " + `) + + await fs.write( + 'src/unrelated.module.css', + css` + .module { + color: var(--color-blue-500); + background-color: var(--color-red-500); + } + `, + ) + await process.onStdout((m) => m.includes('compiled successfully in')) + + expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(` + " + --- ./dist/main.css --- + :root, :host { + --color-red-500: oklch(0.637 0.237 25.331); + --color-blue-500: oklch(0.623 0.214 259.815); + } + .flex { + display: flex; + } + " + `) + }, +) diff --git a/packages/@tailwindcss-postcss/src/index.ts b/packages/@tailwindcss-postcss/src/index.ts index d8ef55776..b15e124b7 100644 --- a/packages/@tailwindcss-postcss/src/index.ts +++ b/packages/@tailwindcss-postcss/src/index.ts @@ -211,10 +211,15 @@ function tailwindcss(opts: PluginOptions = {}): AcceptedPlugin { DEBUG && I.start('Register dependency messages') // Add all found files as direct dependencies for (let file of context.scanner.files) { + let absolutePath = path.resolve(file) + // The CSS file cannot be a dependency of itself + if (absolutePath === result.opts.from) { + continue + } result.messages.push({ type: 'dependency', plugin: '@tailwindcss/postcss', - file: path.resolve(file), + file: absolutePath, parent: result.opts.from, }) }