From 9e0bcbdd6491f1cd8d12400ec5f2959d4eed99b3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 16 Dec 2024 10:50:13 -0800 Subject: [PATCH] wip: restructure watcher backend events Differential Revision: D67289194 --- packages/metro-file-map/src/Watcher.js | 47 +++++---------- packages/metro-file-map/src/flow-types.js | 14 +++++ packages/metro-file-map/src/index.js | 59 +++++++++---------- .../src/watchers/NodeWatcher.js | 54 +++++++---------- .../src/watchers/WatchmanWatcher.js | 27 +++------ .../src/watchers/__tests__/helpers.js | 38 +++++++----- 6 files changed, 110 insertions(+), 129 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/flow-types.js b/packages/metro-file-map/src/flow-types.js index 7293c6a9fc..d65aa6da29 100644 --- a/packages/metro-file-map/src/flow-types.js +++ b/packages/metro-file-map/src/flow-types.js @@ -316,6 +316,20 @@ export type RawMockMap = Map; export type ReadOnlyRawMockMap = $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 47e2e4c9f2..b9a74e2261 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -33,6 +33,7 @@ import type { PerfLoggerFactory, WatchmanClocks, WorkerMetadata, + WatcherBackendChangeEvent, } from './flow-types'; import type {IJestWorker} from 'jest-worker'; @@ -862,27 +863,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 @@ -899,11 +896,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; } @@ -917,14 +913,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; @@ -936,7 +933,7 @@ export default class FileMap extends EventEmitter { const event = { filePath: absoluteFilePath, metadata, - type, + type: change.event, }; if (nextEmit == null) { nextEmit = { @@ -963,19 +960,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 { @@ -987,7 +984,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; @@ -999,7 +996,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. @@ -1012,7 +1009,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/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index 59e9e556d1..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'; @@ -324,11 +327,16 @@ module.exports = class NodeWatcher extends EventEmitter { }, (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() {}, @@ -374,21 +382,8 @@ module.exports = class NodeWatcher extends EventEmitter { * * See also note above for DEBOUNCE_MS. */ - _emitEvent({ - event, - relativePath, - metadata, - }: - | { - event: 'change' | 'add', - relativePath: string, - metadata: ChangeEventMetadata, - } - | { - event: 'delete', - relativePath: string, - metadata?: void, - }) { + _emitEvent(change: Omit) { + const {event, relativePath} = change; const key = event + '-' + relativePath; const addKey = ADD_EVENT + '-' + relativePath; if (event === CHANGE_EVENT && this._changeTimers.has(addKey)) { @@ -404,23 +399,14 @@ module.exports = class NodeWatcher extends EventEmitter { key, setTimeout(() => { this._changeTimers.delete(key); - this._rawEmitEvent(event, relativePath, 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(eventType, file, this.root, metadata); - 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 ee2938d9df..eb06a6328e 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.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 type { Client, @@ -306,23 +309,11 @@ export default class WatchmanWatcher extends EventEmitter { /** * Dispatches the event. */ - _emitEvent({ - event, - relativePath, - metadata, - }: - | { - event: 'change' | 'add', - relativePath: string, - metadata: ChangeEventMetadata, - } - | { - event: 'delete', - relativePath: string, - metadata?: void, - }) { - this.emit(event, relativePath, this.root, metadata); - this.emit(ALL_EVENT, event, relativePath, this.root, metadata); + _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);