Ensure @tailwindcss/postcss rebuilds when the input CSS changes but its mtime is unchanged#20310
Conversation
… its mtime is unchanged
f08c9b8 to
70ccac2
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis change adds input CSS content tracking to the Tailwind PostCSS plugin cache. The plugin stores the last processed input CSS string, compares it with the current CSS text during rebuild checks, and switches to a full rebuild when they differ. The tests add a reusable helper and cover both concurrent execution and stable-mtime input changes. A changelog entry records the fix. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin
participant CacheEntry
Plugin->>Plugin: root.toString()
Plugin->>CacheEntry: compare with inputCss
CacheEntry-->>Plugin: mismatch detected
Plugin->>Plugin: set rebuildStrategy = full
Plugin->>CacheEntry: update inputCss
Related issues: None specified. Related PRs: None specified. Suggested labels: bug, postcss Suggested reviewers: None specified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
Confidence Score: 5/5This looks safe to merge.
Reviews (4): Last reviewed commit: "rename `cachedInputCss` to `inputCss`" | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/@tailwindcss-postcss/src/index.ts (1)
199-208: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
root.toString()re-serializes the input AST on every build.This runs on every
Once()invocation (i.e. every incremental rebuild, not just ones where the input actually changed), adding a full AST→string stringification cost to the hot rebuild path. For most entry files this is small, but for larger hand-authored CSS entry points it adds unnecessary work to every candidate-only rebuild in watch mode.Consider comparing against the raw source text already retained by PostCSS's
Input(root.source?.input.css) instead of re-stringifying the AST, which avoids the extra serialization work while still detecting the "input CSS changed but mtime didn't" case this PR targets.⚡ Proposed optimization
- let inputCss = root.toString() + let inputCss = root.source?.input.css ?? root.toString() if (context.cachedInputCss !== inputCss) { rebuildStrategy = 'full' context.cachedInputCss = inputCss }Please confirm
Input#cssreliably holds the original raw source text passed topostcss.parse()/process()across the PostCSS version this package depends on, since this is an undocumented-adjacent internal property in some PostCSS API references.packages/@tailwindcss-postcss/src/index.test.ts (1)
459-497: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest logic verified correct; minor duplication with existing helper.
Traced through the flow: without the fix, the second
run()call would keep reusing the compiler/scanner built from the first input (sincecontext.compiler ??= createCompiler()is a no-op once set, and mtime alone wouldn't trigger a full rebuild), so.secondwould not appear in the output — this test correctly exercises the regression.The
run()helper (Lines 469-477) duplicates the near-identical helper defined a few dozen lines earlier (Lines 425-433). Consider extracting a shared helper for both tests to reduce duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c7015ee2-c2e6-4a71-9eee-319756345c87
📒 Files selected for processing (3)
CHANGELOG.mdpackages/@tailwindcss-postcss/src/index.test.tspackages/@tailwindcss-postcss/src/index.ts
|
Pushed a follow-up for both nitpicks, swapped |
This information is already scoped to a `CacheEntry`
Summary
@tailwindcss/postcsschooses between an incremental and a full rebuild by comparing the mtimes of the entry file and its resolved@import/@config/@plugingraph. It never looks at the input CSS itself, so when that CSS is produced by an upstream tool (e.g. Sass) and passed to the plugin viaprocess(), it can change while thefromfile's mtime stays the same. The plugin then re-emits its previously cached output and silently drops the change.This stores the input CSS per cache entry and takes the existing full rebuild path when it differs from the previous compile, mirroring the fix the CLI watcher already has for changed input files.
Fixes #20307
Test plan
packages/@tailwindcss-postcss/src/index.test.tsthat compiles two different inputs for the same on-diskfromfile (unchanged mtime) and asserts the second compile reflects the new CSS.main(the second compile returns the stale first output) and passes with the fix.@tailwindcss/postcsspackage tests (all green) and checked formatting with Prettier.