From f302c2944015ddfb61f3cc74cf2cd3522cd9e672 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 15 Dec 2022 12:45:59 -0800 Subject: [PATCH 1/8] Refactor watcher change handling to promises Summary: Some lightweight modernising of `metro-file-map` watcher backends to allow the cleaner addition of an another async step (`readLink`), to follow. (Re `graceful-fs` -> `fs` in `FSEventsWatcher` - this only affects the call to `lstat`, which is [wrapped by `graceful-fs`](https://l.facebook.com/l.php?u=https%3A%2F%2Fgithub.com%2Fisaacs%2Fnode-graceful-fs%2Fblob%2F1f19b0b467e4144260b397343cd60f37c5bdcfda%2Fpolyfills.js%23L292&h=AT1P8P4IV8T7USaqoeaSrADFV0TYg4fSes1ACxBJTf1xD0thUYMKLdwbJ9DsI-jp3yG-j52_EJE7rec2VOa0H_0k5Prll3CAT747p8IGaxvZrscX40tFFtsWIB4OAB695JLank07cGR4vH57l13fVuxo) *only* to fix a very old `guid`/`uid` reporting issue irrelevant to us - in short, `graceful-fs` isn't adding anything here). Changelog: [Internal] Reviewed By: jacdebug Differential Revision: D41522808 fbshipit-source-id: 03819fc489710d3091505a83a3babfa5aceaf4a7 --- .../src/watchers/FSEventsWatcher.js | 57 +++++++------- .../src/watchers/NodeWatcher.js | 74 +++++++++++-------- 2 files changed, 72 insertions(+), 59 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index 8bc39ef04b..9705faee38 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -9,13 +9,14 @@ */ import type {ChangeEventMetadata} from '../flow-types'; +import type {Stats} from 'fs'; // $FlowFixMe[cannot-resolve-module] - Optional, Darwin only import type {FSEvents} from 'fsevents'; // $FlowFixMe[untyped-import] - anymatch import anymatch from 'anymatch'; import EventEmitter from 'events'; -import * as fs from 'graceful-fs'; +import {promises as fsPromises} from 'fs'; import * as path from 'path'; // $FlowFixMe[untyped-import] - walker import walker from 'walker'; @@ -66,17 +67,17 @@ export default class FSEventsWatcher extends EventEmitter { } static _normalizeProxy( - callback: (normalizedPath: string, stats: fs.Stats) => void, + callback: (normalizedPath: string, stats: Stats) => void, // $FlowFixMe[cannot-resolve-name] ): (filepath: string, stats: Stats) => void { - return (filepath: string, stats: fs.Stats): void => + return (filepath: string, stats: Stats): void => callback(path.normalize(filepath), stats); } static _recReaddir( dir: string, - dirCallback: (normalizedPath: string, stats: fs.Stats) => void, - fileCallback: (normalizedPath: string, stats: fs.Stats) => void, + dirCallback: (normalizedPath: string, stats: Stats) => void, + fileCallback: (normalizedPath: string, stats: Stats) => void, // $FlowFixMe[unclear-type] Add types for callback endCallback: Function, // $FlowFixMe[unclear-type] Add types for callback @@ -123,9 +124,11 @@ export default class FSEventsWatcher extends EventEmitter { this.root = path.resolve(dir); - this.fsEventsWatchStopper = fsevents.watch(this.root, path => - this._handleEvent(path), - ); + this.fsEventsWatchStopper = fsevents.watch(this.root, path => { + this._handleEvent(path).catch(error => { + this.emit('error', error); + }); + }); debug(`Watching ${this.root}`); @@ -167,30 +170,17 @@ export default class FSEventsWatcher extends EventEmitter { : this.dot || micromatch([relativePath], '**/*').length > 0; } - _handleEvent(filepath: string) { + async _handleEvent(filepath: string) { const relativePath = path.relative(this.root, filepath); if (!this._isFileIncluded(relativePath)) { return; } - fs.lstat(filepath, (error, stat) => { - if (error && error.code !== 'ENOENT') { - this.emit('error', error); - return; - } - - if (error) { - // Ignore files that aren't tracked and don't exist. - if (!this._tracked.has(filepath)) { - return; - } - - this._emit(DELETE_EVENT, relativePath); - this._tracked.delete(filepath); - return; - } - + try { + const stat = await fsPromises.lstat(filepath); const type = typeFromStat(stat); + + // Ignore files of an unrecognized type if (!type) { return; } @@ -206,7 +196,20 @@ export default class FSEventsWatcher extends EventEmitter { this._tracked.add(filepath); this._emit(ADD_EVENT, relativePath, metadata); } - }); + } catch (error) { + if (error?.code !== 'ENOENT') { + this.emit('error', error); + return; + } + + // Ignore files that aren't tracked and don't exist. + if (!this._tracked.has(filepath)) { + return; + } + + this._emit(DELETE_EVENT, relativePath); + this._tracked.delete(filepath); + } } /** diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index ebbb3bd72f..6ebf378f11 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -25,6 +25,8 @@ const fs = require('fs'); const platform = require('os').platform(); const path = require('path'); +const fsPromises = fs.promises; + const CHANGE_EVENT = common.CHANGE_EVENT; const DELETE_EVENT = common.DELETE_EVENT; const ADD_EVENT = common.ADD_EVENT; @@ -243,29 +245,35 @@ module.exports = class NodeWatcher extends EventEmitter { if (!file) { this._detectChangedFile(dir, event, actualFile => { if (actualFile) { - this._processChange(dir, event, actualFile); + this._processChange(dir, event, actualFile).catch(error => + this.emit('error', error), + ); } }); } else { - this._processChange(dir, event, path.normalize(file)); + this._processChange(dir, event, path.normalize(file)).catch(error => + this.emit('error', error), + ); } } /** * Process changes. */ - _processChange(dir: string, event: string, file: string) { + async _processChange(dir: string, event: string, file: string) { const fullPath = path.join(dir, file); const relativePath = path.join(path.relative(this.root, dir), file); - fs.lstat(fullPath, (error, stat) => { - if (error && error.code !== 'ENOENT') { - this.emit('error', error); - } else if (!error && stat.isDirectory()) { + const registered = this._registered(fullPath); + + try { + const stat = await fsPromises.lstat(fullPath); + if (stat.isDirectory()) { + // win32 emits usless change events on dirs. if (event === 'change') { - // win32 emits usless change events on dirs. return; } + if ( stat && common.isFileIncluded( @@ -300,35 +308,37 @@ module.exports = class NodeWatcher extends EventEmitter { this.ignored, ); } + return; } else { - const registered = this._registered(fullPath); - if (error && error.code === 'ENOENT') { - this._unregister(fullPath); - this._stopWatching(fullPath); - this._unregisterDir(fullPath); - if (registered) { - this._emitEvent(DELETE_EVENT, relativePath); - } + const type = common.typeFromStat(stat); + if (type == null) { + return; + } + const metadata = { + modifiedTime: stat.mtime.getTime(), + size: stat.size, + type, + }; + if (registered) { + this._emitEvent(CHANGE_EVENT, relativePath, metadata); } else { - const type = common.typeFromStat(stat); - if (type == null) { - return; - } - const metadata = { - modifiedTime: stat.mtime.getTime(), - size: stat.size, - type, - }; - if (registered) { - this._emitEvent(CHANGE_EVENT, relativePath, metadata); - } else { - if (this._register(fullPath)) { - this._emitEvent(ADD_EVENT, relativePath, metadata); - } + if (this._register(fullPath)) { + this._emitEvent(ADD_EVENT, relativePath, metadata); } } } - }); + } catch (error) { + if (error?.code !== 'ENOENT') { + this.emit('error', error); + return; + } + this._unregister(fullPath); + this._stopWatching(fullPath); + this._unregisterDir(fullPath); + if (registered) { + this._emitEvent(DELETE_EVENT, relativePath); + } + } } /** From c8416e2ab99cdc7d666773f81699af8ea6e9ce81 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 19 Dec 2022 05:32:11 -0800 Subject: [PATCH 2/8] Add Watcher tests for behaviour on pre-existing files Summary: Refactor watcher integration tests to extract a lot of supporting code for clarity, and add some fixtures *before* creating the Watcher in order to test behaviour when pre-existing files are modified or deleted. Use those fixtures for two additional tests. (Motivation: Similar tests for symlinks have inconsistent results, fixed in the next diff) Changelog: [Internal] Reviewed By: jacdebug Differential Revision: D42110285 fbshipit-source-id: 0721cbc5a41ce3eddef7ff4c4aff0b7ffff34156 --- .../src/watchers/__tests__/helpers.js | 185 ++++++++++++++ .../watchers/__tests__/integration-test.js | 229 +++++++----------- 2 files changed, 266 insertions(+), 148 deletions(-) create mode 100644 packages/metro-file-map/src/watchers/__tests__/helpers.js diff --git a/packages/metro-file-map/src/watchers/__tests__/helpers.js b/packages/metro-file-map/src/watchers/__tests__/helpers.js new file mode 100644 index 0000000000..8cf3fed5b5 --- /dev/null +++ b/packages/metro-file-map/src/watchers/__tests__/helpers.js @@ -0,0 +1,185 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall react_native + */ + +import type {WatcherOptions} from '../common'; + +import NodeWatcher from '../NodeWatcher'; +import FSEventsWatcher from '../FSEventsWatcher'; +import WatchmanWatcher from '../WatchmanWatcher'; +import type {ChangeEventMetadata} from '../../flow-types'; +import {promises as fsPromises} from 'fs'; +import {execSync} from 'child_process'; +import {join} from 'path'; +import os from 'os'; +import invariant from 'invariant'; + +jest.useRealTimers(); + +const {mkdtemp, writeFile} = fsPromises; + +// At runtime we use a more sophisticated + robust Watchman capability check, +// but this simple heuristic is fast to check, synchronous (we can't +// asynchronously skip tests: https://github.com/facebook/jest/issues/8604), +// and will tend to exercise our Watchman tests whenever possible. +const isWatchmanOnPath = () => { + try { + execSync(os.platform() === 'windows' ? 'where watchman' : 'which watchman'); + return true; + } catch { + return false; + } +}; + +// `null` Watchers will be marked as skipped tests. +export const WATCHERS: $ReadOnly<{ + [key: string]: + | Class + | Class + | Class + | null, +}> = { + Node: NodeWatcher, + Watchman: isWatchmanOnPath() ? WatchmanWatcher : null, + FSEvents: FSEventsWatcher.isSupported() ? FSEventsWatcher : null, +}; + +export type EventHelpers = { + nextEvent: (afterFn: () => Promise) => Promise<{ + eventType: string, + path: string, + metadata?: ChangeEventMetadata, + }>, + untilEvent: ( + afterFn: () => Promise, + expectedPath: string, + expectedEvent: 'add' | 'delete' | 'change', + ) => Promise, + allEvents: ( + afterFn: () => Promise, + events: $ReadOnlyArray<[string, 'add' | 'delete' | 'change']>, + opts?: {rejectUnexpected: boolean}, + ) => Promise, +}; + +export const createTempWatchRoot = async ( + watcherName: string, + watchmanConfig: {[key: string]: mixed} | false = {}, +): Promise => { + const tmpDir = await mkdtemp( + join(os.tmpdir(), `metro-watcher-${watcherName}-test-`), + ); + + // os.tmpdir() on macOS gives us a symlink /var/foo -> /private/var/foo, + // we normalise it with realpath so that watchers report predictable + // root-relative paths for change events. + const watchRoot = await fsPromises.realpath(tmpDir); + if (watchmanConfig) { + await writeFile( + join(watchRoot, '.watchmanconfig'), + JSON.stringify(watchmanConfig), + ); + } + + return watchRoot; +}; + +export const startWatching = async ( + watcherName: string, + watchRoot: string, + opts: WatcherOptions, +): (Promise<{ + eventHelpers: EventHelpers, + stopWatching: () => Promise, +}>) => { + const Watcher = WATCHERS[watcherName]; + invariant(Watcher != null, `Watcher ${watcherName} is not supported`); + const watcherInstance = new Watcher(watchRoot, opts); + + await new Promise(resolve => { + watcherInstance.once('ready', resolve); + }); + + const eventHelpers: EventHelpers = { + nextEvent: afterFn => + Promise.all([ + new Promise<{ + eventType: string, + metadata?: ChangeEventMetadata, + path: string, + }>((resolve, reject) => { + const listener = ( + eventType: string, + path: string, + root: string, + metadata?: ChangeEventMetadata, + ) => { + if (path === '') { + // 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}`)); + } + + resolve({eventType, path, metadata}); + }; + watcherInstance.on('all', listener); + }), + afterFn(), + ]).then(([event]) => event), + + untilEvent: (afterFn, expectedPath, expectedEventType) => + eventHelpers.allEvents(afterFn, [[expectedPath, expectedEventType]], { + rejectUnexpected: false, + }), + + // $FlowFixMe[incompatible-use] + allEvents: (afterFn, expectedEvents, {rejectUnexpected = true} = {}) => + Promise.all([ + new Promise((resolve, reject) => { + const tupleToKey = (tuple: $ReadOnlyArray) => + tuple.join('\0'); + const allEventKeys = new Set( + expectedEvents.map(tuple => tupleToKey(tuple)), + ); + const listener = (eventType: string, path: string) => { + if (path === '') { + // FIXME: FSEventsWatcher sometimes reports 'change' events to + // the watch root. + return; + } + const receivedKey = tupleToKey([path, eventType]); + if (allEventKeys.has(receivedKey)) { + allEventKeys.delete(receivedKey); + if (allEventKeys.size === 0) { + watcherInstance.removeListener('all', listener); + resolve(); + } + } else if (rejectUnexpected) { + watcherInstance.removeListener('all', listener); + reject(new Error(`Unexpected event: ${eventType} ${path}.`)); + } + }; + watcherInstance.on('all', listener); + }), + afterFn(), + ]).then(() => {}), + }; + + return { + eventHelpers, + stopWatching: async () => { + await watcherInstance.close(); + }, + }; +}; 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 7e0c7423ef..51247d4cc1 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -9,46 +9,15 @@ * @oncall react_native */ +import type {EventHelpers} from './helpers'; import type {WatcherOptions} from '../common'; -import type {ChangeEventMetadata} from '../../flow-types'; -import NodeWatcher from '../NodeWatcher'; import FSEventsWatcher from '../FSEventsWatcher'; -import WatchmanWatcher from '../WatchmanWatcher'; -import {execSync} from 'child_process'; +import {createTempWatchRoot, startWatching, WATCHERS} from './helpers'; import os from 'os'; import {promises as fsPromises} from 'fs'; -import invariant from 'invariant'; import {join} from 'path'; -const {mkdtemp, mkdir, writeFile, rm, realpath, symlink, unlink} = fsPromises; - -jest.useRealTimers(); - -// At runtime we use a more sophisticated + robust Watchman capability check, -// but this simple heuristic is fast to check, synchronous (we can't -// asynchronously skip tests: https://github.com/facebook/jest/issues/8604), -// and will tend to exercise our Watchman tests whenever possible. -const isWatchmanOnPath = () => { - try { - execSync(os.platform() === 'windows' ? 'where watchman' : 'which watchman'); - return true; - } catch { - return false; - } -}; - -// `null` Watchers will be marked as skipped tests. -const WATCHERS: $ReadOnly<{ - [key: string]: - | Class - | Class - | Class - | null, -}> = { - Node: NodeWatcher, - Watchman: isWatchmanOnPath() ? WatchmanWatcher : null, - FSEvents: FSEventsWatcher.isSupported() ? FSEventsWatcher : null, -}; +const {mkdir, writeFile, rm, symlink, unlink} = fsPromises; test('FSEventsWatcher is supported if and only if darwin', () => { expect(FSEventsWatcher.isSupported()).toBe(os.platform() === 'darwin'); @@ -59,44 +28,33 @@ describe.each(Object.keys(WATCHERS))( watcherName => { let appRoot; let cookieCount = 1; - let watcherInstance; let watchRoot; - let nextEvent: (afterFn: () => Promise) => Promise<{ - eventType: string, - path: string, - metadata?: ChangeEventMetadata, - }>; - let untilEvent: ( - afterFn: () => Promise, - expectedPath: string, - expectedEvent: 'add' | 'delete' | 'change', - ) => Promise; - let allEvents: ( - afterFn: () => Promise, - events: $ReadOnlyArray<[string, 'add' | 'delete' | 'change']>, - opts?: {rejectUnexpected: boolean}, - ) => Promise; - - const Watcher = WATCHERS[watcherName]; + let stopWatching; + let eventHelpers: EventHelpers; // If all tests are skipped, Jest will not run before/after hooks either. - const maybeTest = Watcher ? test : test.skip; + const maybeTest = WATCHERS[watcherName] ? test : test.skip; beforeAll(async () => { - const tmpDir = await mkdtemp( - join(os.tmpdir(), `metro-watcher-${watcherName}-test-`), - ); + watchRoot = await createTempWatchRoot(watcherName); - // os.tmpdir() on macOS gives us a symlink /var/foo -> /private/var/foo, - // we normalise it with realpath so that watchers report predictable - // root-relative paths for change events. - watchRoot = await realpath(tmpDir); - await writeFile(join(watchRoot, '.watchmanconfig'), '{}'); - - // Perform all writes one level deeper than the watch root, so that we - // can reset file fixtures without re-establishing a watch. + // 'app' will be created and deleted before and after each test. appRoot = join(watchRoot, 'app'); + // 'existing' will *not* be reset between tests. These are fixtures used + // for testing the behaviour of the watchers on files that existed before + // the watcher was started. Tests should touch only distinct subsets of + // these files to ensure that tests remain isolated. + await mkdir(join(watchRoot, 'existing')); + await Promise.all([ + writeFile(join(watchRoot, 'existing', 'file-to-delete'), ''), + writeFile(join(watchRoot, 'existing', 'file-to-modify'), ''), + ]); + + // Short delay to ensure that 'add' 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, 10)); + const opts: WatcherOptions = { dot: true, glob: [], @@ -108,84 +66,21 @@ describe.each(Object.keys(WATCHERS))( watchmanDeferStates: [], }; - nextEvent = afterFn => - Promise.all([ - new Promise<{ - eventType: string, - metadata?: ChangeEventMetadata, - path: string, - }>((resolve, reject) => { - const listener = ( - eventType: string, - path: string, - root: string, - metadata?: ChangeEventMetadata, - ) => { - if (path === '') { - // 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}`)); - } - - resolve({eventType, path, metadata}); - }; - watcherInstance.on('all', listener); - }), - afterFn(), - ]).then(([event]) => event); - - untilEvent = (afterFn, expectedPath, expectedEventType) => - allEvents(afterFn, [[expectedPath, expectedEventType]], { - rejectUnexpected: false, - }); - - // $FlowFixMe[incompatible-use] - allEvents = (afterFn, expectedEvents, {rejectUnexpected = true} = {}) => - Promise.all([ - new Promise(async (resolve, reject) => { - const tupleToKey = (tuple: $ReadOnlyArray) => - tuple.join('\0'); - const allEventKeys = new Set( - expectedEvents.map(tuple => tupleToKey(tuple)), - ); - const listener = (eventType: string, path: string) => { - if (path === '') { - // FIXME: FSEventsWatcher sometimes reports 'change' events to - // the watch root. - return; - } - const receivedKey = tupleToKey([path, eventType]); - if (allEventKeys.has(receivedKey)) { - allEventKeys.delete(receivedKey); - if (allEventKeys.size === 0) { - watcherInstance.removeListener('all', listener); - resolve(); - } - } else if (rejectUnexpected) { - watcherInstance.removeListener('all', listener); - reject(new Error(`Unexpected event: ${eventType} ${path}.`)); - } - }; - watcherInstance.on('all', listener); - }), - afterFn(), - ]).then(() => {}); - - invariant(Watcher, 'Use of maybeTest should ensure Watcher is non-null'); - watcherInstance = new Watcher(watchRoot, opts); - await new Promise(resolve => { - watcherInstance.on('ready', resolve); - }); + ({stopWatching, eventHelpers} = await startWatching( + watcherName, + watchRoot, + opts, + )); }); beforeEach(async () => { // Discard events before app add - sometimes pre-init events are reported // after the watcher is ready. - await untilEvent(() => mkdir(appRoot), 'app', 'add'); + expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({ + path: 'app', + eventType: 'add', + metadata: expect.any(Object), + }); }); afterEach(async () => { @@ -193,15 +88,21 @@ describe.each(Object.keys(WATCHERS))( // catch double-counting, unexpected symlink traversal, etc. const cookieName = `cookie-${++cookieCount}`; expect( - await nextEvent(() => writeFile(join(watchRoot, cookieName), '')), + await eventHelpers.nextEvent(() => + writeFile(join(watchRoot, cookieName), ''), + ), ).toMatchObject({path: cookieName, eventType: 'add'}); // Cleanup and wait until the app root deletion is reported - this should // be the last cleanup event emitted. - await untilEvent(() => rm(appRoot, {recursive: true}), 'app', 'delete'); + await eventHelpers.untilEvent( + () => rm(appRoot, {recursive: true}), + 'app', + 'delete', + ); }); afterAll(async () => { - await watcherInstance.close(); + await stopWatching(); await rm(watchRoot, {recursive: true}); }); @@ -209,7 +110,7 @@ describe.each(Object.keys(WATCHERS))( const testFile = join(appRoot, 'test.js'); const relativePath = join('app', 'test.js'); expect( - await nextEvent(() => writeFile(testFile, 'hello world')), + await eventHelpers.nextEvent(() => writeFile(testFile, 'hello world')), ).toStrictEqual({ path: relativePath, eventType: 'add', @@ -223,13 +124,17 @@ describe.each(Object.keys(WATCHERS))( }, }); expect( - await nextEvent(() => writeFile(testFile, 'brave new world')), + await eventHelpers.nextEvent(() => + writeFile(testFile, 'brave new world'), + ), ).toStrictEqual({ path: relativePath, eventType: 'change', metadata: expect.any(Object), }); - expect(await nextEvent(() => unlink(testFile))).toStrictEqual({ + expect( + await eventHelpers.nextEvent(() => unlink(testFile)), + ).toStrictEqual({ path: relativePath, eventType: 'delete', metadata: undefined, @@ -244,7 +149,9 @@ describe.each(Object.keys(WATCHERS))( ])('detects new and deleted symlink to %s', async target => { const newLink = join(appRoot, 'newlink'); const relativePath = join('app', 'newlink'); - expect(await nextEvent(() => symlink(target, newLink))).toStrictEqual({ + expect( + await eventHelpers.nextEvent(() => symlink(target, newLink)), + ).toStrictEqual({ path: relativePath, eventType: 'add', metadata: { @@ -253,17 +160,43 @@ describe.each(Object.keys(WATCHERS))( size: expect.any(Number), }, }); - expect(await nextEvent(() => unlink(newLink))).toStrictEqual({ - path: relativePath, + expect(await eventHelpers.nextEvent(() => unlink(newLink))).toStrictEqual( + { + path: relativePath, + eventType: 'delete', + metadata: undefined, + }, + ); + }); + + maybeTest('detects deletion of a pre-existing file', async () => { + expect( + await eventHelpers.nextEvent(() => + unlink(join(watchRoot, 'existing', 'file-to-delete')), + ), + ).toStrictEqual({ + path: join('existing', 'file-to-delete'), eventType: 'delete', metadata: undefined, }); }); + maybeTest('detects change to a pre-existing file as a change', async () => { + expect( + await eventHelpers.nextEvent(() => + writeFile(join(watchRoot, 'existing', 'file-to-modify'), 'changed'), + ), + ).toStrictEqual({ + path: join('existing', 'file-to-modify'), + eventType: 'change', + metadata: expect.any(Object), + }); + }); + maybeTest( 'emits deletion for all files when a directory is deleted', async () => { - await allEvents( + await eventHelpers.allEvents( async () => { await mkdir(join(appRoot, 'subdir', 'subdir2'), {recursive: true}); await Promise.all([ @@ -281,7 +214,7 @@ describe.each(Object.keys(WATCHERS))( {rejectUnexpected: true}, ); - await allEvents( + await eventHelpers.allEvents( async () => { await rm(join(appRoot, 'subdir'), {recursive: true}); }, From c4055fb0780b54f966825aa5e4f87410ae3df419 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 19 Dec 2022 05:32:11 -0800 Subject: [PATCH 3/8] Consistent reporting of symlink changes across watcher backends Summary: Currently, symlinks are reported (similarly to regular files) by all watcher backends when added or removed (if not ignored), *except* in some specific cases: - Symlinks discovered when adding a new subtree are not always reported by `NodeWatcher`. - Symlinks deleted are not reported by `FSEventsWatcher` or `NodeWatcher`. That's simply because we use `walker` to scan files, and it distinguishes symlinks from regular files - we were not listening to the `symlink` event. - In the case of symlinks in very new directories, that meant we didn't emit an `add` event unless we already had a watch on the parent - the scan is expected to race against the recursive watch, so this was non-deterministic. - In the case of symlink deletions, these were not reported by `NodeWatcher` or `FSEventsWatcher` because those rely on an internal set of "tracked" files provided by `walker`, and don't emit events for untracked files that no longer exist. This diff adds a listener for discovered symlinks and tests to verify pre-existing symlinks correctly have their deletion reported. Changelog: [Internal] *(symlink changes are not yet propagated out of `metro-file-map`, so this fix isn't technically observable)* Reviewed By: jacdebug Differential Revision: D41774723 fbshipit-source-id: 0e9e7d0ecd0bdfbb316af7657e7f2599b5dfb49d --- .../src/watchers/FSEventsWatcher.js | 14 +++++----- .../src/watchers/NodeWatcher.js | 27 +++++++++++++++---- .../watchers/__tests__/integration-test.js | 17 +++++++++--- .../metro-file-map/src/watchers/common.js | 2 ++ 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index 9705faee38..4cdc0a1070 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -78,6 +78,7 @@ export default class FSEventsWatcher extends EventEmitter { dir: string, dirCallback: (normalizedPath: string, stats: Stats) => void, fileCallback: (normalizedPath: string, stats: Stats) => void, + symlinkCallback: (normalizedPath: string, stats: Stats) => void, // $FlowFixMe[unclear-type] Add types for callback endCallback: Function, // $FlowFixMe[unclear-type] Add types for callback @@ -90,6 +91,7 @@ export default class FSEventsWatcher extends EventEmitter { ) .on('dir', FSEventsWatcher._normalizeProxy(dirCallback)) .on('file', FSEventsWatcher._normalizeProxy(fileCallback)) + .on('symlink', FSEventsWatcher._normalizeProxy(symlinkCallback)) .on('error', errorCallback) .on('end', () => { endCallback(); @@ -133,14 +135,14 @@ export default class FSEventsWatcher extends EventEmitter { debug(`Watching ${this.root}`); this._tracked = new Set(); + const trackPath = (filePath: string) => { + this._tracked.add(filePath); + }; FSEventsWatcher._recReaddir( this.root, - (filepath: string) => { - this._tracked.add(filepath); - }, - (filepath: string) => { - this._tracked.add(filepath); - }, + trackPath, + trackPath, + trackPath, // $FlowFixMe[method-unbinding] - Refactor this.emit.bind(this, 'ready'), // $FlowFixMe[method-unbinding] - Refactor diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index 6ebf378f11..1279c16fe0 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -70,7 +70,10 @@ module.exports = class NodeWatcher extends EventEmitter { this._watchdir(dir); }, filename => { - this._register(filename); + this._register(filename, 'f'); + }, + symlink => { + this._register(symlink, 'l'); }, () => { this.emit('ready'); @@ -94,7 +97,7 @@ module.exports = class NodeWatcher extends EventEmitter { * * Return false if ignored or already registered. */ - _register(filepath: string): boolean { + _register(filepath: string, type: ChangeEventMetadata['type']): boolean { const dir = path.dirname(filepath); const filename = path.basename(filepath); if (this._dirRegistry[dir] && this._dirRegistry[dir][filename]) { @@ -103,6 +106,7 @@ module.exports = class NodeWatcher extends EventEmitter { const relativePath = path.relative(this.root, filepath); if ( + type === 'f' && !common.isFileIncluded(this.globs, this.dot, this.doIgnore, relativePath) ) { return false; @@ -173,7 +177,7 @@ module.exports = class NodeWatcher extends EventEmitter { watcher.on('error', this._checkedEmitError); if (this.root !== dir) { - this._register(dir); + this._register(dir, 'd'); } return true; }; @@ -295,7 +299,7 @@ module.exports = class NodeWatcher extends EventEmitter { } }, (file, stats) => { - if (this._register(file)) { + if (this._register(file, 'f')) { this._emitEvent(ADD_EVENT, path.relative(this.root, file), { modifiedTime: stats.mtime.getTime(), size: stats.size, @@ -303,6 +307,19 @@ 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', + }, + ); + } + }, function endCallback() {}, this._checkedEmitError, this.ignored, @@ -322,7 +339,7 @@ module.exports = class NodeWatcher extends EventEmitter { if (registered) { this._emitEvent(CHANGE_EVENT, relativePath, metadata); } else { - if (this._register(fullPath)) { + if (this._register(fullPath, type)) { this._emitEvent(ADD_EVENT, relativePath, metadata); } } 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 51247d4cc1..ba4b03520b 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -49,11 +49,12 @@ describe.each(Object.keys(WATCHERS))( await Promise.all([ writeFile(join(watchRoot, 'existing', 'file-to-delete'), ''), writeFile(join(watchRoot, 'existing', 'file-to-modify'), ''), + symlink('target', join(watchRoot, 'existing', 'symlink-to-delete')), ]); // Short delay to ensure that 'add' 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, 10)); + await new Promise(resolve => setTimeout(resolve, 100)); const opts: WatcherOptions = { dot: true, @@ -74,8 +75,6 @@ describe.each(Object.keys(WATCHERS))( }); beforeEach(async () => { - // Discard events before app add - sometimes pre-init events are reported - // after the watcher is ready. expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({ path: 'app', eventType: 'add', @@ -181,6 +180,18 @@ describe.each(Object.keys(WATCHERS))( }); }); + maybeTest('detects deletion of a pre-existing symlink', async () => { + expect( + await eventHelpers.nextEvent(() => + unlink(join(watchRoot, 'existing', 'symlink-to-delete')), + ), + ).toStrictEqual({ + path: join('existing', 'symlink-to-delete'), + eventType: 'delete', + metadata: undefined, + }); + }); + maybeTest('detects change to a pre-existing file as a change', async () => { expect( await eventHelpers.nextEvent(() => diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index 17cf33fc8a..07e999e83b 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -113,6 +113,7 @@ export function recReaddir( dir: string, dirCallback: (string, Stats) => void, fileCallback: (string, Stats) => void, + symlinkCallback: (string, Stats) => void, endCallback: () => void, errorCallback: Error => void, ignored: ?(boolean | RegExp), @@ -121,6 +122,7 @@ export function recReaddir( .filterDir(currentDir => !anymatch(ignored, currentDir)) .on('dir', normalizeProxy(dirCallback)) .on('file', normalizeProxy(fileCallback)) + .on('symlink', normalizeProxy(symlinkCallback)) .on('error', errorCallback) .on('end', () => { if (platform === 'win32') { From 740e9f0139409a8dcef07e0d0f9515b61f64ac55 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 30 Dec 2022 03:31:23 -0800 Subject: [PATCH 4/8] Fix Windows platform detection, Watchman detection in tests Summary: While checking some `metro-file-map` behaviour on Windows I noticed the Watchman detection we use in integration tests was all wrong :) - `os.platform()` returns `win32` on Windows, not `windows`. - `where` works in a command prompt but not PowerShell - there it's usually an alias for `Where-Object`. `where.exe` works on both. - `where.exe` prints "INFO: Could not find files for the given pattern(s)" to stderr on no match. Use `/Q` for silent running - the exit code is all we need. Reviewed By: motiz88 Differential Revision: D42266748 fbshipit-source-id: 4494cd39dd94da7dc17442264a3828a35aaf9c30 --- packages/metro-file-map/src/watchers/__tests__/helpers.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/metro-file-map/src/watchers/__tests__/helpers.js b/packages/metro-file-map/src/watchers/__tests__/helpers.js index 8cf3fed5b5..58ead90d7a 100644 --- a/packages/metro-file-map/src/watchers/__tests__/helpers.js +++ b/packages/metro-file-map/src/watchers/__tests__/helpers.js @@ -31,7 +31,9 @@ const {mkdtemp, writeFile} = fsPromises; // and will tend to exercise our Watchman tests whenever possible. const isWatchmanOnPath = () => { try { - execSync(os.platform() === 'windows' ? 'where watchman' : 'which watchman'); + execSync( + os.platform() === 'win32' ? 'where.exe /Q watchman' : 'which watchman', + ); return true; } catch { return false; From b6d51d8d12c6d577344809c544b2c7b0e3edd4af Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 13 Feb 2023 14:30:39 -0800 Subject: [PATCH 5/8] Watcher backends: Remove unused `hasIgnore` Summary: The various "ignore" mechanisms in `metro-file-map` are in a bit of tangle - we have explicit and implicit glob ignores, a set of regex arrays, and a `dot` parameter to exclude dotfiles. This is a small step in the right direction by removing the `hasIgnore` variable that's always true for Metro, and has odd side effects when it's false. Changelog: [Internal] (There's no observable effect on Metro since we always set `ignorePattern`, and so `hasIgnore` is always `true`) Reviewed By: motiz88 Differential Revision: D43234562 fbshipit-source-id: 166dca72774f3d6c3fb8c6825bf1a8d429439999 --- packages/metro-file-map/src/watchers/FSEventsWatcher.js | 4 ---- packages/metro-file-map/src/watchers/NodeWatcher.js | 1 - packages/metro-file-map/src/watchers/WatchmanWatcher.js | 2 -- packages/metro-file-map/src/watchers/common.js | 3 --- 4 files changed, 10 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index 4cdc0a1070..d5eeb91ab5 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -57,7 +57,6 @@ export default class FSEventsWatcher extends EventEmitter { +ignored: ?Matcher; +glob: $ReadOnlyArray; +dot: boolean; - +hasIgnore: boolean; +doIgnore: (path: string) => boolean; +fsEventsWatchStopper: () => Promise; _tracked: Set; @@ -119,9 +118,6 @@ export default class FSEventsWatcher extends EventEmitter { this.dot = opts.dot || false; this.ignored = opts.ignored; this.glob = Array.isArray(opts.glob) ? opts.glob : [opts.glob]; - - this.hasIgnore = - Boolean(opts.ignored) && !(Array.isArray(opts) && opts.length > 0); this.doIgnore = opts.ignored ? anymatch(opts.ignored) : () => false; this.root = path.resolve(dir); diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index 1279c16fe0..eb3f588190 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -48,7 +48,6 @@ module.exports = class NodeWatcher extends EventEmitter { doIgnore: string => boolean; dot: boolean; globs: $ReadOnlyArray; - hasIgnore: boolean; ignored: ?(boolean | RegExp); root: string; watched: {[key: string]: FSWatcher, __proto__: null}; diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index 53c02dce0d..4cb7d7fd96 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -46,7 +46,6 @@ export default class WatchmanWatcher extends EventEmitter { dot: boolean; doIgnore: string => boolean; globs: $ReadOnlyArray; - hasIgnore: boolean; root: string; subscriptionName: string; watchProjectInfo: ?$ReadOnly<{ @@ -255,7 +254,6 @@ export default class WatchmanWatcher extends EventEmitter { ); if ( - this.hasIgnore && !common.isFileIncluded(this.globs, this.dot, this.doIgnore, relativePath) ) { return; diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index 07e999e83b..9b9032b504 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -49,7 +49,6 @@ interface Watcher { doIgnore: string => boolean; dot: boolean; globs: $ReadOnlyArray; - hasIgnore: boolean; ignored?: ?(boolean | RegExp); watchmanDeferStates: $ReadOnlyArray; watchmanPath?: ?string; @@ -75,8 +74,6 @@ export const assignOptions = function ( if (!Array.isArray(watcher.globs)) { watcher.globs = [watcher.globs]; } - watcher.hasIgnore = - Boolean(opts.ignored) && !(Array.isArray(opts) && opts.length > 0); watcher.doIgnore = opts.ignored != null && opts.ignored !== false ? anymatch(opts.ignored) From 615a8847fe6a09a3396d0e9130bf474b243c06ac Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 13 Feb 2023 14:30:39 -0800 Subject: [PATCH 6/8] Watcher backends: Don't apply glob filter to symlink/directory change events Summary: Metro uses the `glob` filter of the Watcher backends to include: - `package.json` - Files with a watched extension - Health check files All of these only make sense to apply against regular files - all directory events are ignored downstream, and symlinks must be included because they may (at some point) point to a directory. Separately, we have an `ignorePattern` to exclude source control patterns and the user-provided `blockList`. It continues to make sense to apply `ignorePattern` to all events, and for some backends it helps avoid walking ignored subtrees unnecessarily. Currently, backends apply both the glob filter and ignore pattern to all files. This diff: - Modifies tests to more closely resemble the way Metro uses the backends - providing a glob filter to allowlist file extensions. - Adds a `type` parameter to `common.isFileIncluded` (-> `isIncluded`) so that glob patterns are only applied to known regular files. D43214089 follows by making similar changes downstream on the `Watcher`. Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D43234543 fbshipit-source-id: f9f2289c74e7e0d4c119eee05e0ede76fe7230d4 --- .../src/watchers/FSEventsWatcher.js | 22 ++---- .../src/watchers/NodeWatcher.js | 78 +++++++++---------- .../src/watchers/WatchmanWatcher.js | 20 +++-- .../watchers/__tests__/integration-test.js | 17 ++-- .../metro-file-map/src/watchers/common.js | 12 ++- 5 files changed, 76 insertions(+), 73 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FSEventsWatcher.js b/packages/metro-file-map/src/watchers/FSEventsWatcher.js index d5eeb91ab5..7e8746a5b2 100644 --- a/packages/metro-file-map/src/watchers/FSEventsWatcher.js +++ b/packages/metro-file-map/src/watchers/FSEventsWatcher.js @@ -20,10 +20,7 @@ import {promises as fsPromises} from 'fs'; import * as path from 'path'; // $FlowFixMe[untyped-import] - walker import walker from 'walker'; -import {typeFromStat} from './common'; - -// $FlowFixMe[untyped-import] - micromatch -const micromatch = require('micromatch'); +import {isIncluded, typeFromStat} from './common'; const debug = require('debug')('Metro:FSEventsWatcher'); @@ -159,20 +156,8 @@ export default class FSEventsWatcher extends EventEmitter { } } - _isFileIncluded(relativePath: string): boolean { - if (this.doIgnore(relativePath)) { - return false; - } - return this.glob.length - ? micromatch([relativePath], this.glob, {dot: this.dot}).length > 0 - : this.dot || micromatch([relativePath], '**/*').length > 0; - } - async _handleEvent(filepath: string) { const relativePath = path.relative(this.root, filepath); - if (!this._isFileIncluded(relativePath)) { - return; - } try { const stat = await fsPromises.lstat(filepath); @@ -182,6 +167,11 @@ export default class FSEventsWatcher extends EventEmitter { if (!type) { return; } + + if (!isIncluded(type, this.glob, this.dot, this.doIgnore, relativePath)) { + return; + } + const metadata: ChangeEventMetadata = { type, modifiedTime: stat.mtime.getTime(), diff --git a/packages/metro-file-map/src/watchers/NodeWatcher.js b/packages/metro-file-map/src/watchers/NodeWatcher.js index eb3f588190..72e5975952 100644 --- a/packages/metro-file-map/src/watchers/NodeWatcher.js +++ b/packages/metro-file-map/src/watchers/NodeWatcher.js @@ -106,7 +106,7 @@ module.exports = class NodeWatcher extends EventEmitter { const relativePath = path.relative(this.root, filepath); if ( type === 'f' && - !common.isFileIncluded(this.globs, this.dot, this.doIgnore, relativePath) + !common.isIncluded('f', this.globs, this.dot, this.doIgnore, relativePath) ) { return false; } @@ -278,53 +278,49 @@ module.exports = class NodeWatcher extends EventEmitter { } if ( - stat && - common.isFileIncluded( + !common.isIncluded( + 'd', this.globs, this.dot, this.doIgnore, relativePath, ) ) { - common.recReaddir( - 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', - }); - } - }, - (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', - }); - } - }, - (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', - }, - ); - } - }, - function endCallback() {}, - this._checkedEmitError, - this.ignored, - ); + return; } - return; + common.recReaddir( + 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', + }); + } + }, + (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', + }); + } + }, + (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', + }); + } + }, + function endCallback() {}, + this._checkedEmitError, + this.ignored, + ); } else { const type = common.typeFromStat(stat); if (type == null) { diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index 4cb7d7fd96..170de07a20 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -247,14 +247,26 @@ export default class WatchmanWatcher extends EventEmitter { } = changeDescriptor; debug( - 'Handling change to: %s (new: %s, exists: %s)', + 'Handling change to: %s (new: %s, exists: %s, type: %s)', relativePath, isNew, exists, + type, ); + // Ignore files of an unrecognized type + if (type != null && !(type === 'f' || type === 'd' || type === 'l')) { + return; + } + if ( - !common.isFileIncluded(this.globs, this.dot, this.doIgnore, relativePath) + !common.isIncluded( + type, + this.globs, + this.dot, + this.doIgnore, + relativePath, + ) ) { return; } @@ -274,10 +286,8 @@ export default class WatchmanWatcher extends EventEmitter { ); if ( - type === 'f' || - type === 'l' || // Change event on dirs are mostly useless. - (type === 'd' && eventType !== CHANGE_EVENT) + !(type === 'd' && eventType === CHANGE_EVENT) ) { self._emitEvent(eventType, relativePath, self.root, { modifiedTime: Number(mtime_ms), 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 ba4b03520b..534caeec81 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -47,8 +47,8 @@ describe.each(Object.keys(WATCHERS))( // these files to ensure that tests remain isolated. await mkdir(join(watchRoot, 'existing')); await Promise.all([ - writeFile(join(watchRoot, 'existing', 'file-to-delete'), ''), - writeFile(join(watchRoot, 'existing', 'file-to-modify'), ''), + writeFile(join(watchRoot, 'existing', 'file-to-delete.js'), ''), + writeFile(join(watchRoot, 'existing', 'file-to-modify.js'), ''), symlink('target', join(watchRoot, 'existing', 'symlink-to-delete')), ]); @@ -58,7 +58,7 @@ describe.each(Object.keys(WATCHERS))( const opts: WatcherOptions = { dot: true, - glob: [], + glob: ['**/package.json', '**/*.js', '**/cookie-*'], // We need to ignore `.watchmanconfig` to keep these tests stable. // Even though we write it before initialising watchers, OS-level // delays/debouncing(?) can mean the write is *sometimes* reported by @@ -171,10 +171,10 @@ describe.each(Object.keys(WATCHERS))( maybeTest('detects deletion of a pre-existing file', async () => { expect( await eventHelpers.nextEvent(() => - unlink(join(watchRoot, 'existing', 'file-to-delete')), + unlink(join(watchRoot, 'existing', 'file-to-delete.js')), ), ).toStrictEqual({ - path: join('existing', 'file-to-delete'), + path: join('existing', 'file-to-delete.js'), eventType: 'delete', metadata: undefined, }); @@ -195,10 +195,13 @@ describe.each(Object.keys(WATCHERS))( maybeTest('detects change to a pre-existing file as a change', async () => { expect( await eventHelpers.nextEvent(() => - writeFile(join(watchRoot, 'existing', 'file-to-modify'), 'changed'), + writeFile( + join(watchRoot, 'existing', 'file-to-modify.js'), + 'changed', + ), ), ).toStrictEqual({ - path: join('existing', 'file-to-modify'), + path: join('existing', 'file-to-modify.js'), eventType: 'change', metadata: expect.any(Object), }); diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index 9b9032b504..2dbdb6d99c 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -89,7 +89,8 @@ export const assignOptions = function ( /** * Checks a file relative path against the globs array. */ -export function isFileIncluded( +export function isIncluded( + type: ?('f' | 'l' | 'd'), globs: $ReadOnlyArray, dot: boolean, doIgnore: string => boolean, @@ -98,9 +99,12 @@ export function isFileIncluded( if (doIgnore(relativePath)) { return false; } - return globs.length - ? micromatch.some(relativePath, globs, {dot}) - : dot || micromatch.some(relativePath, '**/*'); + // For non-regular files or if there are no glob matchers, just respect the + // `dot` option to filter dotfiles if dot === false. + if (globs.length === 0 || type !== 'f') { + return dot || micromatch.some(relativePath, '**/*'); + } + return micromatch.some(relativePath, globs, {dot}); } /** From 99bbe6ef279ad689aec2816290080303e0a7387d Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 21 Mar 2023 10:27:44 +0000 Subject: [PATCH 7/8] Add integration test for changed file in new directory (https://github.com/facebook/react-native/issues/36387) --- .../watchers/__tests__/integration-test.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 534caeec81..93f54b82ca 100644 --- a/packages/metro-file-map/src/watchers/__tests__/integration-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/integration-test.js @@ -207,6 +207,33 @@ describe.each(Object.keys(WATCHERS))( }); }); + maybeTest('detects changes to files in a new directory', async () => { + expect( + await eventHelpers.nextEvent(() => mkdir(join(watchRoot, 'newdir'))), + ).toStrictEqual({ + path: join('newdir'), + eventType: 'add', + metadata: { + modifiedTime: expect.any(Number), + size: expect.any(Number), + type: 'd', + }, + }); + expect( + await eventHelpers.nextEvent(() => + writeFile(join(watchRoot, 'newdir', 'file-in-new-dir.js'), 'code'), + ), + ).toStrictEqual({ + path: join('newdir', 'file-in-new-dir.js'), + eventType: 'add', + metadata: { + modifiedTime: expect.any(Number), + size: expect.any(Number), + type: 'f', + }, + }); + }); + maybeTest( 'emits deletion for all files when a directory is deleted', async () => { From b2a70b28d5042174676faa7c87bed548292611fc Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 21 Mar 2023 10:30:57 +0000 Subject: [PATCH 8/8] Bump to 0.73.9 --- lerna.json | 2 +- packages/buck-worker-tool/package.json | 4 +- packages/metro-babel-register/package.json | 2 +- packages/metro-babel-transformer/package.json | 4 +- packages/metro-cache-key/package.json | 4 +- packages/metro-cache/package.json | 6 +-- packages/metro-config/package.json | 10 ++--- packages/metro-core/package.json | 4 +- packages/metro-file-map/package.json | 2 +- packages/metro-hermes-compiler/package.json | 2 +- packages/metro-inspector-proxy/package.json | 2 +- packages/metro-memory-fs/package.json | 2 +- packages/metro-minify-terser/package.json | 2 +- packages/metro-minify-uglify/package.json | 2 +- .../package.json | 2 +- .../package.json | 8 ++-- .../package.json | 2 +- packages/metro-resolver/package.json | 2 +- packages/metro-runtime/package.json | 2 +- packages/metro-source-map/package.json | 6 +-- packages/metro-symbolicate/package.json | 4 +- packages/metro-transform-plugins/package.json | 4 +- packages/metro-transform-worker/package.json | 22 +++++----- packages/metro/package.json | 44 +++++++++---------- packages/ob1/package.json | 2 +- 25 files changed, 73 insertions(+), 73 deletions(-) diff --git a/lerna.json b/lerna.json index d7f9cd38a3..6f530da2a8 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "lerna": "2.4.0", - "version": "0.73.8", + "version": "0.73.9", "npmClient": "yarn", "useWorkspaces": true } diff --git a/packages/buck-worker-tool/package.json b/packages/buck-worker-tool/package.json index dd03f23844..f04b120d7a 100644 --- a/packages/buck-worker-tool/package.json +++ b/packages/buck-worker-tool/package.json @@ -1,6 +1,6 @@ { "name": "buck-worker-tool", - "version": "0.73.8", + "version": "0.73.9", "description": "Implementation of the Buck worker protocol for Node.js.", "license": "MIT", "main": "src/worker-tool.js", @@ -13,7 +13,7 @@ "through": ">=2.2.7 <3" }, "devDependencies": { - "metro-memory-fs": "0.73.8" + "metro-memory-fs": "0.73.9" }, "scripts": { "prepare-release": "test -d build && rm -rf src.real && mv src src.real && mv build src", diff --git a/packages/metro-babel-register/package.json b/packages/metro-babel-register/package.json index fee30c2b11..be6022c342 100644 --- a/packages/metro-babel-register/package.json +++ b/packages/metro-babel-register/package.json @@ -1,6 +1,6 @@ { "name": "metro-babel-register", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 babel/register configuration for Metro.", "main": "src/babel-register.js", "repository": { diff --git a/packages/metro-babel-transformer/package.json b/packages/metro-babel-transformer/package.json index 4818aad75d..07376bdfa3 100644 --- a/packages/metro-babel-transformer/package.json +++ b/packages/metro-babel-transformer/package.json @@ -1,6 +1,6 @@ { "name": "metro-babel-transformer", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Base Babel transformer for Metro.", "main": "src/index.js", "repository": { @@ -19,7 +19,7 @@ "dependencies": { "@babel/core": "^7.20.0", "hermes-parser": "0.8.0", - "metro-source-map": "0.73.8", + "metro-source-map": "0.73.9", "nullthrows": "^1.1.1" } } diff --git a/packages/metro-cache-key/package.json b/packages/metro-cache-key/package.json index 4713008304..a591815aa5 100644 --- a/packages/metro-cache-key/package.json +++ b/packages/metro-cache-key/package.json @@ -1,6 +1,6 @@ { "name": "metro-cache-key", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Cache key utility.", "main": "src/index.js", "repository": { @@ -13,6 +13,6 @@ }, "license": "MIT", "devDependencies": { - "metro-memory-fs": "0.73.8" + "metro-memory-fs": "0.73.9" } } diff --git a/packages/metro-cache/package.json b/packages/metro-cache/package.json index 3273c17863..9d7bc623e0 100644 --- a/packages/metro-cache/package.json +++ b/packages/metro-cache/package.json @@ -1,6 +1,6 @@ { "name": "metro-cache", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Cache layers for Metro.", "main": "src/index.js", "repository": { @@ -12,11 +12,11 @@ "cleanup-release": "test ! -e build && mv src build && mv src.real src" }, "dependencies": { - "metro-core": "0.73.8", + "metro-core": "0.73.9", "rimraf": "^3.0.2" }, "devDependencies": { - "metro-memory-fs": "0.73.8" + "metro-memory-fs": "0.73.9" }, "license": "MIT" } diff --git a/packages/metro-config/package.json b/packages/metro-config/package.json index b54fc423af..0d29dbd419 100644 --- a/packages/metro-config/package.json +++ b/packages/metro-config/package.json @@ -1,6 +1,6 @@ { "name": "metro-config", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Config parser for Metro.", "main": "src/index.js", "repository": { @@ -15,10 +15,10 @@ "dependencies": { "cosmiconfig": "^5.0.5", "jest-validate": "^26.5.2", - "metro": "0.73.8", - "metro-cache": "0.73.8", - "metro-core": "0.73.8", - "metro-runtime": "0.73.8" + "metro": "0.73.9", + "metro-cache": "0.73.9", + "metro-core": "0.73.9", + "metro-runtime": "0.73.9" }, "devDependencies": { "pretty-format": "^26.5.2", diff --git a/packages/metro-core/package.json b/packages/metro-core/package.json index c82f8dcfbf..3eb599f3ef 100644 --- a/packages/metro-core/package.json +++ b/packages/metro-core/package.json @@ -1,6 +1,6 @@ { "name": "metro-core", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Metro's core package.", "main": "src/index.js", "repository": { @@ -13,7 +13,7 @@ }, "dependencies": { "lodash.throttle": "^4.1.1", - "metro-resolver": "0.73.8" + "metro-resolver": "0.73.9" }, "license": "MIT" } diff --git a/packages/metro-file-map/package.json b/packages/metro-file-map/package.json index 5d3312a399..4b4d37cba9 100644 --- a/packages/metro-file-map/package.json +++ b/packages/metro-file-map/package.json @@ -1,6 +1,6 @@ { "name": "metro-file-map", - "version": "0.73.8", + "version": "0.73.9", "description": "[Experimental] - 🚇 File crawling, watching and mapping for Metro", "main": "src/index.js", "repository": { diff --git a/packages/metro-hermes-compiler/package.json b/packages/metro-hermes-compiler/package.json index ef0c5db0d6..bb0e9220dd 100644 --- a/packages/metro-hermes-compiler/package.json +++ b/packages/metro-hermes-compiler/package.json @@ -1,6 +1,6 @@ { "name": "metro-hermes-compiler", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Hermes bytecode compiler for Metro.", "main": "src/index.js", "repository": { diff --git a/packages/metro-inspector-proxy/package.json b/packages/metro-inspector-proxy/package.json index 3b40fc5c45..198e991c73 100644 --- a/packages/metro-inspector-proxy/package.json +++ b/packages/metro-inspector-proxy/package.json @@ -1,6 +1,6 @@ { "name": "metro-inspector-proxy", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Inspector proxy for React Native and dev tools integration.", "main": "src/index.js", "bin": "src/cli.js", diff --git a/packages/metro-memory-fs/package.json b/packages/metro-memory-fs/package.json index a3f57033e2..caeb62f035 100644 --- a/packages/metro-memory-fs/package.json +++ b/packages/metro-memory-fs/package.json @@ -1,6 +1,6 @@ { "name": "metro-memory-fs", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 A memory-based implementation of `fs` useful for testing.", "main": "src/index.js", "repository": { diff --git a/packages/metro-minify-terser/package.json b/packages/metro-minify-terser/package.json index 1b6901ab3c..f698ca447d 100644 --- a/packages/metro-minify-terser/package.json +++ b/packages/metro-minify-terser/package.json @@ -1,6 +1,6 @@ { "name": "metro-minify-terser", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Minifier for Metro based on Terser.", "main": "src/index.js", "repository": { diff --git a/packages/metro-minify-uglify/package.json b/packages/metro-minify-uglify/package.json index 9d610b36d5..19858b33cd 100644 --- a/packages/metro-minify-uglify/package.json +++ b/packages/metro-minify-uglify/package.json @@ -1,6 +1,6 @@ { "name": "metro-minify-uglify", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Minifier for Metro based on Uglify.", "main": "src/index.js", "repository": { diff --git a/packages/metro-react-native-babel-preset/package.json b/packages/metro-react-native-babel-preset/package.json index bb2a92f5df..add0f42c0b 100644 --- a/packages/metro-react-native-babel-preset/package.json +++ b/packages/metro-react-native-babel-preset/package.json @@ -1,6 +1,6 @@ { "name": "metro-react-native-babel-preset", - "version": "0.73.8", + "version": "0.73.9", "description": "Babel preset for React Native applications", "main": "src/index.js", "repository": { diff --git a/packages/metro-react-native-babel-transformer/package.json b/packages/metro-react-native-babel-transformer/package.json index 618d5a67b2..141d1465f1 100644 --- a/packages/metro-react-native-babel-transformer/package.json +++ b/packages/metro-react-native-babel-transformer/package.json @@ -1,6 +1,6 @@ { "name": "metro-react-native-babel-transformer", - "version": "0.73.8", + "version": "0.73.9", "description": "Babel transformer for React Native applications.", "main": "src/index.js", "repository": { @@ -21,9 +21,9 @@ "@babel/core": "^7.20.0", "babel-preset-fbjs": "^3.4.0", "hermes-parser": "0.8.0", - "metro-babel-transformer": "0.73.8", - "metro-react-native-babel-preset": "0.73.8", - "metro-source-map": "0.73.8", + "metro-babel-transformer": "0.73.9", + "metro-react-native-babel-preset": "0.73.9", + "metro-source-map": "0.73.9", "nullthrows": "^1.1.1" }, "peerDependencies": { diff --git a/packages/metro-react-native-interop-tools/package.json b/packages/metro-react-native-interop-tools/package.json index 856f6a1f89..85fc1a0a8e 100644 --- a/packages/metro-react-native-interop-tools/package.json +++ b/packages/metro-react-native-interop-tools/package.json @@ -1,6 +1,6 @@ { "name": "metro-react-native-interop-tools", - "version": "0.73.8", + "version": "0.73.9", "description": "Interop tools for React Native applications", "main": "src/index.js", "repository": { diff --git a/packages/metro-resolver/package.json b/packages/metro-resolver/package.json index bfa3f4b5a8..b21de7132d 100644 --- a/packages/metro-resolver/package.json +++ b/packages/metro-resolver/package.json @@ -1,6 +1,6 @@ { "name": "metro-resolver", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Implementation of Metro's resolution logic.", "main": "src", "repository": { diff --git a/packages/metro-runtime/package.json b/packages/metro-runtime/package.json index 09ca70d189..5b05bdcb6c 100644 --- a/packages/metro-runtime/package.json +++ b/packages/metro-runtime/package.json @@ -1,6 +1,6 @@ { "name": "metro-runtime", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Module required for evaluating Metro bundles.", "main": "src", "repository": { diff --git a/packages/metro-source-map/package.json b/packages/metro-source-map/package.json index b2e51388ff..9542a22cfd 100644 --- a/packages/metro-source-map/package.json +++ b/packages/metro-source-map/package.json @@ -1,6 +1,6 @@ { "name": "metro-source-map", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Source map generator for Metro.", "main": "src/source-map.js", "repository": { @@ -15,9 +15,9 @@ "@babel/traverse": "^7.20.0", "@babel/types": "^7.20.0", "invariant": "^2.2.4", - "metro-symbolicate": "0.73.8", + "metro-symbolicate": "0.73.9", "nullthrows": "^1.1.1", - "ob1": "0.73.8", + "ob1": "0.73.9", "source-map": "^0.5.6", "vlq": "^1.0.0" }, diff --git a/packages/metro-symbolicate/package.json b/packages/metro-symbolicate/package.json index 4bac5539a3..22a6f7a15b 100644 --- a/packages/metro-symbolicate/package.json +++ b/packages/metro-symbolicate/package.json @@ -1,6 +1,6 @@ { "name": "metro-symbolicate", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 A tool to find the source location from JS bundles and stack traces.", "license": "MIT", "main": "./src/index.js", @@ -21,7 +21,7 @@ ], "dependencies": { "invariant": "^2.2.4", - "metro-source-map": "0.73.8", + "metro-source-map": "0.73.9", "nullthrows": "^1.1.1", "source-map": "^0.5.6", "through2": "^2.0.1", diff --git a/packages/metro-transform-plugins/package.json b/packages/metro-transform-plugins/package.json index 205c3f1180..aa67248921 100644 --- a/packages/metro-transform-plugins/package.json +++ b/packages/metro-transform-plugins/package.json @@ -1,6 +1,6 @@ { "name": "metro-transform-plugins", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Transform plugins for Metro.", "main": "src/index.js", "repository": { @@ -24,6 +24,6 @@ "@babel/plugin-syntax-nullish-coalescing-operator": "^7.0.0", "@babel/plugin-transform-flow-strip-types": "^7.0.0", "@babel/types": "^7.20.0", - "metro": "0.73.8" + "metro": "0.73.9" } } diff --git a/packages/metro-transform-worker/package.json b/packages/metro-transform-worker/package.json index d12fcb9af1..aef192b341 100644 --- a/packages/metro-transform-worker/package.json +++ b/packages/metro-transform-worker/package.json @@ -1,6 +1,6 @@ { "name": "metro-transform-worker", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 Transform worker for Metro.", "main": "src/index.js", "repository": { @@ -18,18 +18,18 @@ "@babel/parser": "^7.20.0", "@babel/types": "^7.20.0", "babel-preset-fbjs": "^3.4.0", - "metro": "0.73.8", - "metro-babel-transformer": "0.73.8", - "metro-cache": "0.73.8", - "metro-cache-key": "0.73.8", - "metro-hermes-compiler": "0.73.8", - "metro-source-map": "0.73.8", - "metro-transform-plugins": "0.73.8", + "metro": "0.73.9", + "metro-babel-transformer": "0.73.9", + "metro-cache": "0.73.9", + "metro-cache-key": "0.73.9", + "metro-hermes-compiler": "0.73.9", + "metro-source-map": "0.73.9", + "metro-transform-plugins": "0.73.9", "nullthrows": "^1.1.1" }, "devDependencies": { - "metro-memory-fs": "0.73.8", - "metro-minify-terser": "0.73.8", - "metro-react-native-babel-transformer": "0.73.8" + "metro-memory-fs": "0.73.9", + "metro-minify-terser": "0.73.9", + "metro-react-native-babel-transformer": "0.73.9" } } diff --git a/packages/metro/package.json b/packages/metro/package.json index de97418e8f..f6736034c4 100644 --- a/packages/metro/package.json +++ b/packages/metro/package.json @@ -1,6 +1,6 @@ { "name": "metro", - "version": "0.73.8", + "version": "0.73.9", "description": "🚇 The JavaScript bundler for React Native.", "main": "src/index.js", "bin": "src/cli.js", @@ -35,23 +35,23 @@ "invariant": "^2.2.4", "jest-worker": "^27.2.0", "lodash.throttle": "^4.1.1", - "metro-babel-transformer": "0.73.8", - "metro-cache": "0.73.8", - "metro-cache-key": "0.73.8", - "metro-config": "0.73.8", - "metro-core": "0.73.8", - "metro-file-map": "0.73.8", - "metro-hermes-compiler": "0.73.8", - "metro-inspector-proxy": "0.73.8", - "metro-minify-terser": "0.73.8", - "metro-minify-uglify": "0.73.8", - "metro-react-native-babel-preset": "0.73.8", - "metro-resolver": "0.73.8", - "metro-runtime": "0.73.8", - "metro-source-map": "0.73.8", - "metro-symbolicate": "0.73.8", - "metro-transform-plugins": "0.73.8", - "metro-transform-worker": "0.73.8", + "metro-babel-transformer": "0.73.9", + "metro-cache": "0.73.9", + "metro-cache-key": "0.73.9", + "metro-config": "0.73.9", + "metro-core": "0.73.9", + "metro-file-map": "0.73.9", + "metro-hermes-compiler": "0.73.9", + "metro-inspector-proxy": "0.73.9", + "metro-minify-terser": "0.73.9", + "metro-minify-uglify": "0.73.9", + "metro-react-native-babel-preset": "0.73.9", + "metro-resolver": "0.73.9", + "metro-runtime": "0.73.9", + "metro-source-map": "0.73.9", + "metro-symbolicate": "0.73.9", + "metro-transform-plugins": "0.73.9", + "metro-transform-worker": "0.73.9", "mime-types": "^2.1.27", "node-fetch": "^2.2.0", "nullthrows": "^1.1.1", @@ -70,10 +70,10 @@ "dedent": "^0.7.0", "jest-snapshot": "^26.5.2", "jest-snapshot-serializer-raw": "^1.2.0", - "metro-babel-register": "0.73.8", - "metro-memory-fs": "0.73.8", - "metro-react-native-babel-preset": "0.73.8", - "metro-react-native-babel-transformer": "0.73.8", + "metro-babel-register": "0.73.9", + "metro-memory-fs": "0.73.9", + "metro-react-native-babel-preset": "0.73.9", + "metro-react-native-babel-transformer": "0.73.9", "mock-req": "^0.2.0", "mock-res": "^0.6.0", "stack-trace": "^0.0.10" diff --git a/packages/ob1/package.json b/packages/ob1/package.json index e73ceee676..0b8a6f4afb 100644 --- a/packages/ob1/package.json +++ b/packages/ob1/package.json @@ -1,6 +1,6 @@ { "name": "ob1", - "version": "0.73.8", + "version": "0.73.9", "description": "A small library for working with 0- and 1-based offsets in a type-checked way.", "main": "src/ob1.js", "repository": {