From 1349edad70c8c5c0b05562044ecdb912c4b15078 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 5 Oct 2017 04:45:46 -0700 Subject: [PATCH] Fix edge case of moving a haste file to a different folder Reviewed By: davidaurelio Differential Revision: D5945574 fbshipit-source-id: 5fdd12c0af1b7a92012f4be71ce18f44e42ad4e1 --- .../__tests__/traverseDependencies-test.js | 18 +++++ .../src/DeltaBundler/traverseDependencies.js | 72 +++++++++---------- 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/packages/metro-bundler/src/DeltaBundler/__tests__/traverseDependencies-test.js b/packages/metro-bundler/src/DeltaBundler/__tests__/traverseDependencies-test.js index 4b9fbbcc..f2b003a0 100644 --- a/packages/metro-bundler/src/DeltaBundler/__tests__/traverseDependencies-test.js +++ b/packages/metro-bundler/src/DeltaBundler/__tests__/traverseDependencies-test.js @@ -189,4 +189,22 @@ describe('edge cases', () => { deleted: new Set(['/baz']), }); }); + + it('move a file to a different folder', async () => { + const edges = new Map(); + await initialTraverseDependencies('/bundle', dependencyGraph, {}, edges); + + const moduleBazMoved = createModule({path: '/baz-moved', name: 'baz'}); + mockedDependencyTree.set(moduleFoo.path, [moduleBar, moduleBazMoved]); + mockedDependencies.add(moduleBazMoved); + mockedDependencies.delete(moduleBaz); + + // Modify /baz, rename it to /qux and modify it again. + expect( + await traverseDependencies(['/foo'], dependencyGraph, {}, edges), + ).toEqual({ + added: new Set(['/baz-moved']), + deleted: new Set(['/baz']), + }); + }); }); diff --git a/packages/metro-bundler/src/DeltaBundler/traverseDependencies.js b/packages/metro-bundler/src/DeltaBundler/traverseDependencies.js index 68159984..7fcfed86 100644 --- a/packages/metro-bundler/src/DeltaBundler/traverseDependencies.js +++ b/packages/metro-bundler/src/DeltaBundler/traverseDependencies.js @@ -108,27 +108,45 @@ async function traverseDependenciesForSingleFile( return {added: new Set(), deleted: new Set()}; } - const currentDependencies = new Set( + // Get the absolute path of all sub-dependencies (some of them could have been + // moved but maintain the same relative path). + const currentDependencies = resolveDependencies( + path, await dependencyGraph.getShallowDependencies(path, transformOptions), + dependencyGraph, + transformOptions, ); - const previousDependencies = new Set(edge.dependencies.keys()); + + const previousDependencies = new Set(edge.dependencies.values()); const nonNullEdge = edge; let numProcessed = 0; let total = currentDependencies.size; + const deleted = Array.from(edge.dependencies.entries()) + .map(([relativePath, absolutePath]) => { + if (!currentDependencies.has(absolutePath)) { + return removeDependency(nonNullEdge, relativePath, edges); + } else { + return undefined; + } + }) + .filter(Boolean); + // Check all the module dependencies and start traversing the tree from each // added and removed dependency, to get all the modules that have to be added // and removed from the dependency graph. const added = await Promise.all( - Array.from(currentDependencies).map(async dependency => { + Array.from( + currentDependencies, + ).map(async ([absolutePath, relativePath]) => { let newDependencies; - if (!previousDependencies.has(dependency)) { + if (!previousDependencies.has(absolutePath)) { newDependencies = await addDependency( nonNullEdge, - dependency, + relativePath, dependencyGraph, transformOptions, edges, @@ -149,26 +167,6 @@ async function traverseDependenciesForSingleFile( }), ); - // Check if all currentDependencies are still in the bundle (some files can - // have been removed). - checkModuleDependencies( - path, - currentDependencies, - dependencyGraph, - transformOptions, - edges, - ); - - const deleted = Array.from(previousDependencies) - .map(dependency => { - if (!currentDependencies.has(dependency)) { - return removeDependency(nonNullEdge, dependency, edges); - } else { - return undefined; - } - }) - .filter(Boolean); - return { added: flatten(added), deleted: flatten(deleted), @@ -311,22 +309,24 @@ function resolveEdge( return edges.get(absolutePath); } -function checkModuleDependencies( +function resolveDependencies( parentPath, - dependencies: Set, + dependencies: Array, dependencyGraph: DependencyGraph, transformOptions: JSTransformerOptions, - edges: DependencyEdges, -) { +): Map { const parentModule = dependencyGraph.getModuleForPath(parentPath); - for (const dependency of dependencies.values()) { - dependencyGraph.resolveDependency( - parentModule, - dependency, - transformOptions.platform, - ); - } + return new Map( + dependencies.map(relativePath => [ + dependencyGraph.resolveDependency( + parentModule, + relativePath, + transformOptions.platform, + ).path, + relativePath, + ]), + ); } function flatten(input: Iterable>): Set {