Skip to content

Commit

Permalink
Compare versions properly, not as plain strings (#67)
Browse files Browse the repository at this point in the history
* handle version range coming from browserslist

* ensure no ranged version, to simplify the problem

* sort for oldest version

* simplify earlier, before the sort

* compare as versions for compatibility

* comparison tests

* eol
  • Loading branch information
robatwilliams authored May 5, 2023
1 parent 36c9224 commit 659e9da
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"eqeqeq": ["error", "always", { "null": "never" }],
"func-names": ["error", "as-needed"],
"func-style": ["error", "declaration"],
"id-length": ["error", { "exceptions": ["_"] }],
"id-length": ["error", { "exceptions": ["_", "a", "b", "i"] }],
"no-unused-vars": ["error", { "ignoreRestSiblings": true }],
"no-use-before-define": ["error", "nofunc"],
"spaced-comment": ["error", "always", { "block": { "balanced": true } }],
Expand Down
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
".eslintrc.json": "jsonc"
},

"files.eol": "\n",
"files.insertFinalNewline": true,
"files.trimFinalNewlines": true,
"files.trimTrailingWhitespace": true,
Expand Down
15 changes: 15 additions & 0 deletions packages/eslint-plugin-ecmascript-compat/lib/compareVersions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module.exports = function compareVersions(a, b) {
const aParts = a.split('.').map(Number);
const bParts = b.split('.').map(Number);

for (let i = 0; i < Math.max(aParts.length, bParts.length); i++) {
if (aParts[i] === bParts[i]) {
// eslint-disable-next-line no-continue
continue;
}

return aParts[i] < bParts[i] ? -1 : 1;
}

return 0;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const compareVersions = require('./compareVersions');

test('equal', () => {
expect(compareVersions('1.2.3', '1.2.3')).toBe(0);
});

test('smaller', () => {
expect(compareVersions('1.2.0', '1.2.3')).toBe(-1);
expect(compareVersions('1.2', '1.3')).toBe(-1);
});

test('larger', () => {
expect(compareVersions('1.2.3', '1.2.0')).toBe(1);
});

test('partial on one side', () => {
expect(compareVersions('1.1', '1.2.3')).toBe(-1);
expect(compareVersions('1.1.1', '1.2')).toBe(-1);
});

test('when it matters to treat parts as numbers', () => {
expect(compareVersions('9', '10')).toBe(-1);
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable camelcase, no-underscore-dangle */
const compareVersions = require('./compareVersions');

function unsupportedFeatures(features, targets) {
return features.filter((feature) => !isFeatureSupportedByTargets(feature, targets));
Expand Down Expand Up @@ -30,7 +31,7 @@ function isCompatFeatureSupportedByTarget(compatFeature, target) {
return true;
}

return !support.isNone && target.version >= versionAdded;
return !support.isNone && compareVersions(target.version, versionAdded) >= 0;
}

function getSimpleSupportStatement(compatFeature, target) {
Expand Down
44 changes: 44 additions & 0 deletions packages/eslint-plugin-ecmascript-compat/lib/compatibility.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable camelcase */
const { unsupportedFeatures } = require('./compatibility');
const features = require('./features');

it('supports feature in version introduced', () => {
const feature = {
Expand All @@ -18,6 +19,26 @@ it('supports feature in version introduced', () => {
expect(unsupported).toHaveLength(0);
});

it('supports feature in version later than introduced, treating versions as numbers', () => {
const feature = {
compatFeatures: [
{
__compat: {
support: {
safari: { version_added: '9' },
},
},
},
],
};

const unsupported = unsupportedFeatures(
[feature],
[{ name: 'safari', version: '14.0' }]
);
expect(unsupported).toHaveLength(0);
});

it('doesnt support feature in version before introduced', () => {
const feature = {
compatFeatures: [
Expand Down Expand Up @@ -194,3 +215,26 @@ it('explains what the problem is when compat feature not found in MDN data', ()
unsupportedFeatures([feature], [{ name: 'chrome', version: '73' }]);
}).toThrow("Sparse compatFeatures for rule 'some rule': object,undefined");
});

it('can rely on all the versions in the compatibility data used being semver or partial semver', () => {
// We rely on this to avoid having to deal with ranged versions, for simplicity.
// https://github.com/mdn/browser-compat-data/blob/main/schemas/compat-data-schema.md#ranged-versions

for (const esFeature of features) {
for (const compatFeature of esFeature.compatFeatures) {
// eslint-disable-next-line no-underscore-dangle
for (const supportStatement of Object.values(compatFeature.__compat.support)) {
const simpleSupportStatement = Array.isArray(supportStatement)
? supportStatement[0]
: supportStatement;

if (simpleSupportStatement.version_added !== false) {
// eslint-disable-next-line jest/no-conditional-expect
expect(simpleSupportStatement.version_added).toMatch(
/\d+(?<minor>\.\d+(?<patch>\.\d+)?)?/u
);
}
}
}
}
});
15 changes: 13 additions & 2 deletions packages/eslint-plugin-ecmascript-compat/lib/targetRuntimes.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const browserslist = require('browserslist');
const _ = require('lodash');
const compatData = require('@mdn/browser-compat-data');
const compareVersions = require('./compareVersions');

module.exports = function targetRuntimes() {
// ['chrome 50', ...]
Expand All @@ -9,13 +10,19 @@ module.exports = function targetRuntimes() {
// [ { name, version }, ... ]
const all = allNamedVersions.map((namedVersion) => {
const [name, version] = namedVersion.split(' ');
return { name, version };
return {
name,
version: simplifyVersion(version),
};
});

// { name: oldestVersion }
const oldestOfEach = _.chain(all)
.groupBy('name')
.mapValues((familyMember) => _.sortBy(familyMember, 'version')[0].version)
.mapValues(
(familyMember) =>
familyMember.sort((a, b) => compareVersions(a.version, b.version))[0].version
)
.value();

const mapped = _.mapKeys(oldestOfEach, (version, name) => mapFamilyName(name));
Expand Down Expand Up @@ -55,3 +62,7 @@ const familyNameMapping = {
function mapFamilyName(browserslistName) {
return familyNameMapping[browserslistName] || browserslistName;
}

function simplifyVersion(version) {
return version.includes('-') ? version.split('-')[0] : version;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ it('targets the oldest version of each family', () => {
expect(targetRuntimes()).toEqual([{ name: 'chrome', version: '50' }]);
});

it('targets the oldest version of each family, treating versions as numbers', () => {
process.env.BROWSERSLIST = 'chrome 9-10';

expect(targetRuntimes()).toEqual([{ name: 'chrome', version: '9' }]);
});

it('maps browserslist names to mdn names where necessary', () => {
process.env.BROWSERSLIST = 'Android >= 0';

Expand All @@ -36,5 +42,13 @@ it('handles multiple families', () => {
it('preserves versions as strings to allow semver', () => {
process.env.BROWSERSLIST = 'Node >= 0';

expect(targetRuntimes()).toEqual([{ name: 'nodejs', version: '0.10.0' }]);
expect(targetRuntimes()).toEqual([{ name: 'nodejs', version: '0.2.0' }]);
});

it('simplifies version ranges to the start of the range', () => {
expect(browserslist('iOS 15')).toEqual(['ios_saf 15.0-15.1']);

process.env.BROWSERSLIST = 'iOS 15';

expect(targetRuntimes()).toEqual([{ name: 'safari_ios', version: '15.0' }]);
});

0 comments on commit 659e9da

Please sign in to comment.