Skip to content

Swap chokidar for @parcel/watcher in local-mount#55

Merged
willwashburn merged 8 commits intomainfrom
parcel/watcher
Apr 21, 2026
Merged

Swap chokidar for @parcel/watcher in local-mount#55
willwashburn merged 8 commits intomainfrom
parcel/watcher

Conversation

@willwashburn
Copy link
Copy Markdown
Member

@willwashburn willwashburn commented Apr 21, 2026

Replace chokidar with @parcel/watcher in packages/local-mount. Rework auto-sync to use parcel's subscribe API, add per-path debounce (default 50ms) instead of chokidar's awaitWriteFinish, and introduce buildIgnoreGlobs to precompute watcher ignore patterns. Ensure proper unsubscribe handling and drain pending debounces on stop(), update docs/tests/package.json and lockfile, and bump package versions to 0.2.2.


Open in Devin Review

Replace chokidar with @parcel/watcher in packages/local-mount. Rework auto-sync to use parcel's subscribe API, add per-path debounce (default 50ms) instead of chokidar's awaitWriteFinish, and introduce buildIgnoreGlobs to precompute watcher ignore patterns. Ensure proper unsubscribe handling and drain pending debounces on stop(), update docs/tests/package.json and lockfile, and bump package versions to 0.2.2.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Replaces chokidar with @parcel/watcher for packages/local-mount auto-sync to improve watcher behavior and teardown, while updating docs, tests, and dependency metadata accordingly.

Changes:

  • Swap the auto-sync watcher implementation from chokidar to @parcel/watcher subscriptions and add per-path debouncing.
  • Add buildIgnoreGlobs() to precompute ignore globs for watcher configuration.
  • Update local-mount docs/tests and bump versions/dependencies (including lockfile updates).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/local-mount/src/symlink-mount.ts Updates public docs/comments to reflect the new watcher backend.
