diff --git a/packages/metro-file-map/src/watchers/FallbackWatcher.js b/packages/metro-file-map/src/watchers/FallbackWatcher.js index 5f87b07d7e..1667f6db38 100644 --- a/packages/metro-file-map/src/watchers/FallbackWatcher.js +++ b/packages/metro-file-map/src/watchers/FallbackWatcher.js @@ -215,7 +215,8 @@ export default class FallbackWatcher extends AbstractWatcher { let closest: ?Readonly<{file: string, mtime: Stats['mtime']}> = null; let c = 0; Object.keys(this.#dirRegistry[dir]).forEach((file, i, arr) => { - fs.lstat(path.join(dir, file), (error, stat) => { + const absPath = path.join(dir, file); + fs.lstat(absPath, (error, stat) => { if (found) { return; } @@ -284,17 +285,7 @@ export default class FallbackWatcher extends AbstractWatcher { recReaddir( path.resolve(this.root, relativePath), (dir, stats) => { - if (this.#watchdir(dir)) { - this.#emitEvent({ - event: TOUCH_EVENT, - relativePath: path.relative(this.root, dir), - metadata: { - modifiedTime: stats.mtime.getTime(), - size: stats.size, - type: 'd', - }, - }); - } + this.#watchdir(dir); }, (file, stats) => { if (this.#register(file, 'f')) { diff --git a/packages/metro-file-map/src/watchers/NativeWatcher.js b/packages/metro-file-map/src/watchers/NativeWatcher.js index b191e60df7..165b58139b 100644 --- a/packages/metro-file-map/src/watchers/NativeWatcher.js +++ b/packages/metro-file-map/src/watchers/NativeWatcher.js @@ -108,7 +108,7 @@ export default class NativeWatcher extends AbstractWatcher { const type = typeFromStat(stat); // Ignore files of an unrecognized type - if (!type) { + if (!type || type === 'd') { return; } diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index fddaca522c..b48e77b797 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -263,7 +263,7 @@ export default class WatchmanWatcher extends AbstractWatcher { ); // Ignore files of an unrecognized type - if (type != null && !(type === 'f' || type === 'd' || type === 'l')) { + if (type != null && !(type === 'f' || type === 'l')) { return; } @@ -292,22 +292,17 @@ export default class WatchmanWatcher extends AbstractWatcher { size, ); - if ( - // Change event on dirs are mostly useless. - !(type === 'd' && !isNew) - ) { - const mtime = Number(mtime_ms); - self.emitFileEvent({ - event: TOUCH_EVENT, - clock, - relativePath, - metadata: { - modifiedTime: mtime !== 0 ? mtime : null, - size, - type, - }, - }); - } + const mtime = Number(mtime_ms); + self.emitFileEvent({ + event: TOUCH_EVENT, + clock, + relativePath, + metadata: { + modifiedTime: mtime !== 0 ? mtime : null, + size, + type, + }, + }); } } diff --git a/packages/metro-file-map/src/watchers/__tests__/helpers.js b/packages/metro-file-map/src/watchers/__tests__/helpers.js index 70f3e30681..3fd9c9315a 100644 --- a/packages/metro-file-map/src/watchers/__tests__/helpers.js +++ b/packages/metro-file-map/src/watchers/__tests__/helpers.js @@ -42,7 +42,7 @@ const isWatchmanOnPath = () => { // `null` Watchers will be marked as skipped tests. export const WATCHERS: Readonly<{ - [key: string]: + [key: 'Watchman' | 'Native' | 'Fallback']: | Class | Class | Class @@ -94,7 +94,7 @@ export const createTempWatchRoot = async ( }; export const startWatching = async ( - watcherName: string, + watcherName: keyof typeof WATCHERS, watchRoot: string, opts: WatcherOptions, ): (Promise<{ diff --git a/packages/metro-file-map/src/watchers/__tests__/integration-test.js b/packages/metro-file-map/src/watchers/__tests__/integration-test.js index 9bf5b5193c..bb17104ba9 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -26,9 +26,11 @@ test('NativeWatcher is supported if and only if darwin', () => { expect(NativeWatcher.isSupported()).toBe(os.platform() === 'darwin'); }); +type WatcherKey = keyof typeof WATCHERS; + describe.each(Object.keys(WATCHERS))( 'Watcher integration tests: %s', - watcherName => { + (watcherName: WatcherKey) => { let appRoot; let cookieCount = 1; let watchRoot; @@ -37,10 +39,28 @@ describe.each(Object.keys(WATCHERS))( // If all tests are skipped, Jest will not run before/after hooks either. const maybeTest = WATCHERS[watcherName] ? test : test.skip; - const maybeTestOn = (...platforms: ReadonlyArray) => - platforms.includes(os.platform()) && WATCHERS[watcherName] - ? test - : test.skip; + + async function waitForCookie( + rootRelativeDirPath?: ?string, + opts?: {rejectUnexpected: boolean} = {rejectUnexpected: true}, + ) { + const cookieName = `cookie-${++cookieCount}`; + const relPath = + rootRelativeDirPath != null + ? join(rootRelativeDirPath, cookieName) + : cookieName; + const absPath = join(watchRoot, relPath); + + await eventHelpers.allEvents( + () => writeFile(absPath, ''), + [[relPath, 'touch']], + opts, + ); + expect(await eventHelpers.nextEvent(() => rm(absPath))).toMatchObject({ + path: relPath, + eventType: 'delete', + }); + } beforeAll(async () => { watchRoot = await createTempWatchRoot(watcherName); @@ -82,11 +102,10 @@ describe.each(Object.keys(WATCHERS))( }); beforeEach(async () => { - expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({ - path: 'app', - eventType: 'touch', - metadata: expect.any(Object), - }); + await mkdir(appRoot); + // We can't observe the creation of the app root directly since backends + // don't emit directory events. Instead, create and delete a cookie. + await waitForCookie('app'); }); afterEach(async () => { @@ -98,11 +117,25 @@ describe.each(Object.keys(WATCHERS))( writeFile(join(watchRoot, cookieName), ''), ), ).toMatchObject({path: cookieName, eventType: 'touch'}); - // Cleanup and wait until the app root deletion is reported - this should - // be the last cleanup event emitted. + + // Clean up by deleting the app root. + await rm(appRoot, {recursive: true}); + // Write a regular file to the place we'd normally have the app root + // directory. This guarantees that the directory has been deleted, and + // gives us something we can await the deletion of to ensure all events + // have been flushed. + const cookieName2 = `cookie-${cookieCount}`; + await eventHelpers.untilEvent( + async () => { + await writeFile(join(watchRoot, cookieName2), ''); + }, + cookieName2, + 'touch', + ); + // Finally, await the deletion of the regular file we just created. await eventHelpers.untilEvent( - () => rm(appRoot, {recursive: true}), - 'app', + () => rm(join(watchRoot, cookieName2)), + cookieName2, 'delete', ); }); @@ -215,20 +248,13 @@ describe.each(Object.keys(WATCHERS))( maybeTest('detects changes to files in a new directory', async () => { expect( - await eventHelpers.nextEvent(() => mkdir(join(watchRoot, 'newdir'))), - ).toStrictEqual({ - path: join('newdir'), - eventType: 'touch', - metadata: { - modifiedTime: expect.any(Number), - size: expect.any(Number), - type: 'd', - }, - }); - expect( - await eventHelpers.nextEvent(() => - writeFile(join(watchRoot, 'newdir', 'file-in-new-dir.js'), 'code'), - ), + await eventHelpers.nextEvent(async () => { + await mkdir(join(watchRoot, 'newdir')); + await writeFile( + join(watchRoot, 'newdir', 'file-in-new-dir.js'), + 'code', + ); + }), ).toStrictEqual({ path: join('newdir', 'file-in-new-dir.js'), eventType: 'touch', @@ -240,14 +266,25 @@ describe.each(Object.keys(WATCHERS))( }); }); - /* FIXME: Disabled on Windows and Darwin due to flakiness (occasional - timeouts) - see history. */ - maybeTestOn('darwin')( + maybeTest( 'emits deletion for all files when a directory is deleted', async () => { + if (watcherName !== 'Fallback') { + await mkdir(join(appRoot, 'subdir', 'subdir2'), {recursive: true}); + } else { + // FIXME: A known race in Fallback watcher where it may not be + // watching a directory at the time a new subdirectory is created + // means that we need to create each directory separately and ensure + // it is watched. This could cause real-world problems when file + // trees are created quickly - eg npm install. + await mkdir(join(appRoot, 'subdir')); + await waitForCookie(join('app', 'subdir')); + await mkdir(join(appRoot, 'subdir', 'subdir2')); + await waitForCookie(join('app', 'subdir', 'subdir2')); + } + await eventHelpers.allEvents( async () => { - await mkdir(join(appRoot, 'subdir', 'subdir2'), {recursive: true}); await Promise.all([ writeFile(join(appRoot, 'subdir', 'ignored-file.js'), ''), writeFile(join(appRoot, 'subdir', 'deep.js'), ''), @@ -255,8 +292,6 @@ describe.each(Object.keys(WATCHERS))( ]); }, [ - [join('app', 'subdir'), 'touch'], - [join('app', 'subdir', 'subdir2'), 'touch'], [join('app', 'subdir', 'deep.js'), 'touch'], [join('app', 'subdir', 'subdir2', 'deeper.js'), 'touch'], ], @@ -268,12 +303,19 @@ describe.each(Object.keys(WATCHERS))( await rm(join(appRoot, 'subdir'), {recursive: true}); }, [ - [join('app', 'subdir'), 'delete'], - [join('app', 'subdir', 'subdir2'), 'delete'], [join('app', 'subdir', 'deep.js'), 'delete'], [join('app', 'subdir', 'subdir2', 'deeper.js'), 'delete'], ], - {rejectUnexpected: true}, + // Watchers *may* emit deletion events for directories as well + {rejectUnexpected: false}, + ); + const cookiePath = join('app', `cookie-${++cookieCount}`); + + // Allow deleted events for directories + await eventHelpers.untilEvent( + () => writeFile(join(watchRoot, cookiePath), ''), + cookiePath, + 'touch', ); }, );