fix(cli): exclude noise directories from --watch file watcher - #20388
fix(cli): exclude noise directories from --watch file watcher#20388SomSamantray wants to merge 4 commits into
Conversation
Prevent the CLI's file watcher from recursing into node_modules, .git, .hg, and .svn by passing an ignore option to @parcel/watcher's subscribe(). Previously these directories were watched with no filtering, causing spurious rebuild cycles and contributing to the reported CPU-pegging hang on cold Watchman starts against large, uncached trees. Fixes tailwindlabs#17246
Adds coverage for the ignore-option fix: asserts that changes inside node_modules or .git no longer trigger a watcher rebuild cycle, while confirming the watcher still reacts to real source changes. Could not be executed in this environment (requires a full monorepo build producing dist/ tarballs, which needs the native Oxide crate); the underlying mechanism was independently verified against the installed @parcel/watcher@2.6.0 binary directly.
Two independent code-review passes (correctness and reliability) found that a blanket node_modules/.git ignore on every watcher.subscribe() call can silently break watch-mode rebuilds for an explicit @source path living inside node_modules, if that path's own watch root gets collapsed into a broader ancestor by the existing dedup step -- since @parcel/watcher matches ignore globs relative to the watched root. watchIgnoreFor() now skips ignoring a noise segment for a given directory when another originally-requested directory got collapsed into it and lives inside that segment. Verified empirically against the installed @parcel/watcher@2.6.0 binary directly (0 events before this fix for the nested case, 2 after -- matching pre-regression behavior), and with isolated unit tests of the pure ignore-list logic. Also fixes a test timeout that wasn't scaled for Windows like the rest of the suite (per testing-reviewer), and adds e2e coverage for the nested-@source-inside-node_modules scenario.
WalkthroughThe CLI watcher now ignores default noisy directory segments and computes ignore globs for each watched root. It preserves explicitly requested nested sources and tracked full-rebuild paths during watcher recreation. Integration tests verify that changes in 🚥 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: 4/5The PR is not yet safe to merge because tracking an ordinary dependency inside node_modules disables that directory's ignore rule for the entire project watcher. The dependency fix observes imported files again, but it does so by removing the project-root node_modules exclusion wholesale, allowing unrelated dependency-tree activity to trigger watcher processing and recreating the CPU and rebuild behavior the PR targets. Files Needing Attention: packages/@tailwindcss-cli/src/commands/build/index.ts Reviews (2): Last reviewed commit: "fix(cli): don't ignore noise dirs contai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile review on tailwindlabs#20388 found that the ignore-list exemption only considered scanner.normalizedSources (explicit @source paths), not fullRebuildPaths -- compiler dependencies discovered via @import (e.g. tailwindcss itself, or any imported CSS package resolved from node_modules). If such a dependency's directory was nested under a watched root with no other reason to be exempted, editing it would no longer trigger a rebuild. createWatchers() now also accepts the current fullRebuildPaths so watchIgnoreFor() can exempt a noise segment for a root when a tracked dependency lives inside it, the same way it already does for explicit source directories. Verified with isolated unit tests of the ignore logic covering both the reported regression and a control case (unrelated dependency paths must not spuriously exempt anything), plus new e2e coverage in integrations/cli/index.test.ts.
| let marker = `/${segment}/` | ||
| return !allRequestedDirs.some( | ||
| (other) => other !== dir && other.startsWith(`${dir}/`) && `${other}/`.includes(marker), |
There was a problem hiding this comment.
When a normal Tailwind import or another compiler dependency resolves inside node_modules, watchIgnoreFor removes the node_modules ignore glob from the entire project-root subscription. Unrelated file events anywhere in that directory then reach the callback and initiate rebuild processing, restoring the spurious rebuilds and high CPU usage this change is intended to prevent.
Knowledge Base Used: Build tool integrations
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integrations/cli/index.test.ts (1)
451-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise watcher recreation before the dependency update.
This test changes
base.csswhile the initial watchers are active. It does not test the recreated watcher path inpackages/@tailwindcss-cli/src/commands/build/index.tsat Line 401. A regression that dropsfullRebuildPathsduring watcher recreation will pass.Modify
src/index.cssafter startup, wait for the full rebuild, then modifybase.css.Proposed test addition
await fs.expectFileToContain('dist/out.css', [ css` .imported-marker { color: red; } `, ]) + await fs.write( + 'src/index.css', + css` + `@import` 'tailwindcss/utilities'; + `@import` '../node_modules/my-base-styles/base.css'; + `, + ) + await process.onStderr((m) => m.includes('Done in')) + await fs.write( 'node_modules/my-base-styles/base.css', css`
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8abc1a82-0fb1-444e-9423-ecf27104b631
📒 Files selected for processing (2)
integrations/cli/index.test.tspackages/@tailwindcss-cli/src/commands/build/index.ts
1 participant
Summary
@tailwindcss/cli --watchcan peg the CPU because its file watcher recurses intonode_modulesand.gitwith no filtering, causing spurious rebuild cycles. This adds a targetedignoreoption to@parcel/watcher'ssubscribe()calls to exclude those directories, refined so it doesn't break an explicit@sourcepath that legitimately lives insidenode_modules.Related: #17246
Test plan
Verified the ignore mechanism directly against the installed
@parcel/watcherbinary (spurious rebuilds eliminated; an explicit@source-into-node_modulescase still triggers rebuilds correctly) and added e2e coverage inintegrations/cli/index.test.ts.