Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes how peer dependencies are resolved multiple levels deep #6443

Merged
merged 1 commit into from Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,10 @@
/* @flow */

module.exports = require(`./package.json`);

for (const key of [`dependencies`, `devDependencies`, `peerDependencies`]) {
for (const dep of Object.keys(module.exports[key] || {})) {
// $FlowFixMe The whole point of this file is to be dynamic
module.exports[key][dep] = require(dep);
}
}
@@ -0,0 +1,8 @@
{
"name": "peer-deps-lvl0",
"version": "1.0.0",
"dependencies": {
"no-deps": "1.0.0",
"peer-deps-lvl1": "1.0.0"
}
}
@@ -0,0 +1,10 @@
/* @flow */

module.exports = require(`./package.json`);

for (const key of [`dependencies`, `devDependencies`, `peerDependencies`]) {
for (const dep of Object.keys(module.exports[key] || {})) {
// $FlowFixMe The whole point of this file is to be dynamic
module.exports[key][dep] = require(dep);
}
}
@@ -0,0 +1,10 @@
{
"name": "peer-deps-lvl1",
"version": "1.0.0",
"dependencies": {
"peer-deps-lvl2": "1.0.0"
},
"peerDependencies": {
"no-deps": "*"
}
}
@@ -0,0 +1,10 @@
/* @flow */

module.exports = require(`./package.json`);

for (const key of [`dependencies`, `devDependencies`, `peerDependencies`]) {
for (const dep of Object.keys(module.exports[key] || {})) {
// $FlowFixMe The whole point of this file is to be dynamic
module.exports[key][dep] = require(dep);
}
}
@@ -0,0 +1,7 @@
{
"name": "peer-deps-lvl2",
"version": "1.0.0",
"peerDependencies": {
"no-deps": "*"
}
}
45 changes: 45 additions & 0 deletions packages/pkg-tests/pkg-tests-specs/sources/basic.js
Expand Up @@ -284,6 +284,51 @@ module.exports = (makeTemporaryEnv: PackageDriver) => {
),
);

test(
`it should install in such a way that peer dependencies can be resolved (two levels deep)`,
makeTemporaryEnv(
{
dependencies: {[`peer-deps-lvl0`]: `1.0.0`},
},
async ({path, run, source}) => {
await run(`install`);

await expect(source(`require('peer-deps-lvl0')`)).resolves.toMatchObject({
name: `peer-deps-lvl0`,
version: `1.0.0`,
dependencies: {
[`peer-deps-lvl1`]: {
name: `peer-deps-lvl1`,
version: `1.0.0`,
dependencies: {
[`peer-deps-lvl2`]: {
name: `peer-deps-lvl2`,
version: `1.0.0`,
peerDependencies: {
[`no-deps`]: {
name: `no-deps`,
version: `1.0.0`,
},
},
},
},
peerDependencies: {
[`no-deps`]: {
name: `no-deps`,
version: `1.0.0`,
},
},
},
[`no-deps`]: {
name: `no-deps`,
version: `1.0.0`,
},
},
});
},
),
);

test(
`it should cache the loaded modules`,
makeTemporaryEnv(
Expand Down
31 changes: 24 additions & 7 deletions src/util/generate-pnp-map.js
Expand Up @@ -202,8 +202,12 @@ async function getPackageInformationStores(
return {pkg, ref, loc};
};

const visit = async (seedPatterns: Array<string>, parentData: Array<string> = []) => {
const resolutions = new Map();
const visit = async (
precomputedResolutions: Map<string, string>,
seedPatterns: Array<string>,
parentData: Array<string> = [],
) => {
const resolutions = new Map(precomputedResolutions);
const locations = new Map();

// This first pass will compute the package reference of each of the given patterns
Expand Down Expand Up @@ -337,11 +341,14 @@ async function getPackageInformationStores(
return !pkg || !peerDependencies.has(pkg.name);
});

// We do this in two steps to prevent cyclic dependencies from looping indefinitely
// We inject the partial information in the store right now so that we won't cycle indefinitely
packageInformationStore.set(packageReference, packageInformation);
packageInformation.packageDependencies = await visit(directDependencies, [packageName, packageReference]);

// We now have to inject the peer dependencies
// We must inject the peer dependencies before iterating; one of our dependencies might have a peer dependency
// on one of our peer dependencies, so it must be available from the start (we don't have to do that for direct
// dependencies, because the "visit" function that will iterate over them will automatically add the to the
// candidate resolutions as part of the first step, cf above)

for (const dependencyName of peerDependencies) {
const dependencyReference = resolutions.get(dependencyName);

Expand All @@ -350,6 +357,16 @@ async function getPackageInformationStores(
}
}

const childResolutions = await visit(packageInformation.packageDependencies, directDependencies, [
packageName,
packageReference,
]);

// We can now inject into our package the resolutions we got from the visit function
for (const [name, reference] of childResolutions.entries()) {
packageInformation.packageDependencies.set(name, reference);
}

// Finally, unless a package depends on a previous version of itself (that would be weird but correct...), we
// inject them an implicit dependency to themselves (so that they can require themselves)
if (!packageInformation.packageDependencies.has(packageName)) {
Expand Down Expand Up @@ -389,7 +406,7 @@ async function getPackageInformationStores(

packageInformationStore.set(pkg.version, {
packageLocation: normalizeDirectoryPath(loc),
packageDependencies: await visit(ref.dependencies, [name, pkg.version]),
packageDependencies: await visit(new Map(), ref.dependencies, [name, pkg.version]),
});
}
}
Expand All @@ -403,7 +420,7 @@ async function getPackageInformationStores(
null,
{
packageLocation: normalizeDirectoryPath(config.lockfileFolder),
packageDependencies: await visit(seedPatterns),
packageDependencies: await visit(new Map(), seedPatterns),
},
],
]),
Expand Down