Skip to content

Commit 5380109

Browse files
authored
Watch CSS module files for changes (#17467)
This PR is a follow-up PR for: #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
1 parent c7ba564 commit 5380109

File tree

10 files changed

+316
-50
lines changed

10 files changed

+316
-50
lines changed

.github/workflows/integration-tests.yml

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ jobs:
3131
- cli
3232
- postcss
3333
- workers
34+
- webpack
3435

3536
# Exclude windows and macos from being built on feature branches
3637
run-all:

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3434
- Fix negated `content` rules in legacy JavaScript configuration ([#17255](https://github.com/tailwindlabs/tailwindcss/pull/17255))
3535
- Extract special `@("@")md:…` syntax in Razor files ([#17427](https://github.com/tailwindlabs/tailwindcss/pull/17427))
3636
- Disallow arbitrary values with top-level braces and semicolons as well as unbalanced parentheses and brackets ([#17361](https://github.com/tailwindlabs/tailwindcss/pull/17361))
37-
- Extract used CSS variables from `.css` files ([#17433](https://github.com/tailwindlabs/tailwindcss/pull/17433))
3837
- Ensure the `--theme(…)` function still resolves to the CSS variables even when legacy JS plugins are enabled
38+
- Extract used CSS variables from `.css` files ([#17433](https://github.com/tailwindlabs/tailwindcss/pull/17433), [#17467](https://github.com/tailwindlabs/tailwindcss/pull/17467))
3939

4040
### Changed
4141

crates/oxide/src/extractor/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::cursor;
22
use crate::extractor::machine::Span;
3-
use bstr::ByteSlice;
43
use candidate_machine::CandidateMachine;
54
use css_variable_machine::CssVariableMachine;
65
use machine::{Machine, MachineState};

crates/oxide/src/scanner/mod.rs

+25-45
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ use fxhash::{FxHashMap, FxHashSet};
1616
use ignore::{gitignore::GitignoreBuilder, WalkBuilder};
1717
use rayon::prelude::*;
1818
use std::collections::{BTreeMap, BTreeSet};
19-
use std::path::Path;
20-
use std::path::PathBuf;
19+
use std::path::{Path, PathBuf};
2120
use std::sync::{self, Arc, Mutex};
2221
use std::time::SystemTime;
2322
use tracing::event;
@@ -265,18 +264,21 @@ impl Scanner {
265264
.and_then(|x| x.to_str())
266265
.unwrap_or_default(); // In case the file has no extension
267266

268-
// Special handing for CSS files to extract CSS variables
269-
if extension == "css" {
270-
self.css_files.push(path);
271-
continue;
267+
match extension {
268+
// Special handing for CSS files, we don't want to extract candidates from
269+
// these files, but we do want to extract used CSS variables.
270+
"css" => {
271+
self.css_files.push(path.clone());
272+
}
273+
_ => {
274+
self.changed_content.push(ChangedContent::File(
275+
path.to_path_buf(),
276+
extension.to_owned(),
277+
));
278+
}
272279
}
273280

274281
self.extensions.insert(extension.to_owned());
275-
self.changed_content.push(ChangedContent::File(
276-
path.to_path_buf(),
277-
extension.to_owned(),
278-
));
279-
280282
self.files.push(path);
281283
}
282284
}
@@ -427,43 +429,21 @@ fn read_all_files(changed_content: Vec<ChangedContent>) -> Vec<Vec<u8>> {
427429

428430
#[tracing::instrument(skip_all)]
429431
fn extract_css_variables(blobs: Vec<Vec<u8>>) -> Vec<String> {
430-
let mut result: Vec<_> = blobs
431-
.par_iter()
432-
.flat_map(|blob| blob.par_split(|x| *x == b'\n'))
433-
.filter_map(|blob| {
434-
if blob.is_empty() {
435-
return None;
436-
}
437-
438-
let extracted = crate::extractor::Extractor::new(blob).extract_variables_from_css();
439-
if extracted.is_empty() {
440-
return None;
441-
}
442-
443-
Some(FxHashSet::from_iter(extracted.into_iter().map(
444-
|x| match x {
445-
Extracted::CssVariable(bytes) => bytes,
446-
_ => &[],
447-
},
448-
)))
449-
})
450-
.reduce(Default::default, |mut a, b| {
451-
a.extend(b);
452-
a
453-
})
454-
.into_iter()
455-
.map(|s| unsafe { String::from_utf8_unchecked(s.to_vec()) })
456-
.collect();
457-
458-
// SAFETY: Unstable sort is faster and in this scenario it's also safe because we are
459-
// guaranteed to have unique candidates.
460-
result.par_sort_unstable();
461-
462-
result
432+
extract(blobs, |mut extractor| {
433+
extractor.extract_variables_from_css()
434+
})
463435
}
464436

465437
#[tracing::instrument(skip_all)]
466438
fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
439+
extract(blobs, |mut extractor| extractor.extract())
440+
}
441+
442+
#[tracing::instrument(skip_all)]
443+
fn extract<H>(blobs: Vec<Vec<u8>>, handle: H) -> Vec<String>
444+
where
445+
H: Fn(Extractor) -> Vec<Extracted> + std::marker::Sync,
446+
{
467447
let mut result: Vec<_> = blobs
468448
.par_iter()
469449
.flat_map(|blob| blob.par_split(|x| *x == b'\n'))
@@ -472,7 +452,7 @@ fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
472452
return None;
473453
}
474454

475-
let extracted = crate::extractor::Extractor::new(blob).extract();
455+
let extracted = handle(crate::extractor::Extractor::new(blob));
476456
if extracted.is_empty() {
477457
return None;
478458
}

crates/oxide/tests/scanner.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ mod scanner {
325325
("c.less", ""),
326326
]);
327327

328-
assert_eq!(files, vec!["index.html"]);
328+
assert_eq!(files, vec!["a.css", "index.html"]);
329329
assert_eq!(globs, vec!["*"]);
330330
assert_eq!(normalized_sources, vec!["**/*"]);
331331
}

integrations/cli/index.test.ts

+74
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,80 @@ test(
14681468
},
14691469
)
14701470

1471+
test(
1472+
'changes to CSS files should pick up new CSS variables (if any)',
1473+
{
1474+
fs: {
1475+
'package.json': json`
1476+
{
1477+
"dependencies": {
1478+
"tailwindcss": "workspace:^",
1479+
"@tailwindcss/cli": "workspace:^"
1480+
}
1481+
}
1482+
`,
1483+
'unrelated.module.css': css`
1484+
.module {
1485+
color: var(--color-blue-500);
1486+
}
1487+
`,
1488+
'index.css': css`
1489+
@import 'tailwindcss/theme';
1490+
@import 'tailwindcss/utilities';
1491+
`,
1492+
'index.html': html`<div class="flex"></div>`,
1493+
},
1494+
},
1495+
async ({ spawn, exec, fs, expect }) => {
1496+
// Generate the initial build so output CSS files exist on disk
1497+
await exec('pnpm tailwindcss --input ./index.css --output ./dist/out.css')
1498+
1499+
// NOTE: We are writing to an output CSS file which is not being ignored by
1500+
// `.gitignore` nor marked with `@source not`. This should not result in an
1501+
// infinite loop.
1502+
let process = await spawn(
1503+
'pnpm tailwindcss --input ./index.css --output ./dist/out.css --watch',
1504+
)
1505+
await process.onStderr((m) => m.includes('Done in'))
1506+
1507+
expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(`
1508+
"
1509+
--- ./dist/out.css ---
1510+
:root, :host {
1511+
--color-blue-500: oklch(0.623 0.214 259.815);
1512+
}
1513+
.flex {
1514+
display: flex;
1515+
}
1516+
"
1517+
`)
1518+
1519+
await fs.write(
1520+
'unrelated.module.css',
1521+
css`
1522+
.module {
1523+
color: var(--color-blue-500);
1524+
background-color: var(--color-red-500);
1525+
}
1526+
`,
1527+
)
1528+
await process.onStderr((m) => m.includes('Done in'))
1529+
1530+
expect(await fs.dumpFiles('./dist/*.css')).toMatchInlineSnapshot(`
1531+
"
1532+
--- ./dist/out.css ---
1533+
:root, :host {
1534+
--color-red-500: oklch(0.637 0.237 25.331);
1535+
--color-blue-500: oklch(0.623 0.214 259.815);
1536+
}
1537+
.flex {
1538+
display: flex;
1539+
}
1540+
"
1541+
`)
1542+
},
1543+
)
1544+
14711545
function withBOM(text: string): string {
14721546
return '\uFEFF' + text
14731547
}

integrations/postcss/next.test.ts

+97
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,100 @@ test(
257257
])
258258
},
259259
)
260+
261+
test(
262+
'changes to CSS files should pick up new CSS variables (if any)',
263+
{
264+
fs: {
265+
'package.json': json`
266+
{
267+
"dependencies": {
268+
"react": "^18",
269+
"react-dom": "^18",
270+
"next": "^14"
271+
},
272+
"devDependencies": {
273+
"@tailwindcss/postcss": "workspace:^",
274+
"tailwindcss": "workspace:^"
275+
}
276+
}
277+
`,
278+
'postcss.config.mjs': js`
279+
export default {
280+
plugins: {
281+
'@tailwindcss/postcss': {},
282+
},
283+
}
284+
`,
285+
'next.config.mjs': js`export default {}`,
286+
'app/layout.js': js`
287+
import './globals.css'
288+
289+
export default function RootLayout({ children }) {
290+
return (
291+
<html>
292+
<body>{children}</body>
293+
</html>
294+
)
295+
}
296+
`,
297+
'app/page.js': js`
298+
export default function Page() {
299+
return <div className="flex"></div>
300+
}
301+
`,
302+
'unrelated.module.css': css`
303+
.module {
304+
color: var(--color-blue-500);
305+
}
306+
`,
307+
'app/globals.css': css`
308+
@import 'tailwindcss/theme';
309+
@import 'tailwindcss/utilities';
310+
`,
311+
},
312+
},
313+
async ({ spawn, exec, fs, expect }) => {
314+
// Generate the initial build so output CSS files exist on disk
315+
await exec('pnpm next build')
316+
317+
// NOTE: We are writing to an output CSS file which is not being ignored by
318+
// `.gitignore` nor marked with `@source not`. This should not result in an
319+
// infinite loop.
320+
let process = await spawn(`pnpm next dev`)
321+
322+
let url = ''
323+
await process.onStdout((m) => {
324+
let match = /Local:\s*(http.*)/.exec(m)
325+
if (match) url = match[1]
326+
return Boolean(url)
327+
})
328+
329+
await process.onStdout((m) => m.includes('Ready in'))
330+
331+
await retryAssertion(async () => {
332+
let css = await fetchStyles(url)
333+
expect(css).toContain(candidate`flex`)
334+
expect(css).toContain('--color-blue-500:')
335+
expect(css).not.toContain('--color-red-500:')
336+
})
337+
338+
await fs.write(
339+
'unrelated.module.css',
340+
css`
341+
.module {
342+
color: var(--color-blue-500);
343+
background-color: var(--color-red-500);
344+
}
345+
`,
346+
)
347+
await process.onStdout((m) => m.includes('Compiled in'))
348+
349+
await retryAssertion(async () => {
350+
let css = await fetchStyles(url)
351+
expect(css).toContain(candidate`flex`)
352+
expect(css).toContain('--color-blue-500:')
353+
expect(css).toContain('--color-red-500:')
354+
})
355+
},
356+
)

integrations/utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ export function test(
321321
return [
322322
file,
323323
// Drop license comment
324-
content.replace(/[\s\n]*\/\*! tailwindcss .*? \*\/[\s\n]*/g, ''),
324+
content.replace(/[\s\n]*\/\*![\s\S]*?\*\/[\s\n]*/g, ''),
325325
]
326326
}),
327327
)

0 commit comments

Comments
 (0)