Compare versions correctly (#30)

[lamansky]

eslint-plugin-ecmascript-compat currently compares version number strings with the >= operator, which only yields expected results if each component (major/minor/patch) of both version numbers is the same number of digits, meaning the plugin doesn't work in many cases. For example, the plugin currently considers 9.0.0 to be a higher version number than 10.0.0. This patch adds proper version comparison.

[robatwilliams]

Thanks for this.

Neither browserslist nor MDN compat data specify the format of their versions fully, so I'm not sure how exactly we should be treating them for comparison. Semver as you've used here will cover a lot of cases I expect. However it won't cover everything, and if the format is actually just dot-separated strings we don't need a library to handle that. Most in the data are just a single integer within a string.

The compat data has ranged versions, which are not semver - https://github.com/mdn/browser-compat-data/blob/main/schemas/compat-data-schema.md#ranged-versions

The schema simply says it's a string - https://github.com/mdn/browser-compat-data/blob/main/schemas/compat-data.schema.json#L46

[lamansky]

Neither browserslist nor MDN compat data specify the format of their versions fully

The version numbers that they give are whatever version numbers are specified by the browser/engine manufacturers. Because their version data is dependent on other organizations, I can't imagine that they'd specify a universal version schema for their data (unless it was purely descriptive and not prescriptive).

This means it would be on us to come up with a version comparison method that works with all the data. A simple >= operator definitely isn't it.

Most in the data are just a single integer within a string.

The bug still exists even with single integers: in JavaScript, '9' >= '10' returns true. Currently, the plugin only works if each version component integer is the same number of digits, even if it's only a single component (one integer).

The compat data has ranged versions, which are not semver

I presume we can just strip the ≤ character if it exists.

[robatwilliams]

What I said about "single integer within a string" was just an observation. I understand the problem.

It does seem to be a dotted string up to 3 deep, going by https://github.com/mdn/browser-compat-data/tree/main/browsers.

Why do we need a library is over ~5 lines of code? Split the strings, iterate through until last digit of either, parse the bits, compare as numbers.

If we do want to use a library, this current one looks good; the more established semver one is semver-specific and too strict.

[lamansky]

I picked compare-versions because it's not semver-specific and therefore has more leniency than the semver module, which is relevant because, as you said, we can't assume that every browser is going to have semver-compatible versions. But of course, I have no problem with you using whatever library or function you want. Feel free to commit your preferred solution and then close my pull request.

[NikolayFrantsev]

There is another edge case with compare-versions usage.

Browserslist’s rule last 1 iOS major version returns following mapping:

ios_saf 15.4
ios_saf 15.2-15.3
ios_saf 15.0-15.1

So even with this fix, ESLint returns parsing error:

Error: Failed to load plugin 'ecmascript-compat' declared in '.eslintrc.cjs': Invalid argument not valid semver ('15.0-15.1' received)

[robatwilliams]

Closing in favour of #67