Skip to content

metro-file-map: Start watching while the cache is read #2221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
robhogan wants to merge 1 commit into from
+198 −71
Draft

metro-file-map: Start watching while the cache is read #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 51 additions & 26 deletions packages/metro-file-map/src/Watcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ type WatcherOptions = {
healthCheckFilePrefix: string,
ignoreForCrawl: (filePath: string) => boolean,
ignorePatternForWatch: RegExp,
previousState: CrawlerOptions['previousState'],
perfLogger: ?PerfLogger,
roots: ReadonlyArray<string>,
rootDir: string,
Expand Down Expand Up @@ -107,6 +106,9 @@ export class Watcher extends EventEmitter {
#nextHealthCheckId: number = 0;
#onChange: ?(change: WatcherBackendChangeEvent) => void;
readonly #options: WatcherOptions;
// What prepareCrawl() started, resolving to the node crawl's watchers, if
// any.
#prepared: ?Promise<?ReadonlyArray<?CrawlWatcher>>;
readonly #pendingHealthChecks: Map<
/* basename */ string,
/* resolve */ () => void,
Expand All @@ -122,36 +124,59 @@ export class Watcher extends EventEmitter {
this.#useWatchmanWatcher = options.useWatchman;
}

async crawl(): Promise<CrawlResult> {
this.#options.perfLogger?.point('crawl_start');
const options = this.#options;
/**
* Start anything that must be running before the crawl begins, which in
* watch mode without Watchman is the backends, with their watches
* confirmed. crawl() waits for this, and calling it earlier lets other work,
* such as reading a cache, overlap with it.
*/
async prepareCrawl(): Promise<void> {
await this.#prepareCrawl();
}

// Non-Watchman backends have no clock to subscribe `since`, so they must
// already be watching before the crawl begins, and their events buffered,
// for changes made while crawling not to be missed. Watchman backends are
// started by watch(), which subscribes `since` the crawl's clock to cover
// the same window. If the Watchman crawl falls back to the node crawler,
// there is no clock, so #crawl() starts non-Watchman backends before the
// node crawl instead.
//
// The node crawler lists every directory under the roots, so a
// FallbackWatcher started for it can watch each just before it is listed,
// instead of walking the roots again.
let crawlWatchers: ?ReadonlyArray<?CrawlWatcher> = null;
if (!options.useWatchman) {
const crawlFed =
options.crawl == null && this.#getBackendClass() === FallbackWatcher;
await this.#startBackendsBeforeCrawl({crawlFed});
if (crawlFed) {
crawlWatchers = this.#backends.map(backend =>
backend instanceof FallbackWatcher ? backend.getCrawlWatcher() : null,
);
#prepareCrawl(): Promise<?ReadonlyArray<?CrawlWatcher>> {
this.#prepared ??= (async () => {
const options = this.#options;
options.perfLogger?.point('prepareCrawl_start');
// Non-Watchman backends have no clock to subscribe `since`, so they must
// already be watching before the crawl begins, and their events
// buffered, for changes made while crawling not to be missed. Watchman
// backends are started by watch(), which subscribes `since` the crawl's
// clock to cover the same window. If the Watchman crawl falls back to
// the node crawler, there is no clock, so #crawl() starts non-Watchman
// backends before the node crawl instead.
let crawlWatchers: ?ReadonlyArray<?CrawlWatcher> = null;
if (!options.useWatchman) {
// The node crawler lists every directory under the roots, so a
// FallbackWatcher started for it can watch each just before it is
// listed, instead of walking the roots again.
const crawlFed =
options.crawl == null && this.#getBackendClass() === FallbackWatcher;
await this.#startBackendsBeforeCrawl({crawlFed});
if (crawlFed) {
crawlWatchers = this.#backends.map(backend =>
backend instanceof FallbackWatcher
? backend.getCrawlWatcher()
: null,
);
}
}
}
options.perfLogger?.point('prepareCrawl_end');
return crawlWatchers;
})();
return this.#prepared;
}

async crawl(
previousState: CrawlerOptions['previousState'],
): Promise<CrawlResult> {
this.#options.perfLogger?.point('crawl_start');
const options = this.#options;
const crawlWatchers = await this.#prepareCrawl();

const result = await this.#crawl({
crawlWatchers,
previousState: options.previousState,
previousState,
roots: options.roots,
useWatchman: options.useWatchman,
});
Expand Down
98 changes: 68 additions & 30 deletions packages/metro-file-map/src/__tests__/Watcher-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,6 @@ function createWatcher(
ignoreForCrawl: () => false,
ignorePatternForWatch: /^$/,
perfLogger: null,
previousState: {
clocks: new Map(),
fileSystem: new TreeFS({
processFile: () => {
throw new Error('not implemented');
},
rootDir,
}),
},
rootDir,
roots: [rootDir],
useWatchman,
Expand All @@ -173,6 +164,18 @@ function createWatcher(
});
}

