feat: add watch-ignore flag#8271
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a repeatable --watch-ignore option to the netlify dev CLI and documents examples. The flag values are collected into an array, passed from dev/serve commands into startProxyServer → startProxy → edge-functions initializer, and into EdgeFunctionsRegistryImpl as resolved publishDir and watchIgnore entries. The registry builds watcher ignore rules (directory regexes, exact-file ignores, import map exclusion, and an extension filter). Unit tests validate ignore rule shapes and path resolution. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/commands/dev.md`:
- Line 41: The sentence describing the watch-ignore flag uses incorrect sentence
casing after a period; update the second sentence in the `watch-ignore` flag
description so it begins with a capital letter ("Can be specified multiple
times.") and ensure the rest of the sentence remains unchanged; locate the
`watch-ignore` flag description text and correct its casing.
In `@src/lib/edge-functions/registry.ts`:
- Around line 747-752: The inline explanatory comments around the ignore-pattern
regex should be removed and intent encoded via a descriptive constant: extract
the regex /\.(?!(js|mjs|cjs|ts|tsx|jsx|json|wasm)$)[^./]+$/i into a clearly
named constant (e.g., NON_IMPORTABLE_EXT_PATTERN or
IGNORE_NON_IMPORTABLE_EXTENSIONS) in src/lib/edge-functions/registry.ts and
replace the inline literal with that constant where used; ensure the constant
name documents the behavior (ignores files with non-importable extensions) so
the comments are no longer needed and update any surrounding code references to
use the new symbol (the regex usage site and any related arrays/configs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1ef2ea4-3d58-4e6c-9278-19dfe664b208
📒 Files selected for processing (8)
docs/commands/dev.mdsrc/commands/dev/dev.tssrc/commands/dev/index.tssrc/commands/serve/serve.tssrc/lib/edge-functions/proxy.tssrc/lib/edge-functions/registry.tssrc/utils/proxy-server.tssrc/utils/proxy.ts
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)
tests/unit/lib/edge-functions/watch-ignore.test.ts (1)
1-285:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix Prettier formatting issues before merge.
The pipeline reports that this file has code style issues that Prettier would fix. Run
npm run formatto automatically fix these formatting issues.#!/bin/bash # Description: Show what Prettier would change npx prettier --check tests/unit/lib/edge-functions/watch-ignore.test.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 1 - 285, This test file has Prettier formatting violations; run the project's formatter (npm run format or npx prettier --write) on the failing file and commit the changes so imports, spacing, and trailing commas match project style; locate the test module by looking for the describe blocks (e.g., "toIgnoredRegex", "toIgnoredEntry (watchIgnore entries)"), helper functions makeRegistry and captureIgnored, and the EdgeFunctionsRegistryImpl usage to verify formatting changes and re-run the test pipeline.
🧹 Nitpick comments (5)
tests/unit/lib/edge-functions/watch-ignore.test.ts (5)
23-24: ⚡ Quick winRemove explanatory comment per coding guidelines.
The comment explains what the code does, which violates the guideline: "Never write comments on what the code does; make the code clean and self-explanatory instead." Consider renaming the function to
makeRegistryBypassingConstructoror restructuring the code to be self-explanatory. As per coding guidelines, code in.tsfiles should be clean and self-explanatory without comments describing what it does.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 23 - 24, Remove the explanatory comment and make the intent explicit in code: delete the two-line comment and either rename the helper that creates a partial registry to makeRegistryBypassingConstructor (and update all references) or extract the Object.create + field population into a clearly named function used by setupWatcherForDirectory tests; ensure the new name and/or function parameters describe why the constructor is bypassed (reference setupWatcherForDirectory and the helper that builds the registry) so no comment is necessary.
194-208: 💤 Low valueKnown limitation comment violates guideline but provides valuable context.
The multi-line comment (lines 195-198) documents a known limitation of the extension filter regex. While this violates the guideline "Never write comments on what the code does; make the code clean and self-explanatory instead," it provides important context about non-obvious behavior. Consider moving this documentation to the test name (e.g.,
test('matches dotted directory names as limitation of extension filter')) or to external documentation. As per coding guidelines, TypeScript files should avoid explanatory comments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 194 - 208, The multi-line explanatory comment inside the test should be removed and the same context moved into the test name or external docs; update the test declaration (the test with the body that calls makeRegistry and captureIgnored and assigns extensionFilter) to include the limitation in its description (e.g., rename the test string to mention "dotted directory names" or "limitation of extension filter"), delete the in-test explanatory comment, and ensure the test still references makeRegistry, captureIgnored and extensionFilter unchanged so behavior remains identical.
210-221: 💤 Low valueRemove explanatory comment.
Line 211's comment explains what the test verifies. Per coding guidelines, "Never write comments on what the code does; make the code clean and self-explanatory instead." The test name and assertions should be self-explanatory. As per coding guidelines, avoid comments describing test logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 210 - 221, Remove the explanatory inline comment inside the test 'does not ignore a file nested under a dotted directory name' — specifically delete the comment that starts with "Even though \"/project/v2.0\" itself matches, a .ts file inside it does not." located directly above the registry setup; leave the test body, makeRegistry usage, captureIgnored call and the extensionFilter/assertion unchanged so the test name and assertions remain self-explanatory.
158-158: 💤 Low valueConsider removing explanatory comment.
The comment explains the expected array structure. Per coding guidelines, "Never write comments on what the code does; make the code clean and self-explanatory instead." Consider making this part of the test assertion or removing it. As per coding guidelines, TypeScript code should be self-explanatory without comments describing structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` at line 158, Remove the inline explanatory comment "// servePath regex, publishDir regex, internalImportMapPath string, extension regex" from tests/unit/lib/edge-functions/watch-ignore.test.ts and instead make the expectation self-describing by asserting the array shape directly (e.g., destructure the returned array into named variables or assert each index with clear expect statements) so the test documents the structure without an explanatory comment.
50-51: 💤 Low valueConsider removing explanatory comment.
While this comment explains the testing strategy rather than code logic, the guideline states "Never write comments on what the code does; make the code clean and self-explanatory instead." The test structure and names could convey this information without the comment. As per coding guidelines, TypeScript files should avoid comments explaining code behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 50 - 51, Remove the explanatory comment and make the test self-descriptive: delete the two-line comment above the test, and if helpful rename the test or assertions to clarify intent (e.g., reference setupWatcherForDirectory and that it passes an ignored array into watchDebounced) so the behavior is clear from code/test names alone; ensure references to watchDebounced and the ignored array remain explicit in the test body.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/unit/lib/edge-functions/watch-ignore.test.ts`:
- Around line 1-285: This test file has Prettier formatting violations; run the
project's formatter (npm run format or npx prettier --write) on the failing file
and commit the changes so imports, spacing, and trailing commas match project
style; locate the test module by looking for the describe blocks (e.g.,
"toIgnoredRegex", "toIgnoredEntry (watchIgnore entries)"), helper functions
makeRegistry and captureIgnored, and the EdgeFunctionsRegistryImpl usage to
verify formatting changes and re-run the test pipeline.
---
Nitpick comments:
In `@tests/unit/lib/edge-functions/watch-ignore.test.ts`:
- Around line 23-24: Remove the explanatory comment and make the intent explicit
in code: delete the two-line comment and either rename the helper that creates a
partial registry to makeRegistryBypassingConstructor (and update all references)
or extract the Object.create + field population into a clearly named function
used by setupWatcherForDirectory tests; ensure the new name and/or function
parameters describe why the constructor is bypassed (reference
setupWatcherForDirectory and the helper that builds the registry) so no comment
is necessary.
- Around line 194-208: The multi-line explanatory comment inside the test should
be removed and the same context moved into the test name or external docs;
update the test declaration (the test with the body that calls makeRegistry and
captureIgnored and assigns extensionFilter) to include the limitation in its
description (e.g., rename the test string to mention "dotted directory names" or
"limitation of extension filter"), delete the in-test explanatory comment, and
ensure the test still references makeRegistry, captureIgnored and
extensionFilter unchanged so behavior remains identical.
- Around line 210-221: Remove the explanatory inline comment inside the test
'does not ignore a file nested under a dotted directory name' — specifically
delete the comment that starts with "Even though \"/project/v2.0\" itself
matches, a .ts file inside it does not." located directly above the registry
setup; leave the test body, makeRegistry usage, captureIgnored call and the
extensionFilter/assertion unchanged so the test name and assertions remain
self-explanatory.
- Line 158: Remove the inline explanatory comment "// servePath regex,
publishDir regex, internalImportMapPath string, extension regex" from
tests/unit/lib/edge-functions/watch-ignore.test.ts and instead make the
expectation self-describing by asserting the array shape directly (e.g.,
destructure the returned array into named variables or assert each index with
clear expect statements) so the test documents the structure without an
explanatory comment.
- Around line 50-51: Remove the explanatory comment and make the test
self-descriptive: delete the two-line comment above the test, and if helpful
rename the test or assertions to clarify intent (e.g., reference
setupWatcherForDirectory and that it passes an ignored array into
watchDebounced) so the behavior is clear from code/test names alone; ensure
references to watchDebounced and the ignored array remain explicit in the test
body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b70ab2b3-2db0-4d50-bda1-4787e816291c
📒 Files selected for processing (1)
tests/unit/lib/edge-functions/watch-ignore.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/lib/edge-functions/watch-ignore.test.ts (1)
157-160: ⚡ Quick winRemove behavior-describing inline comment and make the code self-explanatory.
At Line 158, replace index-based access with named destructuring so the intent is clear without a comment.
Suggested refactor
- // servePath regex, publishDir regex, internalImportMapPath string - const importMapEntry = ignored[2] + const [, , importMapEntry] = ignoredAs per coding guidelines, "
**/*.{ts,tsx,js,jsx}: Never write comments on what the code does; make the code clean and self-explanatory instead".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/lib/edge-functions/watch-ignore.test.ts` around lines 157 - 160, Replace the index-based access to the captured array from captureIgnored with named destructuring to make intent explicit: destructure the returned ignored array (from captureIgnored) into meaningful names (e.g., servePathRegex, publishDirRegex, internalImportMapPath) and use internalImportMapPath instead of importMapEntry for the typeof assertion, then remove the inline comment; this clarifies intent in the test without changing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/lib/edge-functions/watch-ignore.test.ts`:
- Around line 165-166: Prettier formatting failed in the test file—run Prettier
to fix formatting for the describe block named "constructor path resolution"
(and the rest of the file), apply the formatted changes, and commit them so CI
formatting checks pass; ensure you use the project's Prettier config (e.g., via
npx prettier --write . or your IDE) before pushing.
---
Nitpick comments:
In `@tests/unit/lib/edge-functions/watch-ignore.test.ts`:
- Around line 157-160: Replace the index-based access to the captured array from
captureIgnored with named destructuring to make intent explicit: destructure the
returned ignored array (from captureIgnored) into meaningful names (e.g.,
servePathRegex, publishDirRegex, internalImportMapPath) and use
internalImportMapPath instead of importMapEntry for the typeof assertion, then
remove the inline comment; this clarifies intent in the test without changing
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d29bbfe2-01aa-4fc9-b4b5-de1bfcbf5822
📒 Files selected for processing (2)
src/lib/edge-functions/registry.tstests/unit/lib/edge-functions/watch-ignore.test.ts
💤 Files with no reviewable changes (1)
- src/lib/edge-functions/registry.ts
1 participant
Summary
Fixes #7763 (https://answers.netlify.com/t/157384/)
TL;DR: The issue is caused because the user has too many files in their repo, causing the watchers in the CLI cause an EMFILE error on MacOS.
This PR does the following:
this.servePathfor Edge Functions. Chokidar doesn't accept globs in v4, so/**was incorrect - we now convert it to RegExpublishDirby default (only if it's not the same asprojectDir) as it shouldn't be possible to import anything from thepublishDirinto Edge Functions (debatable though)devcommand:--watch-ignoreto allow users to configure any additional ignore paths (just excludingpublishDirwasn't enough in case of the above user as they had over 11k files in theirsrcfolder.For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.