robatwilliams / es-compat

Check JavaScript code compatibility with target runtime environments
MIT License
65 stars 13 forks source link

Version comparison inadequately compares strings #30

Closed anomiex closed 1 year ago

anomiex commented 3 years ago

Test file:

[].includes( 'x' );

Browserslist: safari >= 14, ios_saf >= 14

Expected output:

es-compat: checking compatibility for targets { safari_ios: '14.0-14.4', safari: '14' }

No issues found. Files are compatible with the target runtimes.

Actual output:

es-compat: checking compatibility for targets { safari_ios: '14.0-14.4', safari: '14' }

/tmp/foo.js
  1:1  error  ES2016 method 'Array.prototype.includes' is forbidden  ecmascript-compat/compat

✖ 1 problem (1 error, 0 warnings)

Notes: The dataset used reports that the feature was added in version 9 of both browsers. While 14 > 9 is true, '14' > '9' is false.

dossy commented 2 years ago

@lamansky I tried testing this using gitpkg.now.sh, and it doesn't fix the problem for me. How were you able to test this and confirm that it fixes the problem for you?

$ yarn add -D 'https://gitpkg.now.sh/lamansky/es-compat/packages/eslint-plugin-ecmascript-compat?fix-issue-30'

Editing to add: For some reason, VS Code's built-in eslint didn't pick up this change, but when executing yarn lint from the command line to run eslint, it appears to fix the issue. Nice!

robatwilliams commented 1 year ago

46 and #55 were proposed for this

robatwilliams commented 1 year ago

Look what eslint-plugin-compat does: https://github.com/amilajack/eslint-plugin-compat/blob/main/src/providers/mdn-provider.ts

robatwilliams commented 1 year ago

There is also an issue where it does the sort for oldest versions in targetRuntimes.js

robatwilliams commented 1 year ago

Released https://github.com/robatwilliams/es-compat/releases/tag/v3.0.0

with fix for this

FYI @anomiex @dossy @lamansky @NoelDeMartin @NikolayFrantsev @ka2n