Detect new files added during dev session in file watcher#7491
Open
isaacroldan wants to merge 3 commits intomainfrom
Open
Detect new files added during dev session in file watcher#7491isaacroldan wants to merge 3 commits intomainfrom
isaacroldan wants to merge 3 commits intomainfrom
Conversation
The file watcher resolved file ownership statically at start time using extension.watchedFiles(), which expanded globs into a literal list of paths. Files created at runtime weren't in that list, so chokidar's add events were dropped by the "not watched by any extension" check. Now attribute unknown 'add' events to an owning extension by directory containment and watch-pattern matching, register the new path in the dynamic map, and short-circuit shouldIgnoreEvent for paths already tracked. Adds ExtensionInstance.watchPatterns() exposing the raw paths + ignore patterns.
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a dev-session gap where files created after FileWatcher.start() were ignored because extension ownership was determined from a one-time snapshot of ExtensionInstance.watchedFiles(). It adds runtime attribution for previously-unknown add events by matching the new file against each extension’s directory and raw watch patterns, then registers the discovered file for fast subsequent event handling.
Changes:
- Add
ExtensionInstance.watchPatterns()(and refactorwatchedFiles()to use it) so the watcher can pattern-match runtime-created files without relying on a glob snapshot. - Enhance
FileWatcher.handleFileEventto discover owning extension(s) for unknownaddevents and register them intoextensionWatchedFiles. - Update ignore logic to accept events for paths already registered for a handle; add tests covering runtime-added files and follow-up change events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/app/src/cli/services/dev/app-events/file-watcher.ts | Adds runtime “owner discovery” for unknown add events and adjusts ignore logic for runtime-registered paths. |
| packages/app/src/cli/services/dev/app-events/file-watcher.test.ts | Adds test coverage for runtime file creation attribution and subsequent updates. |
| packages/app/src/cli/models/extensions/extension-instance.ts | Introduces watchPatterns() and centralizes default ignore patterns; refactors watchedFiles() to use it. |
| .changeset/file-watcher-detect-new-files.md | Adds a patch changeset documenting the dev-session behavior improvement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+264
to
+273
| // For 'add' events on paths we don't yet track, try to attribute them to an | ||
| // existing extension by directory containment + watch pattern matching. This | ||
| // is what allows files created during a running dev session to be picked up. | ||
| if (isUnknownExtension && event === 'add' && !isExtensionToml) { | ||
| const discovered = this.discoverFileOwners(normalizedPath) | ||
| if (discovered.size > 0) { | ||
| this.extensionWatchedFiles.set(normalizedPath, discovered) | ||
| affectedHandles = discovered | ||
| isUnknownExtension = false | ||
| } |
4 tasks
knip flags the constant as an unused export since it's only consumed inside extension-instance.ts. Make it a module-local const.
Two review-comment fixes from #7491: - shouldIgnoreEvent looked up event.path raw, but extensionWatchedFiles is keyed by normalizePath(file). On Windows, chokidar emits backslash-separated paths and the lookup would miss, causing runtime-discovered files to be re-filtered by the static-list check. Normalize before the lookup. - Runtime discovery added entries to extensionWatchedFiles but never removed them on unlink. In a long-running dev session the map grew unbounded with stale ownership data. Delete the normalized path from the map after pushing file_deleted; subsequent timeouts for other handles are no-ops on the now-missing key.
Contributor
|
/snapit |
Contributor
|
🫰✨ Thanks @melissaluu! Your snapshot has been published to npm. Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260507161656Caution After installing, validate the version by running |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes https://github.com/shop/issues-admin-extensibility/issues/2461
The file watcher resolves which extension owns a path via
extensionWatchedFiles, a map built once atstart()from each extension'swatchedFiles()— a globbed snapshot. Paths created at runtime aren't in the snapshot, so theiraddevents get dropped by the not watched by any extension early-return inhandleFileEvent.This change attributes unknown
addevents to an owning extension at event time:ExtensionInstance.watchPatterns()returning the rawpaths+ignorepatterns fromdevSessionWatchConfig(or defaults).watchedFiles()is refactored to call it.FileWatcher.handleFileEvent: whenaffectedHandlesis empty for anaddevent, look up the owning extension(s) by directory containment and pattern-match the path againstwatchPatterns(). If accepted, register inextensionWatchedFilesso subsequentchange/unlinkevents on the same path are O(1).shouldIgnoreEventshort-circuits to accept when the path is already registered for the handle, so subsequent events on a runtime-discovered file aren't dropped by the static-list check.Why not add a separate "folder" API to extensions: chokidar already watches
fullExtensionDirectoriesrecursively today, so the new files reach the watcher — the bug is in attribution, not in what's being watched.Test plan
file-watcher.test.tstests pass (19/19)extension-instance.test.tstests pass (42/42)nx type-checkpassesnx lintpassesfile_created; runtime-added file outside any extension → ignored; subsequentchangeon a runtime-discovered file →file_updatedshopify app dev, create a new file inside an extension at runtime, verify the dev session picks it up🤖 Generated with Claude Code