Skip to content

Commit

Permalink
Don't apply file extensions filter to symlink change events
Browse files Browse the repository at this point in the history 
Summary:
Currently, `metro-file-map`'s onChange handler filters out non-directory events whose extensions don't match the configured set of watched file extensions (or really, suffices).

We don't want this - symlinks may point to files, directories, or nothing, and this may change over time without modification to the symlink itself. We need to retain them *unless* the canonical path of the symlink itself is ignored by `ignorePattern` (eg, the symlink is under `.git`)`.

This diff shifts things around so that `onChange` ignores early for
 - a) any directory event,
 - b) known-regular files with an unlisted extension,
 - c) any path matching `ignorePattern`.

`delete` events may not have metadata, so we can't ignore them based on a) or b). Instead, we ignore deletion events for files not present in the file map.

Changelog: [Internal]
*(No observable change unless using experimental config)*

Reviewed By: motiz88

Differential Revision: D43214089

fbshipit-source-id: e2c4e4e47cee1e3dbe7292d83a4c16af84e21b06
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed Feb 13, 2023
1 parent 2ca7548 commit a0ba994
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 18 deletions.
112 changes: 109 additions & 3 deletions packages/metro-file-map/src/__tests__/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ describe('HasteMap', () => {
expect(fileSystem.matchFiles('.git')).toEqual([]);
});

