From a72a60e7369cc4960e6e814e2ef2acc31cf93100 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 05:16:43 -0800 Subject: [PATCH] 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 3029fe0cea..38f3429145 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 9845b1b48c..d9cd8cbe32 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 41c448ae03..ced1c0f77d 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 82c62ef768..6224b915ee 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 0117eaec31..99b451985d 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 65cf44c9ad..75fa2829bb 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 bc7f852892..6a5d7e13d4 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 e852e5d435..250e4874ba 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);