Skip to content

Commit

Permalink
Fix handling of deletion of a symlink pointing to a Haste module
Browse files Browse the repository at this point in the history
Summary:
A bug I noticed while testing `unstable_enableSymlinks` on some Haste apps - because `FileSystem.getModuleName()` follows symlinks, we'd incorrectly clear a Haste module from the module map whenever a symlink pointing to the providing file was deleted.

This removes the use of `FileSystem.getModuleName()` and changes `MutableFileSystem.remove()` so that it returns the removed `FileMetadata`, if any. From that, we take the Haste ID (which is empty for symlinks) and remove the Haste module if that is populated.

Changelog:
[Experimental] `unstable_enableSymlinks`: Fix handling of deletion of a symlink pointing to a Haste module

Reviewed By: motiz88

Differential Revision: D43499958

fbshipit-source-id: ddff3d2236ccf7f60442156071289230ce9cec54
  • Loading branch information
robhogan authored and facebook-github-bot committed Feb 23, 2023
1 parent cb52585 commit 0e2a70a
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 6 deletions.
10 changes: 8 additions & 2 deletions packages/metro-file-map/src/HasteFS.js
Expand Up @@ -38,8 +38,14 @@ export default class HasteFS implements MutableFileSystem {
: path.normalize(relativeOrAbsolutePath);
}

remove(filePath: Path) {
this.#files.delete(this._normalizePath(filePath));
remove(filePath: Path): ?FileMetaData {
const normalPath = this._normalizePath(filePath);
const fileMetadata = this.#files.get(normalPath);
if (!fileMetadata) {
return null;
}
this.#files.delete(normalPath);
return fileMetadata;
}

bulkAddOrModify(changedFiles: FileData) {
Expand Down
53 changes: 53 additions & 0 deletions packages/metro-file-map/src/__tests__/index-test.js
Expand Up @@ -1425,6 +1425,9 @@ describe('HasteMap', () => {
});
}

hm_it.only = (title, fn, options) =>
hm_it(title, fn, {...options, only: true});

hm_it('build returns a "live" fileSystem and hasteModuleMap', async hm => {
const {fileSystem, hasteModuleMap} = await hm.build();
const filePath = path.join('/', 'project', 'fruits', 'Banana.js');
Expand Down Expand Up @@ -1467,6 +1470,12 @@ describe('HasteMap', () => {
size: 5,
};

const MOCK_DELETE_LINK = {
type: 'l',
modifiedTime: null,
size: null,
};

const MOCK_CHANGE_FOLDER = {
type: 'd',
modifiedTime: 45,
Expand Down Expand Up @@ -1669,6 +1678,50 @@ describe('HasteMap', () => {
{config: {enableSymlinks: true}},
);

hm_it(
'symlink deletion is handled without affecting the symlink target',
async hm => {
const {fileSystem, hasteModuleMap} = await hm.build();

const symlinkPath = path.join(
'/',
'project',
'fruits',
'LinkToStrawberry.js',
);
const realPath = path.join('/', 'project', 'fruits', 'Strawberry.js');

expect(fileSystem.getModuleName(symlinkPath)).toEqual('Strawberry');
expect(fileSystem.getModuleName(realPath)).toEqual('Strawberry');
expect(hasteModuleMap.getModule('Strawberry', 'g')).toEqual(realPath);

// Delete the symlink
const e = mockEmitters[path.join('/', 'project', 'fruits')];
delete mockFs[symlinkPath];
e.emit(
'all',
'delete',
'LinkToStrawberry.js',
path.join('/', 'project', 'fruits', ''),
null,
);
const {eventsQueue} = await waitForItToChange(hm);

expect(eventsQueue).toHaveLength(1);
expect(eventsQueue).toEqual([
{filePath: symlinkPath, metadata: MOCK_DELETE_LINK, type: 'delete'},
]);

// Symlink is deleted without affecting the Haste module or real file.
expect(fileSystem.exists(symlinkPath)).toBe(false);
expect(fileSystem.exists(realPath)).toBe(true);
expect(fileSystem.getModuleName(symlinkPath)).toEqual(null);
expect(fileSystem.getModuleName(realPath)).toEqual('Strawberry');
expect(hasteModuleMap.getModule('Strawberry', 'g')).toEqual(realPath);
},
{config: {enableSymlinks: true}},
);

hm_it(
'correctly tracks changes to both platform-specific versions of a single module name',
async hm => {
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-file-map/src/flow-types.js
Expand Up @@ -222,7 +222,7 @@ export type ModuleMapItem = {
export type ModuleMetaData = [/* path */ string, /* type */ number];

export interface MutableFileSystem extends FileSystem {
remove(filePath: Path): void;
remove(filePath: Path): ?FileMetaData;
addOrModify(filePath: Path, fileMetadata: FileMetaData): void;
bulkAddOrModify(addedOrModifiedFiles: FileData): void;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/metro-file-map/src/index.js
Expand Up @@ -837,8 +837,11 @@ export default class HasteMap extends EventEmitter {
moduleMap: RawModuleMap,
relativeFilePath: Path,
) {
const moduleName = fileSystem.getModuleName(relativeFilePath);
fileSystem.remove(relativeFilePath);
const fileMetadata = fileSystem.remove(relativeFilePath);
if (fileMetadata == null) {
return;
}
const moduleName = fileMetadata[H.ID] || null; // Empty string indicates no module
if (moduleName == null) {
return;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/metro-file-map/src/lib/TreeFS.js
Expand Up @@ -219,13 +219,18 @@ export default class TreeFS implements MutableFileSystem {
}
}

remove(filePath: Path) {
remove(filePath: Path): ?FileMetaData {
const normalPath = this._normalizePath(filePath);
const fileMetadata = this.#files.get(normalPath);
if (fileMetadata == null) {
return null;
}
this.#files.delete(normalPath);
const directoryParts = normalPath.split(path.sep);
const basename = directoryParts.pop();
const directoryNode = this._mkdirp(directoryParts);
directoryNode.delete(basename);
return fileMetadata;
}

_lookupByNormalPath(
Expand Down

0 comments on commit 0e2a70a

Please sign in to comment.