function emptyState(): CrawlerOptions['previousState'] {
return {
clocks: new Map(),
fileSystem: new TreeFS({
processFile: () => {
throw new Error('not implemented');
},
rootDir,
}),
};
}

beforeEach(() => {
mockStartedBackends.length = 0;
mockNativeLiveOnStart = true;
Expand All @@ -191,7 +194,7 @@ describe('Watcher crawl fallback', () => {
const canonicalPath = path.join('src', 'index.js');
mockNodeCrawl.mockImplementation(async () => crawlResult([canonicalPath]));

const result = await createWatcher(false).crawl();
const result = await createWatcher(false).crawl(emptyState());

expect([...result.changedFiles.keys()]).toEqual([canonicalPath]);
expect(mockNodeCrawl).toHaveBeenCalledTimes(1);
Expand All @@ -201,7 +204,7 @@ describe('Watcher crawl fallback', () => {
test('a failing node crawler propagates, with no retry', async () => {
mockNodeCrawl.mockImplementation(rejectWith('node crawl failed'));

await expect(createWatcher(false).crawl()).rejects.toThrow(
await expect(createWatcher(false).crawl(emptyState())).rejects.toThrow(
'node crawl failed',
);
// The retry exists for Watchman only - retrying node with node is pointless.
Expand All @@ -214,7 +217,7 @@ describe('Watcher crawl fallback', () => {
crawlResult([canonicalPath]),
);

const result = await createWatcher(true).crawl();
const result = await createWatcher(true).crawl(emptyState());

expect([...result.changedFiles.keys()]).toEqual([canonicalPath]);
expect(mockWatchmanCrawl).toHaveBeenCalledTimes(1);
Expand All @@ -226,7 +229,7 @@ describe('Watcher crawl fallback', () => {
mockWatchmanCrawl.mockImplementation(rejectWith('watchman unavailable'));
mockNodeCrawl.mockImplementation(async () => crawlResult([canonicalPath]));

const result = await createWatcher(true).crawl();
const result = await createWatcher(true).crawl(emptyState());

expect([...result.changedFiles.keys()]).toEqual([canonicalPath]);
expect(mockNodeCrawl).toHaveBeenCalledTimes(1);
Expand All @@ -238,7 +241,7 @@ describe('Watcher crawl fallback', () => {
mockNodeCrawl.mockImplementation(rejectWith('node crawl failed'));

const thrown = await createWatcher(true)
.crawl()
.crawl(emptyState())
.then(
() => null,
(error: Error) => error,
Expand All @@ -253,7 +256,7 @@ describe('Watcher crawl fallback', () => {
mockWatchmanCrawl.mockImplementation(rejectWith('watchman unavailable'));
mockNodeCrawl.mockImplementation(async () => crawlResult([]));

await createWatcher(true).crawl();
await createWatcher(true).crawl(emptyState());

expect(mockNodeCrawl.mock.calls[0][0]).toBe(
mockWatchmanCrawl.mock.calls[0][0],
Expand All @@ -273,7 +276,7 @@ describe('Watcher backend choice in watch mode', () => {
mockWatchmanCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(true, null, true);

await watcher.crawl();
await watcher.crawl(emptyState());
expect(mockStartedBackends).toEqual([]);

await watcher.watch(() => {});
Expand All @@ -284,7 +287,7 @@ describe('Watcher backend choice in watch mode', () => {
mockWatchmanCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(true, null, true);

await watcher.crawl();
await watcher.crawl(emptyState());
await watcher.watch(() => {}, new Map([['', 'c:crawl-clock:1']]));
const addedRoot = path.join(path.sep, 'other');
await watcher.watchRoot(addedRoot);
Expand All @@ -307,7 +310,7 @@ describe('Watcher backend choice in watch mode', () => {
});
const watcher = createWatcher(true, null, true);

await watcher.crawl();
await watcher.crawl(emptyState());
const onChange = jest.fn<[WatcherBackendChangeEvent], void>();
await watcher.watch(onChange);

Expand Down Expand Up @@ -366,7 +369,7 @@ describe('Watcher with a backend that is not live on start', () => {
const watcher = createWatcher(false, null, true);
const warn = jest.spyOn(console, 'warn');

await watcher.crawl();
await watcher.crawl(emptyState());

// The first health check went unreported, the second was reported.
expect(writesBeforeCrawl).toBe(2);
Expand All @@ -386,7 +389,7 @@ describe('Watcher with a backend that is not live on start', () => {
const watcher = createWatcher(false, null, true);
const warn = jest.spyOn(console, 'warn');

const crawled = watcher.crawl();
const crawled = watcher.crawl(emptyState());
// $FlowFixMe[prop-missing] - in Jest 29.5+, missing from flow-typed's defs
await jest.advanceTimersByTimeAsync(11000);
await crawled;
Expand All @@ -402,7 +405,7 @@ describe('Watcher with a backend that is not live on start', () => {
test('confirms the watch of a root added later', async () => {
mockNodeCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(false, null, true);
await watcher.crawl();
await watcher.crawl(emptyState());
await watcher.watch(() => {});
writtenFiles = [];

Expand All @@ -412,12 +415,30 @@ describe('Watcher with a backend that is not live on start', () => {
expect(writtenFiles.map(f => path.dirname(f))).toEqual([addedRoot]);
});

test('prepareCrawl() starts and confirms the watch, which the crawl then uses', async () => {
mockNodeCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(false, null, true);

await watcher.prepareCrawl();

expect(mockStartedBackends.map(b => b.root)).toEqual([rootDir]);
expect(writtenFiles.map(f => path.dirname(f))).toEqual([rootDir]);
expect(mockNodeCrawl).not.toHaveBeenCalled();

await watcher.crawl(emptyState());

// Neither started nor confirmed again
expect(mockStartedBackends).toHaveLength(1);
expect(writtenFiles).toHaveLength(1);
expect(mockNodeCrawl).toHaveBeenCalledTimes(1);
});

test('a backend that is live on start is not checked', async () => {
mockNativeLiveOnStart = true;
mockNodeCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(false, null, true);

await watcher.crawl();
await watcher.crawl(emptyState());

expect(writtenFiles).toEqual([]);
});
Expand All @@ -441,15 +462,32 @@ describe('Watcher with FallbackWatcher and the node crawler', () => {
});
const watcher = createWatcher(false, null, true);

await watcher.crawl();
await watcher.crawl(emptyState());

expect(mockStartedBackends[0].crawledDirectories).toEqual([src]);
});

test('after prepareCrawl(), the crawl reports to the watcher it started', async () => {
const src = path.join(rootDir, 'src');
mockNodeCrawl.mockImplementation(async (options, crawlWatchers) => {
crawlWatchers?.[0]?.onDirectory(src);
return crawlResult([]);
});
const watcher = createWatcher(false, null, true);

await watcher.prepareCrawl();
await watcher.crawl(emptyState());

expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([
['fallback', true],
]);
expect(mockStartedBackends[0].crawledDirectories).toEqual([src]);
});

test('a scoped recrawl is not reported to the watcher', async () => {
mockNodeCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(false, null, true);
await watcher.crawl();
await watcher.crawl(emptyState());
mockNodeCrawl.mockClear();

await watcher.recrawl(
Expand All @@ -468,7 +506,7 @@ describe('Watcher with FallbackWatcher and the node crawler', () => {
test('with a custom crawler, the watcher walks the roots itself', async () => {
const watcher = createWatcher(false, async () => crawlResult([]), true);

await watcher.crawl();
await watcher.crawl(emptyState());

expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([
['fallback', false],
Expand All @@ -488,7 +526,7 @@ describe('Watcher crawl option', () => {
const result = await createWatcher(
true,
stubCrawler([canonicalPath]),
).crawl();
).crawl(emptyState());

expect([...result.changedFiles.keys()]).toEqual([canonicalPath]);
expect(mockWatchmanCrawl).not.toHaveBeenCalled();
Expand All @@ -500,7 +538,7 @@ describe('Watcher crawl option', () => {
stubCrawler([]),
);

await createWatcher(true, crawl).crawl();
await createWatcher(true, crawl).crawl(emptyState());

const options = crawl.mock.calls[0][0];
expect(options).toMatchObject({
Expand Down Expand Up @@ -543,9 +581,9 @@ describe('Watcher crawl option', () => {
// what a supplied crawler exists to avoid.
const crawl: Crawler = rejectWith('crawler exploded');

await expect(createWatcher(true, crawl).crawl()).rejects.toThrow(
'crawler exploded',
);
await expect(
createWatcher(true, crawl).crawl(emptyState()),
).rejects.toThrow('crawler exploded');
expect(mockNodeCrawl).not.toHaveBeenCalled();
});
});
Loading
Loading