Avoid crash in simplifyRanges by removing subsets up front

[smikula]

What's the problem this PR addresses?

Resolves #6373. The problem is that simplifyRanges doesn't correctly reduce redundant OR ranges. For example, ~1.0.1 || ~1.0.2 should be simplified to ~1.0.1. As the algorithm runs, it will effectively calculate every combination of terms in such ranges. For example, given two ranges like ~1.0.1 || ~1.0.2, the nextAlternatives array will end up with 2*2 = 4 entries; if you have 100 such ranges you'll end up with 2^100 entries. Growing exponentially like this it's not hard to crash the process.

Arguably packages should not specify peer deps with this sort of redundant range, but sometimes they do (I'm working on cleaning up my project now that I know what the problem is!) Regardless, yarn shouldn't crash when it happens.

How did you fix it?

At the beginning of simplifyRanges, I reduce any range of this sort by splitting it apart and using sember.subset to check if one part of the range is a subset of another, in which case it can be excluded from the simplified range. I short circuit if the range only has one term, to avoid any excess parsing.

I think this is the right fix, but I'm happy to take feedback or hand it off if someone knows better. (Maybe @arcanis as author of this code?)

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[arcanis]

Can we rather do a filter on the set after we already parsed it, rather than the hardcoded || split followed by semver.subset calls (to avoid parsing the ranges twice)?

[smikula]

I considered this, but semver.subset only accepts a string or a Range object. If I have the parsed range, then I need to compare the members of Range.set that look like:

[
  [
    Comparator {
      options: {},
      loose: false,
      operator: '>=',
      semver: [SemVer],
      value: '>=1.0.1'
    },
    Comparator {
      options: {},
      loose: false,
      operator: '<',
      semver: [SemVer],
      value: '<1.1.0-0'
    }
  ],
  [
    Comparator {
      options: {},
      loose: false,
      operator: '>=',
      semver: [SemVer],
      value: '>=1.0.2'
    },
    Comparator {
      options: {},
      loose: false,
      operator: '<',
      semver: [SemVer],
      value: '<1.1.0-0'
    }
  ]
]

So then I need to either:

• Implement my own version of subset (yuck), or...
• Reconstruct a proper range out of the above without any more parsing, which really gets into the guts of semver

Either case seems like it would add a lot of complexity for marginal benefit. Given that I already short-circuit on ranges that don't have ||, I'm expecting the extra cost here is pretty minimal.

What do you think?

[smikula]

@arcanis Any further thoughts here?

[arcanis]

Looks good to me, thanks!