Skip to content

Commit 391eade

Browse files
mxmulBYK
authored andcommitted
Fix: respect patterns with "||" in the range during optimizeResolutions (#4562)
**Summary** Fixes #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 #4547: "manifest optimization respects versions with alternation"
1 parent 8cf5f5d commit 391eade

File tree

5 files changed

+30
-4
lines changed

5 files changed

+30
-4
lines changed

__tests__/commands/install/integration.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,12 @@ test('unbound transitive dependencies should not conflict with top level depende
10571057
});
10581058
});
10591059

1060+
test('manifest optimization respects versions with alternation', async () => {
1061+
await runInstall({flat: true}, 'optimize-version-with-alternation', async config => {
1062+
expect(await getPackageVersion(config, 'lodash')).toEqual('2.4.2');
1063+
});
1064+
});
1065+
10601066
test.concurrent('top level patterns should match after install', (): Promise<void> => {
10611067
return runInstall({}, 'top-level-pattern-check', async (config, reporter) => {
10621068
let integrityError = false;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"version": "0.0.1",
3+
"dependencies": {
4+
"lodash": "^3.0.1 || ^2.0.0"
5+
}
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"dependencies": {
3+
"b": "file:b",
4+
"lodash": "2.4.2"
5+
}
6+
}

src/package-resolver.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -568,11 +568,19 @@ export default class PackageResolver {
568568
return;
569569
}
570570

571+
// reverse sort, so we'll find the maximum satisfying version first
571572
const availableVersions = this.getAllInfoForPatterns(collapsablePatterns).map(manifest => manifest.version);
572-
const combinedRange = collapsablePatterns.map(pattern => normalizePattern(pattern).range).join(' ');
573-
const singleVersion = semver.maxSatisfying(availableVersions, combinedRange);
574-
if (singleVersion) {
575-
this.collapsePackageVersions(name, singleVersion, collapsablePatterns);
573+
availableVersions.sort(semver.rcompare);
574+
575+
const ranges = collapsablePatterns.map(pattern => normalizePattern(pattern).range);
576+
577+
// find the most recent version that satisfies all patterns (if one exists), and
578+
// collapse to that version.
579+
for (const version of availableVersions) {
580+
if (ranges.every(range => semver.satisfies(version, range))) {
581+
this.collapsePackageVersions(name, version, collapsablePatterns);
582+
return;
583+
}
576584
}
577585
}
578586

0 commit comments

Comments
 (0)