Skip to content

Commit

Permalink
Fix shallow dependency resolution
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cpojer authored and facebook-github-bot-7 committed Feb 2, 2016
1 parent db275dd commit 361f3f3
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -89,6 +89,12 @@ describe('DependencyGraph', function() {
'/**',
' * @providesModule a',
' */',
'require("b")',
].join('\n'),
'b.js': [
'/**',
' * @providesModule b',
' */',
].join('\n'),
},
});
Expand All @@ -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,
Expand All @@ -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({
Expand Down

0 comments on commit 361f3f3

Please sign in to comment.