fix(fs): ignore globs fail under dot-prefixed directories#768
fix(fs): ignore globs fail under dot-prefixed directories#768janvennemann wants to merge 2 commits into
Conversation
…den directories Fixes unjs#750
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChanges update package version to 1.17.4, remove automated publishing workflow, refactor the FS driver's ignore pattern matching to support dot-prefixed directories, adjust type exports, and add post-build generation of CommonJS and ES module type declaration files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
191-199:⚠️ Potential issue | 🟠 MajorRemove
esbuildfrom one of the twopnpmbuild-policy lists.
esbuildcurrently appears in bothignoredBuiltDependenciesandonlyBuiltDependencies. This conflicting configuration will cause inconsistent install-script behavior. Decide whetheresbuildshould be ignored or only-built, then remove it from the other list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 191 - 199, The package.json currently lists "esbuild" in both the ignoredBuiltDependencies and onlyBuiltDependencies arrays; remove the duplicate by deciding the intended policy and deleting "esbuild" from the other array so it appears in only one place. Open package.json, find the ignoredBuiltDependencies and onlyBuiltDependencies arrays, and remove the "esbuild" entry from whichever list is not the desired policy (leave it only in the intended list) to eliminate the conflicting configuration.
🧹 Nitpick comments (2)
test/drivers/localstorage.test.ts (1)
10-10: Optional cleanup: remove commented-out console forwarding line.This is fine functionally, but removing dead commented code will keep the test file easier to scan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/drivers/localstorage.test.ts` at line 10, Remove the dead commented-out line "jsdom.virtualConsole.sendTo(console);" from the test file—locate the commented forwarding call (the commented jsdom.virtualConsole.sendTo(console) line) and delete it to clean up the test and improve readability.test/drivers/session-storage.test.ts (1)
10-10: Prefer removing commented-out test setup instead of leaving it inline.If console forwarding is intentionally disabled, consider deleting this line rather than commenting it, to keep test setup clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/drivers/session-storage.test.ts` at line 10, Remove the commented-out console forwarding line (the jsdom.virtualConsole.sendTo(console) comment) from the test setup in session-storage.test.ts; if forwarding is intentionally disabled, delete the commented line instead of leaving it inline to keep the test file clean and avoid dead/commented code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@package.json`:
- Around line 191-199: The package.json currently lists "esbuild" in both the
ignoredBuiltDependencies and onlyBuiltDependencies arrays; remove the duplicate
by deciding the intended policy and deleting "esbuild" from the other array so
it appears in only one place. Open package.json, find the
ignoredBuiltDependencies and onlyBuiltDependencies arrays, and remove the
"esbuild" entry from whichever list is not the desired policy (leave it only in
the intended list) to eliminate the conflicting configuration.
---
Nitpick comments:
In `@test/drivers/localstorage.test.ts`:
- Line 10: Remove the dead commented-out line
"jsdom.virtualConsole.sendTo(console);" from the test file—locate the commented
forwarding call (the commented jsdom.virtualConsole.sendTo(console) line) and
delete it to clean up the test and improve readability.
In `@test/drivers/session-storage.test.ts`:
- Line 10: Remove the commented-out console forwarding line (the
jsdom.virtualConsole.sendTo(console) comment) from the test setup in
session-storage.test.ts; if forwarding is intentionally disabled, delete the
commented line instead of leaving it inline to keep the test file clean and
avoid dead/commented code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 051df150-abdd-4307-9c4f-e8d36bf97b20
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/workflows/publish.yml.gitignoreCHANGELOG.mdbuild.config.tspackage.jsonsrc/drivers/fs.tssrc/drivers/netlify-blobs.tssrc/drivers/s3.tstest/drivers/azure-key-vault.test.tstest/drivers/localstorage.test.tstest/drivers/session-storage.test.ts
💤 Files with no reviewable changes (1)
- .github/workflows/publish.yml
1 participant
Summary
Backport fix for v1. The v2 alpha already resolved this by switching to
node:path'smatchesGlobon relative paths.**/node_modules/**and**/.git/**) usesanymatch, which delegates topicomatchpicomatchdoes not match**across dot-prefixed path segments by default (e.g..codex,.claude)node_modulesfilesThis is particularly problematic when using git worktrees created by Claude Code or Codex, which place worktrees under
~/.claude/or~/.codex/respectively. Any Nuxt project with DevTools enabled ends up watching all ofnode_modulesbecause the**/node_modules/**ignore pattern can't match through the dot-prefixed ancestor.The fix passes
{ dot: true }toanymatchso**crosses dot-prefixed segments.Fixes #750
Related: nuxt/nuxt#33300
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores