Skip to content

Commit fa5b4a3

Browse files
authored
Merge pull request #8993 from Shopify/codex/app-dev-watcher-static-ignores
Exclude build output and temporary files from app dev watchers
2 parents 1a8c51c + bdb2926 commit fa5b4a3

3 files changed

Lines changed: 230 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/app': patch
3+
---
4+
5+
Reduce `app dev` file watching by excluding Cargo `target` directories, Function build output, `.shopify` files, and editor temporary files.

‎packages/app/src/cli/services/dev/app-events/file-watcher.test.ts‎

Lines changed: 175 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import {
99
} from '../../../models/app/app.test-data.js'
1010
import {flushPromises} from '@shopify/cli-kit/node/promises'
1111
import {describe, expect, test, vi} from 'vitest'
12-
import chokidar from 'chokidar'
12+
import chokidar, {FSWatcher} from 'chokidar'
1313
import {AbortController} from '@shopify/cli-kit/node/abort'
1414
import {inTemporaryDirectory, mkdir, writeFile, fileExistsSync} from '@shopify/cli-kit/node/fs'
15-
import {joinPath, normalizePath} from '@shopify/cli-kit/node/path'
15+
import {dirname, joinPath, normalizePath} from '@shopify/cli-kit/node/path'
1616
import {extractImportPathsRecursively} from '@shopify/cli-kit/node/import-extractor'
1717

1818
// Mock the import extractor - will be configured per test
@@ -22,6 +22,8 @@ vi.mock('@shopify/cli-kit/node/import-extractor', () => ({
2222
extractJSImports: vi.fn(() => []),
2323
}))
2424

25+
const realChokidarWatch = chokidar.watch.bind(chokidar)
26+
2527
// Helper to mock watchedFiles for extensions
2628
function mockExtensionWatchedFiles(extension: any, files: string[] = []) {
2729
vi.spyOn(extension, 'watchedFiles').mockReturnValue(files)
@@ -221,7 +223,16 @@ describe('file-watcher events', () => {
221223

222224
// Then
223225
expect(watchSpy).toHaveBeenCalledWith([joinPath(dir, 'shopify.app.toml'), joinPath(dir, 'extensions')], {
224-
ignored: ['**/node_modules/**', '**/.git/**'],
226+
ignored: [
227+
'**/node_modules/**',
228+
'**/.git/**',
229+
'**/.shopify/**',
230+
'**/.DS_Store',
231+
'**/*.swp',
232+
'**/*.swo',
233+
'**/*~',
234+
expect.any(Function),
235+
],
225236
ignoreInitial: true,
226237
persistent: true,
227238
})
@@ -712,6 +723,167 @@ describe('file-watcher events', () => {
712723
})
713724
})
714725

726+
test('chokidar excludes build output and temporary files while preserving source and imports', async () => {
727+
await inTemporaryDirectory(async (dir) => {
728+
// Given
729+
const files = [
730+
// Rust function: Cargo's target directory is build output
731+
'extensions/rust-function/Cargo.toml',
732+
'extensions/rust-function/src/main.rs',
733+
'extensions/rust-function/target/debug/build/output.rlib',
734+
'extensions/rust-function/target/release/build/output.rlib',
735+
'extensions/rust-function/target/wasm32-wasip1/release/rust-function.wasm',
736+
'extensions/rust-function/target/wasm32-wasi/release/rust-function.wasm',
737+
'extensions/rust-function/target/rust-analyzer/flycheck0/stdout',
738+
'extensions/rust-function/target/.rustc_info.json',
739+
'extensions/rust-function/target/custom-profile/output.wasm',
740+
'extensions/rust-function/target/aarch64-unknown-linux-gnu/debug/output.rlib',
741+
'extensions/js-function/src/index.js',
742+
'extensions/js-function/dist/function.js',
743+
'extensions/js-function/dist/index.wasm',
744+
'workspace/Cargo.toml',
745+
'workspace/target/custom-profile/output.wasm',
746+
// UI extension: a source folder named target, and dist, stay watched
747+
'extensions/ui-extension/src/target/index.js',
748+
'extensions/ui-extension/src/target/debug/index.js',
749+
'extensions/ui-extension/target/release/index.js',
750+
'extensions/rust-function/src/target/debug/source.rs',
751+
'shared/target/release/index.js',
752+
'extensions/ui-extension/dist/main.js',
753+
'extensions/ui-extension/generated/main.js',
754+
'extensions/ui-extension/.DS_Store',
755+
'extensions/ui-extension/src/index.js.swp',
756+
'extensions/ui-extension/src/index.js.swo',
757+
'extensions/ui-extension/src/index.js~',
758+
'extensions/.shopify/dev-bundle/output.js',
759+
]
760+
await Promise.all(
761+
files.map(async (file) => {
762+
await mkdir(dirname(joinPath(dir, file)))
763+
await writeFile(joinPath(dir, file), '')
764+
}),
765+
)
766+
const configPath = joinPath(dir, 'shopify.app.toml')
767+
await writeFile(configPath, '')
768+
769+
const rustFunction = await testFunctionExtension({dir: joinPath(dir, 'extensions/rust-function')})
770+
const jsFunction = await testFunctionExtension({dir: joinPath(dir, 'extensions/js-function')})
771+
const uiExtension = await testUIExtension({directory: joinPath(dir, 'extensions/ui-extension')})
772+
mockExtensionWatchedFiles(uiExtension, [
773+
joinPath(dir, 'shared/target/release/index.js'),
774+
joinPath(dir, 'workspace/Cargo.toml'),
775+
joinPath(dir, 'workspace/target/custom-profile/output.wasm'),
776+
])
777+
const app = testAppLinked({
778+
allExtensions: [rustFunction, jsFunction, uiExtension],
779+
directory: dir,
780+
configPath,
781+
})
782+
783+
let watcher: FSWatcher | undefined
784+
let ready = false
785+
let watcherError: Error | undefined
786+
vi.spyOn(chokidar, 'watch').mockImplementation((paths, options) => {
787+
const realWatcher = realChokidarWatch(paths, options)
788+
realWatcher.once('ready', () => {
789+
ready = true
790+
})
791+
realWatcher.on('error', (error) => {
792+
watcherError = error
793+
})
794+
watcher = realWatcher
795+
return realWatcher
796+
})
797+
798+
// When
799+
const abortController = new AbortController()
800+
const fileWatcher = new FileWatcher(app, {...outputOptions, signal: abortController.signal})
801+
try {
802+
await fileWatcher.start()
803+
await vi.waitFor(() => {
804+
if (watcherError) throw watcherError
805+
expect(ready).toBe(true)
806+
})
807+
808+
// Then
809+
const watched = Object.entries(watcher!.getWatched()).flatMap(([directory, names]) =>
810+
names.map((name) => normalizePath(joinPath(directory, name)).replace(`${normalizePath(dir)}/`, '')),
811+
)
812+
813+
expect(watched).toEqual(
814+
expect.arrayContaining([
815+
'extensions/rust-function/Cargo.toml',
816+
'extensions/rust-function/src/main.rs',
817+
'extensions/js-function/src/index.js',
818+
'workspace/Cargo.toml',
819+
'extensions/ui-extension/src/target/index.js',
820+
'extensions/ui-extension/src/target/debug/index.js',
821+
'extensions/ui-extension/target/release/index.js',
822+
'extensions/rust-function/src/target/debug/source.rs',
823+
'shared/target/release/index.js',
824+
'extensions/ui-extension/dist/main.js',
825+
'extensions/ui-extension/generated/main.js',
826+
]),
827+
)
828+
for (const ignored of [
829+
'extensions/rust-function/target',
830+
'extensions/js-function/dist',
831+
'workspace/target',
832+
'extensions/ui-extension/.DS_Store',
833+
'extensions/ui-extension/src/index.js.swp',
834+
'extensions/ui-extension/src/index.js.swo',
835+
'extensions/ui-extension/src/index.js~',
836+
'extensions/.shopify',
837+
]) {
838+
expect(watched.filter((path) => path === ignored || path.startsWith(`${ignored}/`))).toEqual([])
839+
}
840+
} finally {
841+
abortController.abort()
842+
await watcher?.close()
843+
}
844+
})
845+
})
846+
847+
test('app reloads refresh Function output ignores without restarting the watcher', async () => {
848+
await inTemporaryDirectory(async (directory) => {
849+
const existingFunction = await testFunctionExtension({dir: joinPath(directory, 'extensions/existing-function')})
850+
const newFunction = await testFunctionExtension({dir: joinPath(directory, 'extensions/new-function')})
851+
const app = testAppLinked({
852+
allExtensions: [existingFunction],
853+
directory,
854+
configPath: joinPath(directory, 'shopify.app.toml'),
855+
})
856+
const mockWatcher = {
857+
on: vi.fn().mockReturnThis(),
858+
close: vi.fn().mockResolvedValue(undefined),
859+
}
860+
const watchSpy = vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any)
861+
const abortController = new AbortController()
862+
const fileWatcher = new FileWatcher(app, {...outputOptions, signal: abortController.signal})
863+
864+
try {
865+
await fileWatcher.start()
866+
const ignoredPaths = watchSpy.mock.calls[0]![1]!.ignored as (string | ((path: string) => boolean))[]
867+
const ignoreBuildOutput = ignoredPaths.find((pattern) => typeof pattern === 'function') as (
868+
path: string,
869+
) => boolean
870+
const existingOutput = joinPath(existingFunction.directory, 'dist/function.js')
871+
const newOutput = joinPath(newFunction.directory, 'dist/function.js')
872+
expect(ignoreBuildOutput(existingOutput)).toBe(true)
873+
expect(ignoreBuildOutput(newOutput)).toBe(false)
874+
875+
fileWatcher.updateApp(testAppLinked({...app, allExtensions: [newFunction]}))
876+
877+
expect(ignoreBuildOutput(newOutput)).toBe(true)
878+
expect(ignoreBuildOutput(existingOutput)).toBe(false)
879+
expect(ignoreBuildOutput(joinPath(newFunction.directory, 'src/index.js'))).toBe(false)
880+
expect(watchSpy).toHaveBeenCalledTimes(1)
881+
} finally {
882+
abortController.abort()
883+
}
884+
})
885+
})
886+
715887
describe('runtime file discovery', () => {
716888
test('files added at runtime inside an existing extension trigger file_created', async () => {
717889
await inTemporaryDirectory(async (dir) => {

‎packages/app/src/cli/services/dev/app-events/file-watcher.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,24 @@ const EXTENSION_CREATION_TIMEOUT_IN_MS = 60000
1717
const EXTENSION_CREATION_CHECK_INTERVAL_IN_MS = 200
1818
const FILE_DELETE_TIMEOUT_IN_MS = 500
1919

20+
/**
21+
* Exclude these paths before chokidar traverses them to avoid allocating watchers
22+
* or polling files whose events would be discarded later.
23+
*/
24+
const ALWAYS_IGNORED_PATHS = [
25+
// Dependencies and version control
26+
'**/node_modules/**',
27+
'**/.git/**',
28+
// Files the CLI writes itself: dev and deploy bundles, certificates, app security results
29+
'**/.shopify/**',
30+
// macOS Finder metadata
31+
'**/.DS_Store',
32+
// Editor swap and backup files
33+
'**/*.swp',
34+
'**/*.swo',
35+
'**/*~',
36+
]
37+
2038
/**
2139
* Event emitted by the file watcher
2240
*
@@ -118,8 +136,20 @@ export class FileWatcher {
118136

119137
// Create new watcher
120138
const {default: chokidar} = await import('chokidar')
139+
const cargoProjects = new Map<string, boolean>()
121140
this.watcher = chokidar.watch(watchPaths, {
122-
ignored: ['**/node_modules/**', '**/.git/**'],
141+
ignored: [
142+
...ALWAYS_IGNORED_PATHS,
143+
(path: string) => {
144+
const normalizedPath = normalizePath(path)
145+
const isFunctionOutput = this.app.realExtensions.some((extension) => {
146+
if (!extension.isFunctionExtension) return false
147+
const directory = normalizePath(joinPath(extension.directory, dirname(extension.outputRelativePath)))
148+
return normalizedPath === directory || normalizedPath.startsWith(`${directory}/`)
149+
})
150+
return isFunctionOutput || isInCargoTargetDirectory(normalizedPath, cargoProjects)
151+
},
152+
],
123153
persistent: true,
124154
ignoreInitial: true,
125155
})
@@ -430,3 +460,22 @@ export class FileWatcher {
430460
return ignore.default().add(gitIgnoreContent)
431461
}
432462
}
463+
464+
/**
465+
* Match Cargo output even when a descendant is supplied as an explicit watch path.
466+
* A sibling Cargo.toml distinguishes build output from source folders named target.
467+
*/
468+
function isInCargoTargetDirectory(path: string, cargoProjects: Map<string, boolean>): boolean {
469+
const segments = path.split('/')
470+
for (let index = segments.length - 1; index > 0; index--) {
471+
if (segments[index] !== 'target') continue
472+
const projectDirectory = segments.slice(0, index).join('/')
473+
let isCargoProject = cargoProjects.get(projectDirectory)
474+
if (isCargoProject === undefined) {
475+
isCargoProject = fileExistsSync(joinPath(projectDirectory, 'Cargo.toml'))
476+
cargoProjects.set(projectDirectory, isCargoProject)
477+
}
478+
if (isCargoProject) return true
479+
}
480+
return false
481+
}

0 commit comments

Comments
 (0)