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

Fix: Resolve peerDependencies from all higher levels, not just root #4478

Merged
merged 11 commits into from
Sep 16, 2017
70 changes: 43 additions & 27 deletions src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,34 +432,50 @@ export default class PackageLinker {

resolvePeerModules() {
for (const pkg of this.resolver.getManifests()) {
this._resolvePeerModules(pkg);
}
}

_resolvePeerModules(pkg: Manifest) {
const peerDeps = pkg.peerDependencies;
if (!peerDeps) {
return;
}

const ref = pkg._reference;
invariant(ref, 'Package reference is missing');
const peerDeps = pkg.peerDependencies;
if (!peerDeps) {
continue;
}
const ref = pkg._reference;
invariant(ref, 'Package reference is missing');

for (const peerDepName in peerDeps) {
const range = peerDeps[peerDepName];
const peerPkgs = this.resolver.getAllInfoForPackageName(peerDepName);

let incorrectPeer = false;
let resolvedLevelDistance = Infinity;
let resolvedPeerPkgPattern;
for (const peerPkg of peerPkgs) {
const peerPkgRef = peerPkg._reference;
if (!(peerPkgRef && peerPkgRef.patterns)) {
continue;
}
const levelDistance = ref.level - peerPkgRef.level;
if (levelDistance >= 0 && levelDistance < resolvedLevelDistance) {
if (this._satisfiesPeerDependency(range, peerPkgRef.version)) {
resolvedLevelDistance = levelDistance;
resolvedPeerPkgPattern = peerPkgRef.patterns;
this.reporter.verbose(
this.reporter.lang(
'selectedPeer',
`${pkg.name}@${pkg.version}`,
`${peerDepName}@${range}`,
peerPkgRef.level,
),
);
} else {
incorrectPeer = true;
}
}
}

for (const name in peerDeps) {
const range = peerDeps[name];
const pkgs = this.resolver.getAllInfoForPackageName(name);
const found = pkgs.find(pkg => {
const {root, version} = pkg._reference || {};
return root && this._satisfiesPeerDependency(range, version);
});
const foundPattern = found && found._reference && found._reference.patterns;

if (foundPattern) {
ref.addDependencies(foundPattern);
} else {
const depError = pkgs.length > 0 ? 'incorrectPeer' : 'unmetPeer';
const [pkgHuman, depHuman] = [`${pkg.name}@${pkg.version}`, `${name}@${range}`];
this.reporter.warn(this.reporter.lang(depError, pkgHuman, depHuman));
if (resolvedPeerPkgPattern) {
ref.addDependencies(resolvedPeerPkgPattern);
} else {
const depError = incorrectPeer ? 'incorrectPeer' : 'unmetPeer';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can define let depError = 'unmetPeer' instead of let incorrectPeer = false and set it to 'incorrectPeer' above right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hahaha, right! Stupid me. Will do that too. Rest looks fine? I'll add tests soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup the rest looks good!

this.reporter.warn(this.reporter.lang(depError, `${pkg.name}@${pkg.version}`, `${peerDepName}@${range}`));
}
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/package-reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default class PackageReference {
this.permissions = {};
this.patterns = [];
this.optional = null;
this.root = false;
this.level = Infinity;
this.ignore = false;
this.incompatible = false;
this.fresh = false;
Expand All @@ -39,7 +39,7 @@ export default class PackageReference {
lockfile: Lockfile;
config: Config;

root: boolean;
level: number;
name: string;
version: string;
uid: string;
Expand All @@ -66,9 +66,13 @@ export default class PackageReference {
addRequest(request: PackageRequest) {
this.requests.push(request);

if (!request.parentRequest) {
this.root = true;
let requestLevel = -1;
let currentRequest = request;
while (currentRequest && requestLevel < this.level) {
requestLevel += 1;
currentRequest = currentRequest.parentRequest;
}
this.level = requestLevel;
}

prune() {
Expand Down
1 change: 1 addition & 0 deletions src/reporters/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ const messages = {

unmetPeer: '$0 has unmet peer dependency $1.',
incorrectPeer: '$0 has incorrect peer dependency $1.',
selectedPeer: 'Selecting $1 at level $2 as the peer dependency of $0.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw this earlier, nice touch!

missingBundledDependency: '$0 is missing a bundled dependency $1. This should be reported to the package maintainer.',

savedNewDependency: 'Saved 1 new dependency.',
Expand Down