packages/local-mount/src/auto-sync.ts Reworks auto-sync around @parcel/watcher, adds debounced sync scheduling and ignore glob building, updates stop/unsubscribe behavior.
packages/local-mount/src/auto-sync.test.ts Updates test helper comment to remove chokidar-specific language.
packages/local-mount/package.json Replaces chokidar dependency with @parcel/watcher.
packages/local-mount/README.md Updates AutoSync documentation to describe debounce and new watcher backend.
package-lock.json Reflects dependency and version updates, including @parcel/watcher and package version bumps.
.trajectories/index.json Adds/updates trajectory metadata for this change.
.trajectories/completed/2026-04/traj_iuzm83ogm43k.md Adds trajectory summary for the completed work.
.trajectories/completed/2026-04/traj_iuzm83ogm43k.json Adds structured trajectory details for the completed work.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/**
* Per-path event debounce in ms. Rapid watcher events for the same path
* are coalesced into a single sync. Default: 50.
*/
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSyncOptions.writeFinishMs is now used as an event debounce (default 50ms), so the option name no longer reflects its behavior (it reads like chokidar's awaitWriteFinish). Consider adding a correctly named option (e.g. debounceMs) and treating writeFinishMs as a deprecated alias for backwards compatibility, or renaming the option throughout if a breaking change is acceptable.

Suggested change
*/
*/
debounceMs?: number;
/**
* @deprecated Use `debounceMs` instead.
* Backwards-compatible alias for the per-path event debounce in ms.
*/

Copilot uses AI. Check for mistakes.
Comment on lines +187 to +202
function buildIgnoreGlobs(ctx: AutoSyncContext): string[] {
// @parcel/watcher matches globs against absolute paths via globset. For each
// excluded directory name, ignore both the directory itself and everything
// beneath it, anywhere under the watched root. The `isExcluded` predicate is
// driven by a Set of directory names, so we probe a small set of common
// exclusions rather than introspecting it. The in-handler `isSyncCandidate`
// filter is authoritative — this is just a perf hint so the watcher doesn't
// recurse into heavy trees like node_modules or .git.
const globs: string[] = [];
const candidates = ['.git', 'node_modules', 'dist', 'build', '.next', '.cache'];
for (const name of candidates) {
if (ctx.isExcluded(name)) {
globs.push(`**/${name}`, `**/${name}/**`);
}
}
return globs;
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildIgnoreGlobs() only ignores a hard-coded list of directory names. This means user-specified excludeDirs (and any excluded prefixes like foo/bar) will no longer be pruned at the watcher level, potentially causing @parcel/watcher to traverse/watch large excluded subtrees and hit OS watcher limits. Consider passing the actual exclude set / exclude globs through AutoSyncContext so the ignore list can reflect all exclusions, not just common ones.

Suggested change
function buildIgnoreGlobs(ctx: AutoSyncContext): string[] {
// @parcel/watcher matches globs against absolute paths via globset. For each
// excluded directory name, ignore both the directory itself and everything
// beneath it, anywhere under the watched root. The `isExcluded` predicate is
// driven by a Set of directory names, so we probe a small set of common
// exclusions rather than introspecting it. The in-handler `isSyncCandidate`
// filter is authoritative — this is just a perf hint so the watcher doesn't
// recurse into heavy trees like node_modules or .git.
const globs: string[] = [];
const candidates = ['.git', 'node_modules', 'dist', 'build', '.next', '.cache'];
for (const name of candidates) {
if (ctx.isExcluded(name)) {
globs.push(`**/${name}`, `**/${name}/**`);
}
}
return globs;
interface AutoSyncContextWithWatcherExcludes extends AutoSyncContext {
excludeDirs?: Iterable<string>;
excludeGlobs?: Iterable<string>;
}
function escapeGlobLiteral(value: string): string {
return value.replace(/([*?[\]{}()!+@\\])/g, '\\$1');
}
function normalizeExcludePath(value: string): string {
return value
.replace(/\\/g, '/')
.split('/')
.filter(Boolean)
.join('/');
}
function addIgnoredDirGlobs(globs: Set<string>, value: string): void {
const normalized = normalizeExcludePath(value);
if (!normalized) return;
const escaped = normalized
.split('/')
.map((part) => escapeGlobLiteral(part))
.join('/');
globs.add(`**/${escaped}`);
globs.add(`**/${escaped}/**`);
}
function buildIgnoreGlobs(ctx: AutoSyncContext): string[] {
// @parcel/watcher matches globs against absolute paths via globset. Keep the
// legacy common-directory fast paths, but also honor any concrete exclusions
// provided on the context so the watcher can prune user-configured excluded
// trees, including nested prefixes like "foo/bar".
const globs = new Set<string>();
const candidates = ['.git', 'node_modules', 'dist', 'build', '.next', '.cache'];
for (const name of candidates) {
if (ctx.isExcluded(name)) {
addIgnoredDirGlobs(globs, name);
}
}
const watcherCtx = ctx as AutoSyncContextWithWatcherExcludes;
for (const excludedDir of watcherCtx.excludeDirs ?? []) {
addIgnoredDirGlobs(globs, excludedDir);
}
for (const glob of watcherCtx.excludeGlobs ?? []) {
if (glob) globs.add(glob);
}
return [...globs];

Copilot uses AI. Check for mistakes.
Comment thread .trajectories/index.json Outdated
"status": "completed",
"startedAt": "2026-04-20T20:35:15.759Z",
"completedAt": "2026-04-20T20:58:15.412Z",
"path": "/Users/will/Projects/relayfile/.trajectories/completed/2026-04/traj_iuzm83ogm43k.json"
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry embeds an absolute local filesystem path (/Users/...) in the repo. This is environment-specific and can leak developer info; consider storing a repo-relative path (or omitting the path field) so the file is portable across machines.

Suggested change
"path": "/Users/will/Projects/relayfile/.trajectories/completed/2026-04/traj_iuzm83ogm43k.json"
"path": ".trajectories/completed/2026-04/traj_iuzm83ogm43k.json"

Copilot uses AI. Check for mistakes.
},
"commits": [],
"filesChanged": [],
"projectId": "/Users/will/Projects/relayfile",
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

projectId contains an absolute local path (/Users/...), which is machine-specific and can leak developer information in published history. Consider using a repo identifier (e.g. name/URL) or a relative path, or removing this field from committed artifacts.

Suggested change
"projectId": "/Users/will/Projects/relayfile",
"projectId": "relayfile",

Copilot uses AI. Check for mistakes.
/** Full-reconcile interval as a safety net. Default: 10_000 ms. */
scanIntervalMs?: number;
/** chokidar `awaitWriteFinish` stability threshold. Default: 200 ms. */
/** Per-path event debounce in ms. Default: 50 ms. */
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option is documented as a debounce but still named writeFinishMs, which reads like chokidar’s old awaitWriteFinish setting. If you keep the legacy option name for compatibility, consider explicitly noting that writeFinishMs now controls debounce; otherwise update the public option name (and examples) to something like debounceMs.

Suggested change
/** Per-path event debounce in ms. Default: 50 ms. */
/** Legacy option name retained for compatibility; controls per-path event debounce. Default: 50 ms. */

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Rename the AutoSync option from writeFinishMs to debounceMs across the package. Updated the public interface, implementation (default still 50ms), README examples, and all tests to use the new option name to keep semantics unchanged.
Add initial CHANGELOG.md files for CLI, core, SDK (typescript), local-mount, and file-observer packages and populate them with recent release history and an Unreleased section. Update README to document where package changelogs live and add a short process for how PRs should add entries and how releases should be cut (Keep a Changelog format and compare-link guidance).
devin-ai-integration[bot]

This comment was marked as resolved.

Add a finalize-changelogs script and wire it into the publish workflow so releases rename each package's `## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, insert a fresh `## [Unreleased]` section, update compare links and append a release link. The workflow skips finalization for prereleases and dry runs, and now includes CHANGELOG.md files in the build artifact list and in the release commit. Update README to document the automated finalization behavior and the script's role.
devin-ai-integration[bot]

This comment was marked as resolved.

Use Promise.allSettled when subscribing to mount/project watchers in startAutoSync so each subscribe outcome is tracked independently. If one subscribe rejects, the fulfilled subscription is unsubscribed and cleared before re-throwing the original error. Also switch teardown to Promise.allSettled to avoid unhandled rejections and add a CHANGELOG entry documenting the fix.
Prevent timers from running after AutoSync is stopped by adding a `stopped` flag and refusing to schedule new debounces once `stop()` begins. Move clearing of `pendingDebounces` until after watcher unsubscribes resolve so any events delivered during the unsubscribe await are collected and cancelled before `stop()` returns. Also update CHANGELOG with the behavior/fix so callers can rely on “stopped means quiesced.”
@willwashburn willwashburn merged commit 1bf41ec into main Apr 21, 2026
6 checks passed
@willwashburn willwashburn deleted the parcel/watcher branch April 21, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants