Skip to content

Commit

Permalink
Fix: respect patterns with "||" in the range during optimizeResolutio…
Browse files Browse the repository at this point in the history
…ns (yarnpkg#4562)

**Summary**

Fixes yarnpkg#4547 by testing each version against all ranges individually, rather than munging the patterns together to get a single range.

**Test plan**

Existing tests, plus a regression test to repro yarnpkg#4547: "manifest optimization respects versions with alternation"
  • Loading branch information
mxmul authored and joaolucasl committed Oct 27, 2017
1 parent ba37a01 commit 19a8aac
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 4 deletions.
6 changes: 6 additions & 0 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,12 @@ test('unbound transitive dependencies should not conflict with top level depende
});
});

test('manifest optimization respects versions with alternation', async () => {
await runInstall({flat: true}, 'optimize-version-with-alternation', async config => {
expect(await getPackageVersion(config, 'lodash')).toEqual('2.4.2');
});
});

test.concurrent('top level patterns should match after install', (): Promise<void> => {
return runInstall({}, 'top-level-pattern-check', async (config, reporter) => {
let integrityError = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"version": "0.0.1",
"dependencies": {
"lodash": "^3.0.1 || ^2.0.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"dependencies": {
"b": "file:b",
"lodash": "2.4.2"
}
}
Binary file not shown.
16 changes: 12 additions & 4 deletions src/package-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,19 @@ export default class PackageResolver {
return;
}

// reverse sort, so we'll find the maximum satisfying version first
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);
availableVersions.sort(semver.rcompare);

const ranges = collapsablePatterns.map(pattern => normalizePattern(pattern).range);

// find the most recent version that satisfies all patterns (if one exists), and
// collapse to that version.
for (const version of availableVersions) {
if (ranges.every(range => semver.satisfies(version, range))) {
this.collapsePackageVersions(name, version, collapsablePatterns);
return;
}
}
}

Expand Down

0 comments on commit 19a8aac

Please sign in to comment.