Skip to content

Commit

Permalink
Fix: unbound transitive dependencies should not conflict with top lev…
Browse files Browse the repository at this point in the history
…el dependency (yarnpkg#4488)

**Summary**

Fixes yarnpkg#3780, and makes the failing test from yarnpkg#3779 passing.

As a final step of package resolution, for each dependency we check whether any version satisfies all resolved version ranges. 

**Test plan**

Fixes an existing (failing) test: "unbound transitive dependencies should not conflict with top level dependency"
  • Loading branch information
mxmul authored and joaolucasl committed Oct 27, 2017
1 parent 2f01936 commit d6e0b7d
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
3 changes: 1 addition & 2 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1049,8 +1049,7 @@ test.concurrent('transitive file: dependencies should work', (): Promise<void> =
});
});

// Unskip once https://github.com/yarnpkg/yarn/issues/3778 is resolved
test.skip('unbound transitive dependencies should not conflict with top level dependency', async () => {
test('unbound transitive dependencies should not conflict with top level dependency', async () => {
await runInstall({flat: true}, 'install-conflicts', async config => {
expect((await fs.readJson(path.join(config.cwd, 'node_modules', 'left-pad', 'package.json'))).version).toEqual(
'1.0.0',
Expand Down
42 changes: 42 additions & 0 deletions src/package-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,14 @@ export default class PackageResolver {

getAllInfoForPackageName(name: string): Array<Manifest> {
const patterns = this.patternsByPackage[name] || [];
return this.getAllInfoForPatterns(patterns);
}

/**
* Retrieve all the package info stored for a list of patterns.
*/

getAllInfoForPatterns(patterns: string[]): Array<Manifest> {
const infos = [];
const seen = new Set();

Expand Down Expand Up @@ -284,6 +292,13 @@ export default class PackageResolver {

collapseAllVersionsOfPackage(name: string, version: string): string {
const patterns = this.dedupePatterns(this.patternsByPackage[name]);
return this.collapsePackageVersions(name, version, patterns);
}

/**
* Make all given patterns resolve to version.
*/
collapsePackageVersions(name: string, version: string, patterns: string[]): string {
const human = `${name}@${version}`;

// get manifest that matches the version we're collapsing too
Expand Down Expand Up @@ -530,10 +545,37 @@ export default class PackageResolver {
this.resolveToResolution(req);
}

for (const dep of deps) {
const name = normalizePattern(dep.pattern).name;
this.optimizeResolutions(name);
}

activity.end();
this.activity = null;
}

// for a given package, see if a single manifest can satisfy all ranges
optimizeResolutions(name: string) {
const patterns: Array<string> = this.dedupePatterns(this.patternsByPackage[name] || []);

// don't optimize things that already have a lockfile entry:
// https://github.com/yarnpkg/yarn/issues/79
const collapsablePatterns = patterns.filter(pattern => {
const remote = this.patterns[pattern]._remote;
return !this.lockfile.getLocked(pattern) && (!remote || remote.type !== 'workspace');
});
if (collapsablePatterns.length < 2) {
return;
}

const availableVersions = this.getAllInfoForPatterns(collapsablePatterns).map(manifest => manifest.version);
const combinedRange = collapsablePatterns.map(pattern => normalizePattern(pattern).range).join(' ');
const singleVersion = semver.maxSatisfying(availableVersions, combinedRange);
if (singleVersion) {
this.collapsePackageVersions(name, singleVersion, collapsablePatterns);
}
}

/**
* Called by the package requester for packages that this resolver already had
* a matching version for. Delay the resolve, because better matches can still be
Expand Down

0 comments on commit d6e0b7d

Please sign in to comment.