From 76c9307ed61efa7794b30b4e585cc5941ed73e16 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 26 Sep 2022 06:28:18 -0700 Subject: [PATCH] Make Watchman existence check lazy and async Summary: (This is a change I originally made "upstream" to `jest-haste-map` in https://github.com/facebook/jest/pull/12675) Currently, `metro-file-map` checks for Watchman existence at import time (top level) with a blocking `execSync` call. This has a few downsides: - Blocking at the top level holds up main thread (by 20-200ms) at a time when there's CPU work to be done. - `execSync` spawns a shell, which isn't necessary here - `execFile` is more efficient. - The exec happens totally unnecessarily even if the consumer specifies `useWatchman: false`. This diff extracts this to an async utility function, executed lazily and memoized by the core class. Reviewed By: huntie Differential Revision: D39772133 fbshipit-source-id: 7f7d5acd3506cb0c27a8f67e8813e867f25a83ba --- .../src/__tests__/index-test.js | 6 +-- packages/metro-file-map/src/index.js | 51 +++++++++---------- .../src/lib/__tests__/canUseWatchman-test.js | 44 ++++++++++++++++ .../metro-file-map/src/lib/canUseWatchman.js | 21 ++++++++ 4 files changed, 93 insertions(+), 29 deletions(-) create mode 100644 packages/metro-file-map/src/lib/__tests__/canUseWatchman-test.js create mode 100644 packages/metro-file-map/src/lib/canUseWatchman.js diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index d7e8e5a8f4..e4385ba90d 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -17,9 +17,9 @@ function mockHashContents(contents) { return crypto.createHash('sha1').update(contents).digest('hex'); } -jest.mock('child_process', () => ({ - // If this does not throw, we'll use the (mocked) watchman crawler - execSync() {}, +jest.mock('../lib/canUseWatchman', () => ({ + __esModule: true, + default: async () => true, })); jest.mock('jest-worker', () => ({ diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index b8019df8b5..414f634dfe 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -36,6 +36,7 @@ import {DiskCacheManager} from './cache/DiskCacheManager'; import H from './constants'; import getMockName from './getMockName'; import HasteFS from './HasteFS'; +import canUseWatchman from './lib/canUseWatchman'; import deepCloneInternalData from './lib/deepCloneInternalData'; import * as fastPath from './lib/fast_path'; import getPlatformExtension from './lib/getPlatformExtension'; @@ -48,7 +49,6 @@ import NodeWatcher from './watchers/NodeWatcher'; // $FlowFixMe[untyped-import] - WatchmanWatcher import WatchmanWatcher from './watchers/WatchmanWatcher'; import {getSha1, worker} from './worker'; -import {execSync} from 'child_process'; import EventEmitter from 'events'; import invariant from 'invariant'; // $FlowFixMe[untyped-import] - jest-regex-util @@ -145,14 +145,6 @@ const VCS_DIRECTORIES = ['.git', '.hg'] .map(vcs => escapePathForRegex(path.sep + vcs + path.sep)) .join('|'); -const canUseWatchman = ((): boolean => { - try { - execSync('watchman --version', {stdio: ['ignore']}); - return true; - } catch {} - return false; -})(); - /** * HasteMap is a JavaScript implementation of Facebook's haste module system. * @@ -233,6 +225,7 @@ const canUseWatchman = ((): boolean => { export default class HasteMap extends EventEmitter { _buildPromise: ?Promise; _cachePath: Path; + _canUseWatchmanPromise: Promise; _changeInterval: ?IntervalID; _console: Console; _options: InternalOptions; @@ -787,7 +780,7 @@ export default class HasteMap extends EventEmitter { return this._worker; } - _crawl(hasteMap: InternalData): Promise this._ignore(filePath); - const crawl = - canUseWatchman && this._options.useWatchman ? watchmanCrawl : nodeCrawl; + const crawl = (await this._shouldUseWatchman()) ? watchmanCrawl : nodeCrawl; const crawlerOptions: CrawlerOptions = { abortSignal: this._crawlerAbortController.signal, computeSha1: options.computeSha1, @@ -851,11 +843,11 @@ export default class HasteMap extends EventEmitter { /** * Watch mode */ - _watch(hasteMap: InternalData): Promise { + async _watch(hasteMap: InternalData): Promise { this._options.perfLogger?.point('watch_start'); if (!this._options.watch) { this._options.perfLogger?.point('watch_end'); - return Promise.resolve(); + return; } // In watch mode, we'll only warn about module collisions and we'll retain @@ -864,12 +856,11 @@ export default class HasteMap extends EventEmitter { this._options.retainAllFiles = true; // WatchmanWatcher > FSEventsWatcher > sane.NodeWatcher - const WatcherImpl = - canUseWatchman && this._options.useWatchman - ? WatchmanWatcher - : FSEventsWatcher.isSupported() - ? FSEventsWatcher - : NodeWatcher; + const WatcherImpl = (await this._shouldUseWatchman()) + ? WatchmanWatcher + : FSEventsWatcher.isSupported() + ? FSEventsWatcher + : NodeWatcher; const extensions = this._options.extensions; const ignorePattern = this._options.ignorePattern; @@ -1067,12 +1058,10 @@ export default class HasteMap extends EventEmitter { this._changeInterval = setInterval(emitChange, CHANGE_INTERVAL); - return Promise.all(this._options.roots.map(createWatcher)).then( - watchers => { - this._watchers = watchers; - this._options.perfLogger?.point('watch_end'); - }, - ); + await Promise.all(this._options.roots.map(createWatcher)).then(watchers => { + this._watchers = watchers; + this._options.perfLogger?.point('watch_end'); + }); } /** @@ -1163,6 +1152,16 @@ export default class HasteMap extends EventEmitter { ); } + async _shouldUseWatchman(): Promise { + if (!this._options.useWatchman) { + return false; + } + if (!this._canUseWatchmanPromise) { + this._canUseWatchmanPromise = canUseWatchman(); + } + return this._canUseWatchmanPromise; + } + _createEmptyMap(): InternalData { return { clocks: new Map(), diff --git a/packages/metro-file-map/src/lib/__tests__/canUseWatchman-test.js b/packages/metro-file-map/src/lib/__tests__/canUseWatchman-test.js new file mode 100644 index 0000000000..7b7b3d90e7 --- /dev/null +++ b/packages/metro-file-map/src/lib/__tests__/canUseWatchman-test.js @@ -0,0 +1,44 @@ +/** + * 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. + * + * @format + * @flow strict + */ + +import canUseWatchman from '../canUseWatchman'; + +const mockExecFile = jest.fn(); +jest.mock('child_process', () => ({ + execFile: (...args) => mockExecFile(...args), +})); + +describe('canUseWatchman', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('executes watchman --version and returns true on success', async () => { + mockExecFile.mockImplementation((file, args, cb) => { + expect(file).toBe('watchman'); + expect(args).toStrictEqual(['--version']); + cb(null, {stdout: 'v123'}); + }); + expect(await canUseWatchman()).toBe(true); + expect(mockExecFile).toHaveBeenCalledWith( + 'watchman', + ['--version'], + expect.any(Function), + ); + }); + + it('returns false when execFile fails', async () => { + mockExecFile.mockImplementation((file, args, cb) => { + cb(new Error()); + }); + expect(await canUseWatchman()).toBe(false); + expect(mockExecFile).toHaveBeenCalled(); + }); +}); diff --git a/packages/metro-file-map/src/lib/canUseWatchman.js b/packages/metro-file-map/src/lib/canUseWatchman.js new file mode 100644 index 0000000000..a9234fe243 --- /dev/null +++ b/packages/metro-file-map/src/lib/canUseWatchman.js @@ -0,0 +1,21 @@ +/** + * 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. + * + * @format + * @flow strict + */ + +import {execFile} from 'child_process'; +import {promisify} from 'util'; + +export default async function canUseWatchman(): Promise { + try { + await promisify(execFile)('watchman', ['--version']); + return true; + } catch { + return false; + } +}