From 361f3f30d3ae2b48429a1738d309a2693c4a1038 Mon Sep 17 00:00:00 2001 From: Christoph Pojer Date: Mon, 1 Feb 2016 19:08:09 -0800 Subject: [PATCH] Fix shallow dependency resolution Summary: The feature added in D2862850 only adds the module that requests shallow dependency resolution to the dependencies of a module. The reason why this is happens is because `collect` calls `response.addDependency` but if `recursive` is set to `false`, dependencies don't call `collect` and therefore don't get added to the resolution response. This fixes it by adding dependencies outside of the `collect` call. public Reviewed By: davidaurelio Differential Revision: D2885152 fb-gh-sync-id: 8ab3b6c6b7cd45d59615a99ac87984a41b5d7025 --- .../DependencyGraph/ResolutionRequest.js | 3 +- .../__tests__/DependencyGraph-test.js | 76 ++++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js index 265156179..e95237e64 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/ResolutionRequest.js @@ -110,8 +110,8 @@ class ResolutionRequest { const visited = Object.create(null); visited[entry.hash()] = true; + response.pushDependency(entry); const collect = (mod) => { - response.pushDependency(mod); return mod.getDependencies().then( depNames => Promise.all( depNames.map(name => this.resolveDependency(mod, name)) @@ -163,6 +163,7 @@ class ResolutionRequest { p = p.then(() => { if (!visited[modDep.hash()]) { visited[modDep.hash()] = true; + response.pushDependency(modDep); if (recursive) { return collect(modDep); } diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index 464985f69..66d02729a 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -21,8 +21,8 @@ const mocksPattern = /(?:[\\/]|^)__mocks__[\\/]([^\/]+)\.js$/; describe('DependencyGraph', function() { let defaults; - function getOrderedDependenciesAsJSON(dgraph, entry, platform) { - return dgraph.getDependencies(entry, platform) + function getOrderedDependenciesAsJSON(dgraph, entry, platform, recursive = true) { + return dgraph.getDependencies(entry, platform, recursive) .then(response => response.finalize()) .then(({ dependencies }) => Promise.all(dependencies.map(dep => Promise.all([ dep.getName(), @@ -89,6 +89,12 @@ describe('DependencyGraph', function() { '/**', ' * @providesModule a', ' */', + 'require("b")', + ].join('\n'), + 'b.js': [ + '/**', + ' * @providesModule b', + ' */', ].join('\n'), }, }); @@ -114,6 +120,17 @@ describe('DependencyGraph', function() { { id: 'a', path: '/root/a.js', + dependencies: ['b'], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + resolveDependency: undefined, + }, + { + id: 'b', + path: '/root/b.js', dependencies: [], isAsset: false, isAsset_DEPRECATED: false, @@ -126,6 +143,61 @@ describe('DependencyGraph', function() { }); }); + pit('should get shallow dependencies', function() { + var root = '/root'; + fs.__setMockFilesystem({ + 'root': { + 'index.js': [ + '/**', + ' * @providesModule index', + ' */', + 'require("a")', + ].join('\n'), + 'a.js': [ + '/**', + ' * @providesModule a', + ' */', + 'require("b")', + ].join('\n'), + 'b.js': [ + '/**', + ' * @providesModule b', + ' */', + ].join('\n'), + }, + }); + + var dgraph = new DependencyGraph({ + ...defaults, + roots: [root], + }); + return getOrderedDependenciesAsJSON(dgraph, '/root/index.js', null, false).then(function(deps) { + expect(deps) + .toEqual([ + { + id: 'index', + path: '/root/index.js', + dependencies: ['a'], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + { + id: 'a', + path: '/root/a.js', + dependencies: ['b'], + isAsset: false, + isAsset_DEPRECATED: false, + isJSON: false, + isPolyfill: false, + resolution: undefined, + }, + ]); + }); + }); + pit('should get dependencies with the correct extensions', function() { var root = '/root'; fs.__setMockFilesystem({