it('warn on ignore pattern except for regex', async () => {
it('throw on ignore pattern except for regex', async () => {
const config = {ignorePattern: 'Kiwi', ...defaultConfig};
mockFs['/project/fruits/Kiwi.js'] = `
// Kiwi!
Expand Down Expand Up @@ -1410,8 +1410,12 @@ describe('HasteMap', () => {
if (options.mockFs) {
mockFs = options.mockFs;
}
const watchConfig = {...defaultConfig, watch: true};
const hm = new HasteMap(watchConfig);
const config = {
...defaultConfig,
watch: true,
...options.config,
};
const hm = new HasteMap(config);
await hm.build();
try {
await fn(hm);
Expand Down Expand Up @@ -1457,6 +1461,12 @@ describe('HasteMap', () => {
size: null,
};

const MOCK_CHANGE_LINK = {
type: 'l',
modifiedTime: 46,
size: 5,
};

const MOCK_CHANGE_FOLDER = {
type: 'd',
modifiedTime: 45,
Expand Down Expand Up @@ -1561,6 +1571,102 @@ describe('HasteMap', () => {
},
);

hm_it(
'does not emit changes for regular files with unwatched extensions',
async hm => {
const {fileSystem} = await hm.build();
mockFs[path.join('/', 'project', 'fruits', 'Banana.unwatched')] = '';

const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit(
'all',
'add',
path.join('Banana.js'),
path.join('/', 'project', 'fruits', ''),
MOCK_CHANGE_FILE,
);
e.emit(
'all',
'add',
path.join('Banana.unwatched'),
path.join('/', 'project', 'fruits', ''),
MOCK_CHANGE_FILE,
);
const {eventsQueue} = await waitForItToChange(hm);
const filePath = path.join('/', 'project', 'fruits', 'Banana.js');
expect(eventsQueue).toHaveLength(1);
expect(eventsQueue).toEqual([
{filePath, metadata: MOCK_CHANGE_FILE, type: 'add'},
]);
expect(fileSystem.getModuleName(filePath)).toBeDefined();
},
);

hm_it('does not emit delete events for unknown files', async hm => {
const {fileSystem} = await hm.build();
mockFs[path.join('/', 'project', 'fruits', 'Banana.unwatched')] = '';

const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit(
'all',
'delete',
path.join('Banana.js'),
path.join('/', 'project', 'fruits', ''),
null,
);
e.emit(
'all',
'delete',
path.join('Unknown.ext'),
path.join('/', 'project', 'fruits', ''),
null,
);
const {eventsQueue} = await waitForItToChange(hm);
const filePath = path.join('/', 'project', 'fruits', 'Banana.js');
expect(eventsQueue).toHaveLength(1);
expect(eventsQueue).toEqual([
{filePath, metadata: MOCK_DELETE_FILE, type: 'delete'},
]);
expect(fileSystem.getModuleName(filePath)).toBeDefined();
expect(console.warn).not.toHaveBeenCalled();
expect(console.error).not.toHaveBeenCalled();
});

hm_it(
'does emit changes for symlinks with unlisted extensions',
async hm => {
const {fileSystem} = await hm.build();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
mockFs[path.join('/', 'project', 'fruits', 'LinkToStrawberry.ext')] = {
link: 'Strawberry.js',
};
e.emit(
'all',
'add',
path.join('LinkToStrawberry.ext'),
path.join('/', 'project', 'fruits', ''),
MOCK_CHANGE_LINK,
);
const {eventsQueue} = await waitForItToChange(hm);
const filePath = path.join(
'/',
'project',
'fruits',
'LinkToStrawberry.ext',
);
expect(eventsQueue).toHaveLength(1);
expect(eventsQueue).toEqual([
{filePath, metadata: MOCK_CHANGE_LINK, type: 'add'},
]);
const linkStats = fileSystem.linkStats(filePath);
expect(linkStats).toEqual({
fileType: 'l',
modifiedTime: 46,
});
},
{config: {enableSymlinks: true}},
);

hm_it(
'correctly tracks changes to both platform-specific versions of a single module name',
async hm => {
Expand Down
37 changes: 22 additions & 15 deletions packages/metro-file-map/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,9 @@ export default class HasteMap extends EventEmitter {
this._options.throwOnModuleCollision = false;
this._options.retainAllFiles = true;

const extensions = this._options.extensions;
const hasWatchedExtension = (filePath: string) =>
this._options.extensions.some(ext => filePath.endsWith(ext));

const rootDir = this._options.rootDir;

let changeQueue: Promise<null | void> = Promise.resolve();
Expand Down Expand Up @@ -926,15 +928,24 @@ export default class HasteMap extends EventEmitter {
root: Path,
metadata: ?ChangeEventMetadata,
) => {
const absoluteFilePath = path.join(root, normalizePathSep(filePath));
if (
(metadata && metadata.type === 'd') ||
this._ignore(absoluteFilePath) ||
!extensions.some(extension => absoluteFilePath.endsWith(extension))
metadata &&
// Ignore all directory events
(metadata.type === 'd' ||
// Ignore regular files with unwatched extensions
(metadata.type === 'f' && !hasWatchedExtension(filePath)))
) {
return;
}

const absoluteFilePath = path.join(root, normalizePathSep(filePath));

// Ignore files (including symlinks) whose path matches ignorePattern
// (we don't ignore node_modules in watch mode)
if (this._options.ignorePattern.test(absoluteFilePath)) {
return;
}

const relativeFilePath = fastPath.relative(rootDir, absoluteFilePath);
const linkStats = fileSystem.linkStats(relativeFilePath);

Expand Down Expand Up @@ -1030,10 +1041,11 @@ export default class HasteMap extends EventEmitter {
// point.
}
} else if (type === 'delete') {
invariant(
linkStats?.fileType != null,
'delete event received for file of unknown type',
);
if (linkStats == null) {
// Don't emit deletion events for files we weren't retaining.
// This is expected for deletion of an ignored file.
return null;
}
enqueueEvent({
modifiedTime: null,
size: null,
Expand Down Expand Up @@ -1157,12 +1169,7 @@ export default class HasteMap extends EventEmitter {
* Helpers
*/
_ignore(filePath: Path): boolean {
const ignorePattern = this._options.ignorePattern;
const ignoreMatched =
ignorePattern instanceof RegExp
? ignorePattern.test(filePath)
: ignorePattern && ignorePattern(filePath);

const ignoreMatched = this._options.ignorePattern.test(filePath);
return (
ignoreMatched ||
(!this._options.retainAllFiles && filePath.includes(NODE_MODULES))
Expand Down

0 comments on commit a0ba994

Please sign in to comment.