Skip to content

Commit

Permalink
Make Watchman existence check lazy and async
Browse files Browse the repository at this point in the history
Summary:
(This is a change I originally made "upstream" to `jest-haste-map` in jestjs/jest#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
  • Loading branch information
robhogan authored and facebook-github-bot committed Sep 26, 2022
1 parent ab58166 commit 76c9307
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 29 deletions.
6 changes: 3 additions & 3 deletions packages/metro-file-map/src/__tests__/index-test.js
Expand Up @@ -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', () => ({
Expand Down
51 changes: 25 additions & 26 deletions packages/metro-file-map/src/index.js
Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -233,6 +225,7 @@ const canUseWatchman = ((): boolean => {
export default class HasteMap extends EventEmitter {
_buildPromise: ?Promise<InternalDataObject>;
_cachePath: Path;
_canUseWatchmanPromise: Promise<boolean>;
_changeInterval: ?IntervalID;
_console: Console;
_options: InternalOptions;
Expand Down Expand Up @@ -787,7 +780,7 @@ export default class HasteMap extends EventEmitter {
return this._worker;
}

_crawl(hasteMap: InternalData): Promise<?(
async _crawl(hasteMap: InternalData): Promise<?(
| Promise<{
changedFiles?: FileData,
hasteMap: InternalData,
Expand All @@ -798,8 +791,7 @@ export default class HasteMap extends EventEmitter {
this._options.perfLogger?.point('crawl_start');
const options = this._options;
const ignore = (filePath: string) => 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,
Expand Down Expand Up @@ -851,11 +843,11 @@ export default class HasteMap extends EventEmitter {
/**
* Watch mode
*/
_watch(hasteMap: InternalData): Promise<void> {
async _watch(hasteMap: InternalData): Promise<void> {
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
Expand All @@ -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;
Expand Down Expand Up @@ -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');
});
}

/**
Expand Down Expand Up @@ -1163,6 +1152,16 @@ export default class HasteMap extends EventEmitter {
);
}

async _shouldUseWatchman(): Promise<boolean> {
if (!this._options.useWatchman) {
return false;
}
if (!this._canUseWatchmanPromise) {
this._canUseWatchmanPromise = canUseWatchman();
}
return this._canUseWatchmanPromise;
}

_createEmptyMap(): InternalData {
return {
clocks: new Map(),
Expand Down
44 changes: 44 additions & 0 deletions 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();
});
});
21 changes: 21 additions & 0 deletions 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<boolean> {
try {
await promisify(execFile)('watchman', ['--version']);
return true;
} catch {
return false;
}
}

0 comments on commit 76c9307

Please sign in to comment.