Skip to content

metro-file-map: Recrawl a directory after an unnamed change in FallbackWatcher #1968

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

Closed
robhogan wants to merge 1 commit into from
Closed

metro-file-map: Recrawl a directory after an unnamed change in FallbackWatcher #1968

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
80 changes: 31 additions & 49 deletions packages/metro-file-map/src/watchers/FallbackWatcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const fsPromises = fs.promises;

const TOUCH_EVENT = common.TOUCH_EVENT;
const DELETE_EVENT = common.DELETE_EVENT;
const RECRAWL_EVENT = common.RECRAWL_EVENT;

/**
* This setting delays all events. It suppresses 'change' events that
Expand Down Expand Up @@ -236,67 +237,48 @@ export default class FallbackWatcher extends AbstractWatcher {
await Promise.all(promises);
}

/**
* On some platforms, as pointed out on the fs docs (most likely just win32)
* the file argument might be missing from the fs event. Try to detect what
* change by detecting if something was deleted or the most recent file change.
*/
#detectChangedFile(
dir: string,
event: string,
callback: (file: string) => void,
) {
if (!this.#dirRegistry[dir]) {
return;
}

let found = false;
let closest: ?Readonly<{file: string, mtime: Stats['mtime']}> = null;
let c = 0;
Object.keys(this.#dirRegistry[dir]).forEach((file, i, arr) => {
fs.lstat(path.join(dir, file), (error, stat) => {
if (found) {
return;
}

if (error) {
if (isIgnorableFileError(error)) {
found = true;
callback(file);
} else {
this.emitError(error);
}
} else {
if (closest == null || stat.mtime > closest.mtime) {
closest = {file, mtime: stat.mtime};
}
if (arr.length === ++c) {
callback(closest.file);
}
}
});
});
}

/**
* Normalize fs events and pass it on to be processed.
*/
#normalizeChange(dir: string, event: string, file: string) {
if (!file) {
this.#detectChangedFile(dir, event, actualFile => {
if (actualFile) {
this.#processChange(dir, event, actualFile).catch(error =>
this.emitError(error),
);
}
});
this.#processUnnamedChange(dir).catch(error => this.emitError(error));
} else {
this.#processChange(dir, event, path.normalize(file)).catch(error =>
this.emitError(error),
);
}
}

/**
* Process an event that doesn't name the changed entry. Windows sends these
* when changes to a directory overflow the buffer they're reported through,
* so any number of entries under `dir` may have been added, changed or
* removed.
*/
async #processUnnamedChange(dir: string) {
// Watch and register anything new, so that later changes are reported.
await recReaddir(
dir,
subdir => {
this.#watchdir(subdir);
},
filename => {
this.#register(filename, 'f');
},
symlink => {
this.#register(symlink, 'l');
},
this.#checkedEmitError,
this.ignored,
);
// Then have the file map reconcile everything under `dir`.
this.#emitEvent({
event: RECRAWL_EVENT,
relativePath: path.relative(this.root, dir),
});
}

/**
* Process changes.
*/
Expand Down
67 changes: 65 additions & 2 deletions packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* @oncall react_native
*/

import type {WatcherBackendChangeEvent} from '../../flow-types';

import FallbackWatcher from '../FallbackWatcher';
import {createTempWatchRoot} from './helpers';
import EventEmitter from 'node:events';
Expand All @@ -32,6 +34,10 @@ describe('FallbackWatcher', () => {
let calls: Array<string>;
let watchFailure: ?{code: string, path: string};
let watchOverride: ?{path: string, watcher: ErroredFSWatcher};
// The listener passed to `fs.watch` for each directory, and the directories
// whose events are withheld from it.
let listeners: Map<string, (event: string, filename: ?string) => void>;
let mutedDirs: Set<string>;

const indexOfCall = (op: 'watch' | 'readdir', dir: string) =>
calls.indexOf(`${op}:${dir}`);
Expand All @@ -46,9 +52,11 @@ describe('FallbackWatcher', () => {
calls = [];
watchFailure = null;
watchOverride = null;
listeners = new Map();
mutedDirs = new Set();

const {watch} = fs;
jest.spyOn(fs, 'watch').mockImplementation((dir, ...args) => {
jest.spyOn(fs, 'watch').mockImplementation((dir, options, listener) => {
calls.push(`watch:${String(dir)}`);
const override = watchOverride;
if (override != null && dir === override.path) {
Expand All @@ -63,7 +71,12 @@ describe('FallbackWatcher', () => {
error.code = failure.code;
throw error;
}
return watch(dir, ...args);
listeners.set(String(dir), listener);
return watch(dir, options, (event, filename) => {
if (!mutedDirs.has(String(dir))) {
listener(event, filename);
}
});
});
const {readdir} = fs.promises;
// $FlowFixMe[incompatible-call] - variadic passthrough
Expand Down Expand Up @@ -175,6 +188,56 @@ describe('FallbackWatcher', () => {
expectWatchedBeforeListed(join(watchRoot, 'a'));
});
});

// Windows reports a change with no filename when changes to a directory
// overflow its buffer, so any number of entries under it may have changed.
describe('when an event does not name the changed entry', () => {
const emitUnnamedChange = (dir: string) => {
const listener = listeners.get(dir);
if (listener == null) {
throw new Error(`Not watching ${dir}`);
}
listener('change', null);
};

beforeEach(async () => {
await mkdir(join(watchRoot, 'a'));
await writeFile(join(watchRoot, 'a', 'existing.js'), '');
await watcher?.startWatching();
// Only the unnamed change reports anything, and on macOS the root's
// watcher also sees changes in subdirectories.
mutedDirs.add(watchRoot);
mutedDirs.add(join(watchRoot, 'a'));
calls = [];
});

test('requests a recrawl of the directory', async () => {
const events: Array<WatcherBackendChangeEvent> = [];
watcher?.onFileEvent(event => {
events.push(event);
});
await writeFile(join(watchRoot, 'a', 'new.js'), '');
await rm(join(watchRoot, 'a', 'existing.js'));

emitUnnamedChange(join(watchRoot, 'a'));

await waitFor(() => events.some(event => event.event === 'recrawl'));
expect(events).toEqual([
{event: 'recrawl', relativePath: 'a', root: watchRoot},
]);
});

test('watches a new directory under it', async () => {
await mkdir(join(watchRoot, 'a', 'b'));

emitUnnamedChange(join(watchRoot, 'a'));

await waitFor(
() => indexOfCall('readdir', join(watchRoot, 'a', 'b')) >= 0,
);
expectWatchedBeforeListed(join(watchRoot, 'a', 'b'));
});
});
});

function fsError(code: string, path: string): Error {
Expand Down
Loading