From cd2511f9f524c7f6516901efce2865d85181b2b3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 17:42:59 +0000 Subject: [PATCH 1/4] [metro] Flow-ignore .hg directory Summary: When using Sapling in an OSS checkout, Flow does not ignore files under `.hg`, leading to false errors especially in Saplings `origbackups`, which may contain old versions of files out of context. Changelog: Internal Test Plan: ## Before ``` yarn flow ls | grep '\.hg' /Users/robhogan/workspace/metro-sl/.hg/runlog/41hxy19HHF6cGBn491508.json /Users/robhogan/workspace/metro-sl/.hg/runlog/37Z4u1Env4S8RkEa44097.json <...> /Users/robhogan/workspace/metro-sl/.hg/origbackups/packages/metro-file-map/src/watchers/common.js /Users/robhogan/workspace/metro-sl/.hg/origbackups/packages/metro-file-map/src/watchers/FSEventsWatcher.js ``` ``` yarn flow ls | wc -l 25908 ``` ## After ``` yarn flow ls | grep '\.hg' ``` ``` yarn flow ls | wc -l 25861 ``` Reviewers: Subscribers: Tasks: Tags: --- .flowconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/.flowconfig b/.flowconfig index b6bb98611..d30b5c0e9 100644 --- a/.flowconfig +++ b/.flowconfig @@ -1,5 +1,6 @@ [ignore] /packages/.*/build/.* +/\.hg/.* # this transient dep bundles tests with their package, which flow attempts to parse # and crashes out as the test includes purposely malformed json \(/.*\)?/node_modules/resolve/test/.* From 13750206411d7da46edaa3a50785e3fd387d3a1e Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 18:00:35 +0000 Subject: [PATCH 2/4] Watcher backends: use Flow-typed object payloads Summary: Ensure `'add'` and `'change'` events fired by watcher backends always have associated `metadata` (and that `'delete'` events never do), by typing change event payloads as objects with the `event: 'change' | 'add' | 'delete'` as a discriminator. (This is in preparation for refactoring out the distinction between `'change'` and `'add'` from backends, which means they don't need to track files and can do much less IO) Changelog: Internal Differential Revision: D67287046 --- packages/metro-file-map/src/Watcher.js | 47 +-- .../src/__tests__/index-test.js | 366 ++++++++---------- packages/metro-file-map/src/flow-types.js | 14 + packages/metro-file-map/src/index.js | 59 ++- .../src/watchers/FSEventsWatcher.js | 28 +- .../src/watchers/NodeWatcher.js | 75 ++-- .../src/watchers/WatchmanWatcher.js | 28 +- .../src/watchers/__tests__/helpers.js | 38 +- 8 files changed, 322 insertions(+), 333 deletions(-) diff --git a/packages/metro-file-map/src/Watcher.js b/packages/metro-file-map/src/Watcher.js index 3029fe0ce..38f342914 100644 --- a/packages/metro-file-map/src/Watcher.js +++ b/packages/metro-file-map/src/Watcher.js @@ -9,12 +9,12 @@ */ import type { - ChangeEventMetadata, Console, CrawlerOptions, FileData, Path, PerfLogger, + WatcherBackendChangeEvent, WatchmanClocks, } from './flow-types'; import type {WatcherOptions as WatcherBackendOptions} from './watchers/common'; @@ -163,14 +163,7 @@ export class Watcher extends EventEmitter { } } - async watch( - onChange: ( - type: string, - filePath: string, - root: string, - metadata: ChangeEventMetadata, - ) => void, - ) { + async watch(onChange: (change: WatcherBackendChangeEvent) => void) { const {extensions, ignorePattern, useWatchman} = this._options; // WatchmanWatcher > FSEventsWatcher > sane.NodeWatcher @@ -214,29 +207,21 @@ export class Watcher extends EventEmitter { watcher.once('ready', () => { clearTimeout(rejectTimeout); - watcher.on( - 'all', - ( - type: string, - filePath: string, - root: string, - metadata: ChangeEventMetadata, - ) => { - const basename = path.basename(filePath); - if (basename.startsWith(this._options.healthCheckFilePrefix)) { - if (type === ADD_EVENT || type === CHANGE_EVENT) { - debug( - 'Observed possible health check cookie: %s in %s', - filePath, - root, - ); - this._handleHealthCheckObservation(basename); - } - return; + watcher.on('all', (change: WatcherBackendChangeEvent) => { + const basename = path.basename(change.relativePath); + if (basename.startsWith(this._options.healthCheckFilePrefix)) { + if (change.event === ADD_EVENT || change.event === CHANGE_EVENT) { + debug( + 'Observed possible health check cookie: %s in %s', + change.relativePath, + root, + ); + this._handleHealthCheckObservation(basename); } - onChange(type, filePath, root, metadata); - }, - ); + return; + } + onChange(change); + }); resolve(watcher); }); }); diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index 9845b1b48..d9cd8cbe3 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -1528,9 +1528,9 @@ describe('FileMap', () => { }); } - function mockDeleteFile(dirPath, filePath) { - const e = mockEmitters[dirPath]; - e.emit('all', 'delete', filePath, dirPath, undefined); + function mockDeleteFile(root, relativePath) { + const e = mockEmitters[root]; + e.emit('all', {event: 'delete', relativePath, root}); } function fm_it(title, fn, options) { @@ -1620,20 +1620,18 @@ describe('FileMap', () => { // Pear! `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'add', - 'Tomato.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'change', - 'Pear.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'add', + relativePath: 'Tomato.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'change', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toEqual([ { @@ -1663,20 +1661,18 @@ describe('FileMap', () => { mockFs[path.join('/', 'project', 'fruits', 'Tomato.js')] = ` // Tomato! `; - e.emit( - 'all', - 'change', - 'Tomato.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'change', - 'Tomato.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'change', + relativePath: 'Tomato.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'change', + relativePath: 'Tomato.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toHaveLength(1); }); @@ -1690,14 +1686,18 @@ describe('FileMap', () => { mockFs[path.join(fruitsRoot, 'Tomato.js')] = ` // Tomato! `; - e.emit('all', 'change', 'Tomato.js', fruitsRoot, MOCK_CHANGE_FILE); - e.emit( - 'all', - 'change', - 'LinkToStrawberry.js', - fruitsRoot, - MOCK_CHANGE_LINK, - ); + e.emit('all', { + event: 'change', + relativePath: 'Tomato.js', + root: fruitsRoot, + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'change', + relativePath: 'LinkToStrawberry.js', + root: fruitsRoot, + metadata: MOCK_CHANGE_LINK, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toEqual([ { @@ -1721,14 +1721,18 @@ describe('FileMap', () => { mockFs[path.join(fruitsRoot, 'Tomato.js')] = ` // Tomato! `; - e.emit('all', 'change', 'Tomato.js', fruitsRoot, MOCK_CHANGE_FILE); - e.emit( - 'all', - 'change', - 'LinkToStrawberry.js', - fruitsRoot, - MOCK_CHANGE_LINK, - ); + e.emit('all', { + event: 'change', + relativePath: 'Tomato.js', + root: fruitsRoot, + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'change', + relativePath: 'LinkToStrawberry.js', + root: fruitsRoot, + metadata: MOCK_CHANGE_LINK, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toEqual([ { @@ -1754,13 +1758,12 @@ describe('FileMap', () => { async hm => { const {fileSystem} = await hm.build(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'add', - 'apple.js', - path.join('/', 'project', 'fruits', 'node_modules', ''), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'add', + relativePath: 'apple.js', + root: path.join('/', 'project', 'fruits', 'node_modules', ''), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); const filePath = path.join( '/', @@ -1784,20 +1787,18 @@ describe('FileMap', () => { mockFs[path.join('/', 'project', 'fruits', 'Banana.unwatched')] = ''; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'add', - path.join('Banana.js'), - path.join('/', 'project', 'fruits', ''), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'add', - path.join('Banana.unwatched'), - path.join('/', 'project', 'fruits', ''), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'add', + relativePath: 'Banana.js', + root: path.join('/', 'project', 'fruits', ''), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'add', + relativePath: 'Banana.unwatched', + root: path.join('/', 'project', 'fruits', ''), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); const filePath = path.join('/', 'project', 'fruits', 'Banana.js'); expect(eventsQueue).toHaveLength(1); @@ -1813,20 +1814,16 @@ describe('FileMap', () => { mockFs[path.join('/', 'project', 'fruits', 'Banana.unwatched')] = ''; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'delete', - path.join('Banana.js'), - path.join('/', 'project', 'fruits', ''), - null, - ); - e.emit( - 'all', - 'delete', - path.join('Unknown.ext'), - path.join('/', 'project', 'fruits', ''), - null, - ); + e.emit('all', { + event: 'delete', + relativePath: 'Banana.js', + root: path.join('/', 'project', 'fruits', ''), + }); + e.emit('all', { + event: 'delete', + relativePath: 'Unknown.ext', + root: path.join('/', 'project', 'fruits', ''), + }); const {eventsQueue} = await waitForItToChange(hm); const filePath = path.join('/', 'project', 'fruits', 'Banana.js'); expect(eventsQueue).toHaveLength(1); @@ -1846,13 +1843,12 @@ describe('FileMap', () => { mockFs[path.join('/', 'project', 'fruits', 'LinkToStrawberry.ext')] = { link: 'Strawberry.js', }; - e.emit( - 'all', - 'add', - path.join('LinkToStrawberry.ext'), - path.join('/', 'project', 'fruits', ''), - MOCK_CHANGE_LINK, - ); + e.emit('all', { + event: 'add', + relativePath: 'LinkToStrawberry.ext', + root: path.join('/', 'project', 'fruits', ''), + metadata: MOCK_CHANGE_LINK, + }); const {eventsQueue} = await waitForItToChange(hm); const filePath = path.join( '/', @@ -1895,13 +1891,11 @@ describe('FileMap', () => { // Delete the symlink const e = mockEmitters[path.join('/', 'project', 'fruits')]; delete mockFs[symlinkPath]; - e.emit( - 'all', - 'delete', - 'LinkToStrawberry.js', - path.join('/', 'project', 'fruits', ''), - null, - ); + e.emit('all', { + event: 'delete', + relativePath: 'LinkToStrawberry.js', + root: path.join('/', 'project', 'fruits', ''), + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toHaveLength(1); @@ -1926,20 +1920,18 @@ describe('FileMap', () => { expect(hasteMap.getModule('Orange', 'ios')).toBeTruthy(); expect(hasteMap.getModule('Orange', 'android')).toBeTruthy(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'change', - 'Orange.ios.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'change', - 'Orange.android.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'change', + relativePath: 'Orange.ios.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'change', + relativePath: 'Orange.android.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toHaveLength(2); expect(eventsQueue).toEqual([ @@ -1996,20 +1988,17 @@ describe('FileMap', () => { mockFs[newPath] = mockFs[oldPath]; mockFs[oldPath] = null; - mockEmitters[path.join('/', 'project', 'vegetables')].emit( - 'all', - 'delete', - 'Melon.js', - path.join('/', 'project', 'vegetables'), - null, - ); - mockEmitters[path.join('/', 'project', 'fruits')].emit( - 'all', - 'add', - 'Melon.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + mockEmitters[path.join('/', 'project', 'vegetables')].emit('all', { + event: 'delete', + relativePath: 'Melon.js', + root: path.join('/', 'project', 'vegetables'), + }); + mockEmitters[path.join('/', 'project', 'fruits')].emit('all', { + event: 'add', + relativePath: 'Melon.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); @@ -2043,20 +2032,18 @@ describe('FileMap', () => { // Pear too! `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'change', - 'Pear.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'add', - 'Pear.js', - path.join('/', 'project', 'fruits', 'another'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'change', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'add', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits', 'another'), + metadata: MOCK_CHANGE_FILE, + }); await waitForItToChange(hm); expect( fileSystem.exists( @@ -2093,20 +2080,18 @@ describe('FileMap', () => { `; const {fileSystem} = await hm.build(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'change', - 'Pear.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'add', - 'Pear.js', - path.join('/', 'project', 'fruits', 'another'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'change', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'add', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits', 'another'), + metadata: MOCK_CHANGE_FILE, + }); await new Promise((resolve, reject) => { console.error.mockImplementationOnce(() => { reject(new Error('should not print error')); @@ -2147,20 +2132,18 @@ describe('FileMap', () => { // Pear! `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'delete', - 'Pear.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'add', - 'Pear2.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'delete', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'add', + relativePath: 'Pear2.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); await waitForItToChange(hm); expect(hasteMap.getModule('Pear')).toBe( path.join('/', 'project', 'fruits', 'another', 'Pear.js'), @@ -2180,20 +2163,17 @@ describe('FileMap', () => { // Pear too! `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; - e.emit( - 'all', - 'add', - 'Pear2.js', - path.join('/', 'project', 'fruits', 'another'), - MOCK_CHANGE_FILE, - ); - e.emit( - 'all', - 'delete', - 'Pear.js', - path.join('/', 'project', 'fruits', 'another'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'add', + relativePath: 'Pear2.js', + root: path.join('/', 'project', 'fruits', 'another'), + metadata: MOCK_CHANGE_FILE, + }); + e.emit('all', { + event: 'delete', + relativePath: 'Pear.js', + root: path.join('/', 'project', 'fruits', 'another'), + }); await waitForItToChange(hm); expect(hasteMap.getModule('Pear')).toBe( path.join('/', 'project', 'fruits', 'Pear.js'), @@ -2208,20 +2188,18 @@ describe('FileMap', () => { mockFs[path.join('/', 'project', 'fruits', 'tomato.js', 'index.js')] = ` // Tomato! `; - e.emit( - 'all', - 'change', - 'tomato.js', - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FOLDER, - ); - e.emit( - 'all', - 'change', - path.join('tomato.js', 'index.js'), - path.join('/', 'project', 'fruits'), - MOCK_CHANGE_FILE, - ); + e.emit('all', { + event: 'change', + relativePath: 'tomato.js', + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FOLDER, + }); + e.emit('all', { + event: 'change', + relativePath: path.join('tomato.js', 'index.js'), + root: path.join('/', 'project', 'fruits'), + metadata: MOCK_CHANGE_FILE, + }); const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toHaveLength(1); }); diff --git a/packages/metro-file-map/src/flow-types.js b/packages/metro-file-map/src/flow-types.js index 41c448ae0..ced1c0f77 100644 --- a/packages/metro-file-map/src/flow-types.js +++ b/packages/metro-file-map/src/flow-types.js @@ -322,6 +322,20 @@ export type ReadOnlyRawMockMap = $ReadOnly<{ mocks: $ReadOnlyMap, }>; +export type WatcherBackendChangeEvent = + | $ReadOnly<{ + event: 'change' | 'add', + relativePath: string, + root: string, + metadata: ChangeEventMetadata, + }> + | $ReadOnly<{ + event: 'delete', + relativePath: string, + root: string, + metadata?: void, + }>; + export type WatchmanClockSpec = | string | $ReadOnly<{scm: $ReadOnly<{'mergebase-with': string}>}>; diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 82c62ef76..6224b915e 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -31,6 +31,7 @@ import type { Path, PerfLogger, PerfLoggerFactory, + WatcherBackendChangeEvent, WatchmanClocks, WorkerMetadata, } from './flow-types'; @@ -864,27 +865,23 @@ export default class FileMap extends EventEmitter { nextEmit = null; }; - const onChange = ( - type: string, - filePath: Path, - root: Path, - metadata: ?ChangeEventMetadata, - ) => { + const onChange = (change: WatcherBackendChangeEvent) => { if ( - metadata && + change.metadata && // Ignore all directory events - (metadata.type === 'd' || + (change.metadata.type === 'd' || // Ignore regular files with unwatched extensions - (metadata.type === 'f' && !hasWatchedExtension(filePath)) || + (change.metadata.type === 'f' && + !hasWatchedExtension(change.relativePath)) || // Don't emit events relating to symlinks if enableSymlinks: false - (!this._options.enableSymlinks && metadata?.type === 'l')) + (!this._options.enableSymlinks && change.metadata?.type === 'l')) ) { return; } const absoluteFilePath = path.join( - root, - normalizePathSeparatorsToSystem(filePath), + change.root, + normalizePathSeparatorsToSystem(change.relativePath), ); // Ignore files (including symlinks) whose path matches ignorePattern @@ -901,11 +898,10 @@ export default class FileMap extends EventEmitter { // null, then it is assumed that the watcher does not have capabilities // to detect modified time, and change processing proceeds. if ( - type === 'change' && + change.event === 'change' && linkStats != null && - metadata && - metadata.modifiedTime != null && - linkStats.modifiedTime === metadata.modifiedTime + change.metadata.modifiedTime != null && + linkStats.modifiedTime === change.metadata.modifiedTime ) { return; } @@ -919,14 +915,15 @@ export default class FileMap extends EventEmitter { nextEmit != null && nextEmit.eventsQueue.find( event => - event.type === type && + event.type === change.event && event.filePath === absoluteFilePath && - ((!event.metadata && !metadata) || + ((!event.metadata && !change.metadata) || (event.metadata && - metadata && + change.metadata && event.metadata.modifiedTime != null && - metadata.modifiedTime != null && - event.metadata.modifiedTime === metadata.modifiedTime)), + change.metadata.modifiedTime != null && + event.metadata.modifiedTime === + change.metadata.modifiedTime)), ) ) { return null; @@ -938,7 +935,7 @@ export default class FileMap extends EventEmitter { const event = { filePath: absoluteFilePath, metadata, - type, + type: change.event, }; if (nextEmit == null) { nextEmit = { @@ -965,19 +962,19 @@ export default class FileMap extends EventEmitter { // If the file was added or changed, // parse it and update the haste map. - if (type === 'add' || type === 'change') { + if (change.event === 'add' || change.event === 'change') { invariant( - metadata != null && metadata.size != null, - 'since the file exists or changed, it should have metadata', + change.metadata.size != null, + 'since the file exists or changed, it should have known size', ); const fileMetadata: FileMetaData = [ '', - metadata.modifiedTime, - metadata.size, + change.metadata.modifiedTime, + change.metadata.size, 0, '', null, - metadata.type === 'l' ? 1 : 0, + change.metadata.type === 'l' ? 1 : 0, ]; try { @@ -989,7 +986,7 @@ export default class FileMap extends EventEmitter { {forceInBand: true}, // No need to clean up workers ); fileSystem.addOrModify(relativeFilePath, fileMetadata); - enqueueEvent(metadata); + enqueueEvent(change.metadata); } catch (e) { if (!['ENOENT', 'EACCESS'].includes(e.code)) { throw e; @@ -1001,7 +998,7 @@ export default class FileMap extends EventEmitter { // event for it, and we'll clean up in the usual way at that // point. } - } else if (type === 'delete') { + } else if (change.event === 'delete') { if (linkStats == null) { // Don't emit deletion events for files we weren't retaining. // This is expected for deletion of an ignored file. @@ -1014,7 +1011,7 @@ export default class FileMap extends EventEmitter { }); } else { throw new Error( - `metro-file-map: Unrecognized event type from watcher: ${type}`, + `metro-file-map: Unrecognized event type from watcher: ${change.event}`, ); } return null; diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index 0117eaec3..99b451985 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -8,7 +8,10 @@ * @flow strict-local */ -import type {ChangeEventMetadata} from '../flow-types'; +import type { + ChangeEventMetadata, + WatcherBackendChangeEvent, +} from '../flow-types'; // $FlowFixMe[cannot-resolve-module] - Optional, Darwin only // $FlowFixMe[untyped-type-import] import type {FSEvents} from 'fsevents'; @@ -35,12 +38,6 @@ const DELETE_EVENT = 'delete'; const ADD_EVENT = 'add'; const ALL_EVENT = 'all'; -type FsEventsWatcherEvent = - | typeof CHANGE_EVENT - | typeof DELETE_EVENT - | typeof ADD_EVENT - | typeof ALL_EVENT; - /** * Export `FSEventsWatcher` class. * Watches `dir`. @@ -165,10 +162,10 @@ export default class FSEventsWatcher extends EventEmitter { }; if (this._tracked.has(filepath)) { - this._emit(CHANGE_EVENT, relativePath, metadata); + this._emit({event: CHANGE_EVENT, relativePath, metadata}); } else { this._tracked.add(filepath); - this._emit(ADD_EVENT, relativePath, metadata); + this._emit({event: ADD_EVENT, relativePath, metadata}); } } catch (error) { if (error?.code !== 'ENOENT') { @@ -181,7 +178,7 @@ export default class FSEventsWatcher extends EventEmitter { return; } - this._emit(DELETE_EVENT, relativePath); + this._emit({event: DELETE_EVENT, relativePath}); this._tracked.delete(filepath); } } @@ -189,12 +186,11 @@ export default class FSEventsWatcher extends EventEmitter { /** * Emit events. */ - _emit( - type: FsEventsWatcherEvent, - file: string, - metadata?: ChangeEventMetadata, - ) { - this.emit(ALL_EVENT, type, file, this.root, metadata); + _emit(payload: Omit) { + this.emit(ALL_EVENT, { + ...payload, + root: this.root, + } as WatcherBackendChangeEvent); } getPauseReason(): ?string { diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index 65cf44c9a..75fa2829b 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -15,7 +15,10 @@ 'use strict'; -import type {ChangeEventMetadata} from '../flow-types'; +import type { + ChangeEventMetadata, + WatcherBackendChangeEvent, +} from '../flow-types'; import type {WatcherOptions} from './common'; import type {FSWatcher, Stats} from 'fs'; @@ -298,29 +301,42 @@ module.exports = class NodeWatcher extends EventEmitter { path.resolve(this.root, relativePath), (dir, stats) => { if (this._watchdir(dir)) { - this._emitEvent(ADD_EVENT, path.relative(this.root, dir), { - modifiedTime: stats.mtime.getTime(), - size: stats.size, - type: 'd', + this._emitEvent({ + event: ADD_EVENT, + relativePath: path.relative(this.root, dir), + metadata: { + modifiedTime: stats.mtime.getTime(), + size: stats.size, + type: 'd', + }, }); } }, (file, stats) => { if (this._register(file, 'f')) { - this._emitEvent(ADD_EVENT, path.relative(this.root, file), { - modifiedTime: stats.mtime.getTime(), - size: stats.size, - type: 'f', + this._emitEvent({ + event: ADD_EVENT, + relativePath: path.relative(this.root, file), + metadata: { + modifiedTime: stats.mtime.getTime(), + size: stats.size, + type: 'f', + }, }); } }, (symlink, stats) => { if (this._register(symlink, 'l')) { - this._rawEmitEvent(ADD_EVENT, path.relative(this.root, symlink), { - modifiedTime: stats.mtime.getTime(), - size: stats.size, - type: 'l', - }); + this.emit(ALL_EVENT, { + event: ADD_EVENT, + relativePath: path.relative(this.root, symlink), + root: this.root, + metadata: { + modifiedTime: stats.mtime.getTime(), + size: stats.size, + type: 'l', + }, + } as WatcherBackendChangeEvent); } }, function endCallback() {}, @@ -338,10 +354,10 @@ module.exports = class NodeWatcher extends EventEmitter { type, }; if (registered) { - this._emitEvent(CHANGE_EVENT, relativePath, metadata); + this._emitEvent({event: CHANGE_EVENT, relativePath, metadata}); } else { if (this._register(fullPath, type)) { - this._emitEvent(ADD_EVENT, relativePath, metadata); + this._emitEvent({event: ADD_EVENT, relativePath, metadata}); } } } @@ -353,7 +369,7 @@ module.exports = class NodeWatcher extends EventEmitter { this._unregister(fullPath); this._unregisterDir(fullPath); if (registered) { - this._emitEvent(DELETE_EVENT, relativePath); + this._emitEvent({event: DELETE_EVENT, relativePath}); } await this._stopWatching(fullPath); } @@ -366,10 +382,11 @@ module.exports = class NodeWatcher extends EventEmitter { * * See also note above for DEBOUNCE_MS. */ - _emitEvent(type: string, file: string, metadata?: ChangeEventMetadata) { - const key = type + '-' + file; - const addKey = ADD_EVENT + '-' + file; - if (type === CHANGE_EVENT && this._changeTimers.has(addKey)) { + _emitEvent(change: Omit) { + const {event, relativePath} = change; + const key = event + '-' + relativePath; + const addKey = ADD_EVENT + '-' + relativePath; + if (event === CHANGE_EVENT && this._changeTimers.has(addKey)) { // Ignore the change event that is immediately fired after an add event. // (This happens on Linux). return; @@ -382,22 +399,14 @@ module.exports = class NodeWatcher extends EventEmitter { key, setTimeout(() => { this._changeTimers.delete(key); - this._rawEmitEvent(type, file, metadata); + this.emit(ALL_EVENT, { + ...change, + root: this.root, + } as WatcherBackendChangeEvent); }, DEBOUNCE_MS), ); } - /** - * Actually emit the events - */ - _rawEmitEvent( - eventType: string, - file: string, - metadata: ?ChangeEventMetadata, - ) { - this.emit(ALL_EVENT, eventType, file, this.root, metadata); - } - getPauseReason(): ?string { return null; } diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index bc7f85289..6a5d7e13d 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -9,7 +9,7 @@ * @oncall react_native */ -import type {ChangeEventMetadata} from '../flow-types'; +import type {WatcherBackendChangeEvent} from '../flow-types'; import type {WatcherOptions} from './common'; import type { Client, @@ -272,7 +272,7 @@ export default class WatchmanWatcher extends EventEmitter { } if (!exists) { - self._emitEvent(DELETE_EVENT, relativePath, self.root); + self._emitEvent({event: DELETE_EVENT, relativePath}); } else { const eventType = isNew ? ADD_EVENT : CHANGE_EVENT; invariant( @@ -290,10 +290,14 @@ export default class WatchmanWatcher extends EventEmitter { !(type === 'd' && eventType === CHANGE_EVENT) ) { const mtime = Number(mtime_ms); - self._emitEvent(eventType, relativePath, self.root, { - modifiedTime: mtime !== 0 ? mtime : null, - size, - type, + self._emitEvent({ + event: eventType, + relativePath, + metadata: { + modifiedTime: mtime !== 0 ? mtime : null, + size, + type, + }, }); } } @@ -302,13 +306,11 @@ export default class WatchmanWatcher extends EventEmitter { /** * Dispatches the event. */ - _emitEvent( - eventType: string, - filepath: string, - root: string, - changeMetadata?: ChangeEventMetadata, - ) { - this.emit(ALL_EVENT, eventType, filepath, root, changeMetadata); + _emitEvent(change: Omit) { + this.emit(ALL_EVENT, { + ...change, + root: this.root, + } as WatcherBackendChangeEvent); } /** diff --git a/packages/metro-file-map/src/watchers/__tests__/helpers.js b/packages/metro-file-map/src/watchers/__tests__/helpers.js index e852e5d43..250e4874b 100644 --- a/packages/metro-file-map/src/watchers/__tests__/helpers.js +++ b/packages/metro-file-map/src/watchers/__tests__/helpers.js @@ -9,7 +9,10 @@ * @oncall react_native */ -import type {ChangeEventMetadata} from '../../flow-types'; +import type { + ChangeEventMetadata, + WatcherBackendChangeEvent, +} from '../../flow-types'; import type {WatcherOptions} from '../common'; import FSEventsWatcher from '../FSEventsWatcher'; @@ -117,23 +120,24 @@ export const startWatching = async ( metadata?: ChangeEventMetadata, path: string, }>((resolve, reject) => { - const listener = ( - eventType: string, - path: string, - root: string, - metadata?: ChangeEventMetadata, - ) => { - if (path === '') { + const listener = (change: WatcherBackendChangeEvent) => { + if (change.relativePath === '') { // FIXME: FSEventsWatcher sometimes reports 'change' events to // the watch root. return; } watcherInstance.removeListener('all', listener); - if (root !== watchRoot) { - reject(new Error(`Expected root ${watchRoot}, got ${root}`)); + if (change.root !== watchRoot) { + reject( + new Error(`Expected root ${watchRoot}, got ${change.root}`), + ); } - resolve({eventType, path, metadata}); + resolve({ + eventType: change.event, + path: change.relativePath, + metadata: change.metadata, + }); }; watcherInstance.on('all', listener); }), @@ -154,13 +158,13 @@ export const startWatching = async ( const allEventKeys = new Set( expectedEvents.map(tuple => tupleToKey(tuple)), ); - const listener = (eventType: string, path: string) => { - if (path === '') { + const listener = (change: WatcherBackendChangeEvent) => { + if (change.relativePath === '') { // FIXME: FSEventsWatcher sometimes reports 'change' events to // the watch root. return; } - const receivedKey = tupleToKey([path, eventType]); + const receivedKey = tupleToKey([change.relativePath, change.event]); if (allEventKeys.has(receivedKey)) { allEventKeys.delete(receivedKey); if (allEventKeys.size === 0) { @@ -169,7 +173,11 @@ export const startWatching = async ( } } else if (rejectUnexpected) { watcherInstance.removeListener('all', listener); - reject(new Error(`Unexpected event: ${eventType} ${path}.`)); + reject( + new Error( + `Unexpected event: ${change.event} ${change.relativePath}.`, + ), + ); } }; watcherInstance.on('all', listener); From f981dbcea6ffba459b8f41d2b4ea0cc5b72d20e3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 18:00:35 +0000 Subject: [PATCH 3/4] add/change -> touch Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: --- packages/metro-file-map/src/Watcher.js | 4 +- .../src/__tests__/index-test.js | 62 +++++++++---------- packages/metro-file-map/src/flow-types.js | 2 +- packages/metro-file-map/src/index.js | 19 ++++-- .../src/watchers/FSEventsWatcher.js | 10 +-- .../src/watchers/NodeWatcher.js | 25 +++----- .../src/watchers/WatchmanWatcher.js | 8 +-- .../src/watchers/__tests__/helpers.js | 8 +-- .../watchers/__tests__/integration-test.js | 26 ++++---- .../metro-file-map/src/watchers/common.js | 3 +- 10 files changed, 77 insertions(+), 90 deletions(-) diff --git a/packages/metro-file-map/src/Watcher.js b/packages/metro-file-map/src/Watcher.js index 38f342914..5da323e76 100644 --- a/packages/metro-file-map/src/Watcher.js +++ b/packages/metro-file-map/src/Watcher.js @@ -22,7 +22,7 @@ import type {AbortSignal} from 'node-abort-controller'; import nodeCrawl from './crawlers/node'; import watchmanCrawl from './crawlers/watchman'; -import {ADD_EVENT, CHANGE_EVENT} from './watchers/common'; +import {TOUCH_EVENT} from './watchers/common'; import FSEventsWatcher from './watchers/FSEventsWatcher'; import NodeWatcher from './watchers/NodeWatcher'; import WatchmanWatcher from './watchers/WatchmanWatcher'; @@ -210,7 +210,7 @@ export class Watcher extends EventEmitter { watcher.on('all', (change: WatcherBackendChangeEvent) => { const basename = path.basename(change.relativePath); if (basename.startsWith(this._options.healthCheckFilePrefix)) { - if (change.event === ADD_EVENT || change.event === CHANGE_EVENT) { + if (change.event === TOUCH_EVENT) { debug( 'Observed possible health check cookie: %s in %s', change.relativePath, diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index d9cd8cbe3..1ca10dab1 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -1621,13 +1621,13 @@ describe('FileMap', () => { `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Tomato.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Pear.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, @@ -1662,13 +1662,13 @@ describe('FileMap', () => { // Tomato! `; e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Tomato.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Tomato.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, @@ -1683,17 +1683,14 @@ describe('FileMap', () => { const {fileSystem} = await hm.build(); const fruitsRoot = path.join('/', 'project', 'fruits'); const e = mockEmitters[fruitsRoot]; - mockFs[path.join(fruitsRoot, 'Tomato.js')] = ` - // Tomato! - `; e.emit('all', { - event: 'change', - relativePath: 'Tomato.js', + event: 'touch', + relativePath: 'Strawberry.js', root: fruitsRoot, metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'LinkToStrawberry.js', root: fruitsRoot, metadata: MOCK_CHANGE_LINK, @@ -1701,7 +1698,7 @@ describe('FileMap', () => { const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toEqual([ { - filePath: path.join(fruitsRoot, 'Tomato.js'), + filePath: path.join(fruitsRoot, 'Strawberry.js'), metadata: MOCK_CHANGE_FILE, type: 'change', }, @@ -1718,17 +1715,14 @@ describe('FileMap', () => { const {fileSystem} = await hm.build(); const fruitsRoot = path.join('/', 'project', 'fruits'); const e = mockEmitters[fruitsRoot]; - mockFs[path.join(fruitsRoot, 'Tomato.js')] = ` - // Tomato! - `; e.emit('all', { - event: 'change', - relativePath: 'Tomato.js', + event: 'touch', + relativePath: 'Strawberry.js', root: fruitsRoot, metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'LinkToStrawberry.js', root: fruitsRoot, metadata: MOCK_CHANGE_LINK, @@ -1736,7 +1730,7 @@ describe('FileMap', () => { const {eventsQueue} = await waitForItToChange(hm); expect(eventsQueue).toEqual([ { - filePath: path.join(fruitsRoot, 'Tomato.js'), + filePath: path.join(fruitsRoot, 'Strawberry.js'), metadata: MOCK_CHANGE_FILE, type: 'change', }, @@ -1759,7 +1753,7 @@ describe('FileMap', () => { const {fileSystem} = await hm.build(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'apple.js', root: path.join('/', 'project', 'fruits', 'node_modules', ''), metadata: MOCK_CHANGE_FILE, @@ -1788,13 +1782,13 @@ describe('FileMap', () => { const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Banana.js', root: path.join('/', 'project', 'fruits', ''), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Banana.unwatched', root: path.join('/', 'project', 'fruits', ''), metadata: MOCK_CHANGE_FILE, @@ -1803,7 +1797,7 @@ describe('FileMap', () => { const filePath = path.join('/', 'project', 'fruits', 'Banana.js'); expect(eventsQueue).toHaveLength(1); expect(eventsQueue).toEqual([ - {filePath, metadata: MOCK_CHANGE_FILE, type: 'add'}, + {filePath, metadata: MOCK_CHANGE_FILE, type: 'change'}, ]); expect(fileSystem.getModuleName(filePath)).toBeDefined(); }, @@ -1844,7 +1838,7 @@ describe('FileMap', () => { link: 'Strawberry.js', }; e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'LinkToStrawberry.ext', root: path.join('/', 'project', 'fruits', ''), metadata: MOCK_CHANGE_LINK, @@ -1921,13 +1915,13 @@ describe('FileMap', () => { expect(hasteMap.getModule('Orange', 'android')).toBeTruthy(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Orange.ios.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Orange.android.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, @@ -1994,7 +1988,7 @@ describe('FileMap', () => { root: path.join('/', 'project', 'vegetables'), }); mockEmitters[path.join('/', 'project', 'fruits')].emit('all', { - event: 'add', + event: 'touch', relativePath: 'Melon.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, @@ -2033,13 +2027,13 @@ describe('FileMap', () => { `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Pear.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Pear.js', root: path.join('/', 'project', 'fruits', 'another'), metadata: MOCK_CHANGE_FILE, @@ -2081,13 +2075,13 @@ describe('FileMap', () => { const {fileSystem} = await hm.build(); const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'Pear.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Pear.js', root: path.join('/', 'project', 'fruits', 'another'), metadata: MOCK_CHANGE_FILE, @@ -2139,7 +2133,7 @@ describe('FileMap', () => { metadata: MOCK_CHANGE_FILE, }); e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Pear2.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, @@ -2164,7 +2158,7 @@ describe('FileMap', () => { `; const e = mockEmitters[path.join('/', 'project', 'fruits')]; e.emit('all', { - event: 'add', + event: 'touch', relativePath: 'Pear2.js', root: path.join('/', 'project', 'fruits', 'another'), metadata: MOCK_CHANGE_FILE, @@ -2189,13 +2183,13 @@ describe('FileMap', () => { // Tomato! `; e.emit('all', { - event: 'change', + event: 'touch', relativePath: 'tomato.js', root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FOLDER, }); e.emit('all', { - event: 'change', + event: 'touch', relativePath: path.join('tomato.js', 'index.js'), root: path.join('/', 'project', 'fruits'), metadata: MOCK_CHANGE_FILE, diff --git a/packages/metro-file-map/src/flow-types.js b/packages/metro-file-map/src/flow-types.js index ced1c0f77..48ea26607 100644 --- a/packages/metro-file-map/src/flow-types.js +++ b/packages/metro-file-map/src/flow-types.js @@ -324,7 +324,7 @@ export type ReadOnlyRawMockMap = $ReadOnly<{ export type WatcherBackendChangeEvent = | $ReadOnly<{ - event: 'change' | 'add', + event: 'touch', relativePath: string, root: string, metadata: ChangeEventMetadata, diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 6224b915e..65ddc2d04 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -898,7 +898,7 @@ export default class FileMap extends EventEmitter { // null, then it is assumed that the watcher does not have capabilities // to detect modified time, and change processing proceeds. if ( - change.event === 'change' && + change.event === 'touch' && linkStats != null && change.metadata.modifiedTime != null && linkStats.modifiedTime === change.metadata.modifiedTime @@ -906,6 +906,15 @@ export default class FileMap extends EventEmitter { return; } + // Emitted events, unlike memoryless backend events, specify 'add' or + // 'change' instead of 'touch'. + const eventTypeToEmit = + change.event === 'touch' + ? linkStats == null + ? 'add' + : 'change' + : 'delete'; + const onChangeStartTime = performance.timeOrigin + performance.now(); changeQueue = changeQueue @@ -915,7 +924,7 @@ export default class FileMap extends EventEmitter { nextEmit != null && nextEmit.eventsQueue.find( event => - event.type === change.event && + event.type === eventTypeToEmit && event.filePath === absoluteFilePath && ((!event.metadata && !change.metadata) || (event.metadata && @@ -935,7 +944,7 @@ export default class FileMap extends EventEmitter { const event = { filePath: absoluteFilePath, metadata, - type: change.event, + type: eventTypeToEmit, }; if (nextEmit == null) { nextEmit = { @@ -960,9 +969,9 @@ export default class FileMap extends EventEmitter { ); } - // If the file was added or changed, + // If the file was added or modified, // parse it and update the haste map. - if (change.event === 'add' || change.event === 'change') { + if (change.event === 'touch') { invariant( change.metadata.size != null, 'since the file exists or changed, it should have known size', diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index 99b451985..b2aa576eb 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -33,9 +33,8 @@ try { // Optional dependency, only supported on Darwin. } -const CHANGE_EVENT = 'change'; +const TOUCH_EVENT = 'touch'; const DELETE_EVENT = 'delete'; -const ADD_EVENT = 'add'; const ALL_EVENT = 'all'; /** @@ -161,12 +160,7 @@ export default class FSEventsWatcher extends EventEmitter { size: stat.size, }; - if (this._tracked.has(filepath)) { - this._emit({event: CHANGE_EVENT, relativePath, metadata}); - } else { - this._tracked.add(filepath); - this._emit({event: ADD_EVENT, relativePath, metadata}); - } + this._emit({event: TOUCH_EVENT, relativePath, metadata}); } catch (error) { if (error?.code !== 'ENOENT') { this.emit('error', error); diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index 75fa2829b..839bdc168 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -30,9 +30,8 @@ const path = require('path'); const fsPromises = fs.promises; -const CHANGE_EVENT = common.CHANGE_EVENT; +const TOUCH_EVENT = common.TOUCH_EVENT; const DELETE_EVENT = common.DELETE_EVENT; -const ADD_EVENT = common.ADD_EVENT; const ALL_EVENT = common.ALL_EVENT; /** @@ -302,7 +301,7 @@ module.exports = class NodeWatcher extends EventEmitter { (dir, stats) => { if (this._watchdir(dir)) { this._emitEvent({ - event: ADD_EVENT, + event: TOUCH_EVENT, relativePath: path.relative(this.root, dir), metadata: { modifiedTime: stats.mtime.getTime(), @@ -315,7 +314,7 @@ module.exports = class NodeWatcher extends EventEmitter { (file, stats) => { if (this._register(file, 'f')) { this._emitEvent({ - event: ADD_EVENT, + event: TOUCH_EVENT, relativePath: path.relative(this.root, file), metadata: { modifiedTime: stats.mtime.getTime(), @@ -328,7 +327,7 @@ module.exports = class NodeWatcher extends EventEmitter { (symlink, stats) => { if (this._register(symlink, 'l')) { this.emit(ALL_EVENT, { - event: ADD_EVENT, + event: TOUCH_EVENT, relativePath: path.relative(this.root, symlink), root: this.root, metadata: { @@ -354,10 +353,10 @@ module.exports = class NodeWatcher extends EventEmitter { type, }; if (registered) { - this._emitEvent({event: CHANGE_EVENT, relativePath, metadata}); + this._emitEvent({event: TOUCH_EVENT, relativePath, metadata}); } else { if (this._register(fullPath, type)) { - this._emitEvent({event: ADD_EVENT, relativePath, metadata}); + this._emitEvent({event: TOUCH_EVENT, relativePath, metadata}); } } } @@ -376,21 +375,15 @@ module.exports = class NodeWatcher extends EventEmitter { } /** - * Emits the given event after debouncing, to 1) suppress 'change' events - * immediately following an 'add', and 2) to only emit the latest 'change' - * event when received in quick succession for a given file. + * Emits the given event after debouncing, to emit only the latest + * information when we receive several events in quick succession. E.g., + * Linux emits two events for every new file. * * See also note above for DEBOUNCE_MS. */ _emitEvent(change: Omit) { const {event, relativePath} = change; const key = event + '-' + relativePath; - const addKey = ADD_EVENT + '-' + relativePath; - if (event === CHANGE_EVENT && this._changeTimers.has(addKey)) { - // Ignore the change event that is immediately fired after an add event. - // (This happens on Linux). - return; - } const existingTimer = this._changeTimers.get(key); if (existingTimer) { clearTimeout(existingTimer); diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index 6a5d7e13d..5ff39e0cb 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -32,9 +32,8 @@ import path from 'path'; const debug = require('debug')('Metro:WatchmanWatcher'); -const CHANGE_EVENT = common.CHANGE_EVENT; const DELETE_EVENT = common.DELETE_EVENT; -const ADD_EVENT = common.ADD_EVENT; +const TOUCH_EVENT = common.TOUCH_EVENT; const ALL_EVENT = common.ALL_EVENT; const SUB_PREFIX = 'metro-file-map'; @@ -274,7 +273,6 @@ export default class WatchmanWatcher extends EventEmitter { if (!exists) { self._emitEvent({event: DELETE_EVENT, relativePath}); } else { - const eventType = isNew ? ADD_EVENT : CHANGE_EVENT; invariant( type != null && mtime_ms != null && size != null, 'Watchman file change event for "%s" missing some requested metadata. ' + @@ -287,11 +285,11 @@ export default class WatchmanWatcher extends EventEmitter { if ( // Change event on dirs are mostly useless. - !(type === 'd' && eventType === CHANGE_EVENT) + !(type === 'd' && !isNew) ) { const mtime = Number(mtime_ms); self._emitEvent({ - event: eventType, + event: TOUCH_EVENT, relativePath, metadata: { modifiedTime: mtime !== 0 ? mtime : null, diff --git a/packages/metro-file-map/src/watchers/__tests__/helpers.js b/packages/metro-file-map/src/watchers/__tests__/helpers.js index 250e4874b..05e72fced 100644 --- a/packages/metro-file-map/src/watchers/__tests__/helpers.js +++ b/packages/metro-file-map/src/watchers/__tests__/helpers.js @@ -65,11 +65,11 @@ export type EventHelpers = { untilEvent: ( afterFn: () => Promise, expectedPath: string, - expectedEvent: 'add' | 'delete' | 'change', + expectedEvent: 'touch' | 'delete', ) => Promise, allEvents: ( afterFn: () => Promise, - events: $ReadOnlyArray<[string, 'add' | 'delete' | 'change']>, + events: $ReadOnlyArray<[string, 'touch' | 'delete']>, opts?: {rejectUnexpected: boolean}, ) => Promise, }; @@ -122,7 +122,7 @@ export const startWatching = async ( }>((resolve, reject) => { const listener = (change: WatcherBackendChangeEvent) => { if (change.relativePath === '') { - // FIXME: FSEventsWatcher sometimes reports 'change' events to + // FIXME: FSEventsWatcher sometimes reports 'touch' events to // the watch root. return; } @@ -160,7 +160,7 @@ export const startWatching = async ( ); const listener = (change: WatcherBackendChangeEvent) => { if (change.relativePath === '') { - // FIXME: FSEventsWatcher sometimes reports 'change' events to + // FIXME: FSEventsWatcher sometimes reports 'touch' events to // the watch root. return; } 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 bad98e1f7..5d980c2e6 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -52,7 +52,7 @@ describe.each(Object.keys(WATCHERS))( symlink('target', join(watchRoot, 'existing', 'symlink-to-delete')), ]); - // Short delay to ensure that 'add' events for the files above are not + // Short delay to ensure that 'touch' events for the files above are not // reported by the OS to the watcher we haven't established yet. await new Promise(resolve => setTimeout(resolve, 100)); @@ -77,7 +77,7 @@ describe.each(Object.keys(WATCHERS))( beforeEach(async () => { expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({ path: 'app', - eventType: 'add', + eventType: 'touch', metadata: expect.any(Object), }); }); @@ -90,7 +90,7 @@ describe.each(Object.keys(WATCHERS))( await eventHelpers.nextEvent(() => writeFile(join(watchRoot, cookieName), ''), ), - ).toMatchObject({path: cookieName, eventType: 'add'}); + ).toMatchObject({path: cookieName, eventType: 'touch'}); // Cleanup and wait until the app root deletion is reported - this should // be the last cleanup event emitted. await eventHelpers.untilEvent( @@ -112,7 +112,7 @@ describe.each(Object.keys(WATCHERS))( await eventHelpers.nextEvent(() => writeFile(testFile, 'hello world')), ).toStrictEqual({ path: relativePath, - eventType: 'add', + eventType: 'touch', metadata: { type: 'f', modifiedTime: expect.any(Number), @@ -128,7 +128,7 @@ describe.each(Object.keys(WATCHERS))( ), ).toStrictEqual({ path: relativePath, - eventType: 'change', + eventType: 'touch', metadata: expect.any(Object), }); expect( @@ -151,7 +151,7 @@ describe.each(Object.keys(WATCHERS))( await eventHelpers.nextEvent(() => symlink(target, newLink)), ).toStrictEqual({ path: relativePath, - eventType: 'add', + eventType: 'touch', metadata: { type: 'l', modifiedTime: expect.any(Number), @@ -201,7 +201,7 @@ describe.each(Object.keys(WATCHERS))( ), ).toStrictEqual({ path: join('existing', 'file-to-modify.js'), - eventType: 'change', + eventType: 'touch', metadata: expect.any(Object), }); }); @@ -211,7 +211,7 @@ describe.each(Object.keys(WATCHERS))( await eventHelpers.nextEvent(() => mkdir(join(watchRoot, 'newdir'))), ).toStrictEqual({ path: join('newdir'), - eventType: 'add', + eventType: 'touch', metadata: { modifiedTime: expect.any(Number), size: expect.any(Number), @@ -224,7 +224,7 @@ describe.each(Object.keys(WATCHERS))( ), ).toStrictEqual({ path: join('newdir', 'file-in-new-dir.js'), - eventType: 'add', + eventType: 'touch', metadata: { modifiedTime: expect.any(Number), size: expect.any(Number), @@ -246,10 +246,10 @@ describe.each(Object.keys(WATCHERS))( ]); }, [ - [join('app', 'subdir'), 'add'], - [join('app', 'subdir', 'subdir2'), 'add'], - [join('app', 'subdir', 'deep.js'), 'add'], - [join('app', 'subdir', 'subdir2', 'deeper.js'), 'add'], + [join('app', 'subdir'), 'touch'], + [join('app', 'subdir', 'subdir2'), 'touch'], + [join('app', 'subdir', 'deep.js'), 'touch'], + [join('app', 'subdir', 'subdir2', 'deeper.js'), 'touch'], ], {rejectUnexpected: true}, ); diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index f65631bce..334334229 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -29,9 +29,8 @@ const walker = require('walker'); /** * Constants */ -export const CHANGE_EVENT = 'change'; export const DELETE_EVENT = 'delete'; -export const ADD_EVENT = 'add'; +export const TOUCH_EVENT = 'touch'; export const ALL_EVENT = 'all'; export type WatcherOptions = $ReadOnly<{ From 9740e9962c66b230f76f4035f6fd5de387aab30f Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 18:00:35 +0000 Subject: [PATCH 4/4] FSEventsWatcher: Don't track files Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: --- .../src/watchers/FSEventsWatcher.js | 35 ++----------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index b2aa576eb..830dd17eb 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -16,7 +16,7 @@ import type { // $FlowFixMe[untyped-type-import] import type {FSEvents} from 'fsevents'; -import {isIncluded, recReaddir, typeFromStat} from './common'; +import {isIncluded, typeFromStat} from './common'; import EventEmitter from 'events'; import {promises as fsPromises} from 'fs'; import * as path from 'path'; @@ -48,8 +48,6 @@ export default class FSEventsWatcher extends EventEmitter { +dot: boolean; +doIgnore: (path: string) => boolean; +fsEventsWatchStopper: () => Promise; - +watcherInitialReaddirPromise: Promise; - _tracked: Set; static isSupported(): boolean { return fsevents != null; @@ -94,40 +92,17 @@ export default class FSEventsWatcher extends EventEmitter { }); debug(`Watching ${this.root}`); - - this._tracked = new Set(); - const trackPath = (filePath: string) => { - this._tracked.add(path.normalize(filePath)); - }; - this.watcherInitialReaddirPromise = new Promise(resolve => { - recReaddir( - this.root, - trackPath, - trackPath, - trackPath, - () => { - this.emit('ready'); - resolve(); - }, - (...args) => { - this.emit('error', ...args); - resolve(); - }, - this.ignored, - ); - }); } /** * End watching. */ async close(callback?: () => void): Promise { - await this.watcherInitialReaddirPromise; await this.fsEventsWatchStopper(); this.removeAllListeners(); await new Promise(resolve => { - // it takes around 100ms for fsevents to release its resounces after + // it takes around 100ms for fsevents to release its resources after // watching is stopped. See __tests__/server-torn-down-test.js setTimeout(() => { if (typeof callback === 'function') { @@ -167,13 +142,7 @@ export default class FSEventsWatcher extends EventEmitter { return; } - // Ignore files that aren't tracked and don't exist. - if (!this._tracked.has(filepath)) { - return; - } - this._emit({event: DELETE_EVENT, relativePath}); - this._tracked.delete(filepath); } }