From f981dbcea6ffba459b8f41d2b4ea0cc5b72d20e3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Sat, 21 Dec 2024 18:00:35 +0000 Subject: [PATCH] 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<{