search

wikimedia / eslint-plugin-no-jquery

Control allowance of certain jQuery functions, and suggest or autofix alternatives.
MIT License
32 stars 8 forks source link

Replace Array#indexOf with Array#includes #251

Closed edg2s closed 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #251 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #251   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           97        97           
  Lines          540       540           
=========================================
  Hits           540       540
Impacted Files Coverage Δ
src/rules/no-animate-toggle.js 100.00% <ø> (ø)
src/rules/no-parse-html-literal.js 100.00% <ø> (ø)
src/rules/no-sizzle.js 100.00% <ø> (ø)
src/rules/no-event-shorthand.js 100.00% <100.00%> (ø)
src/utils.js 100.00% <100.00%> (ø)
jdforrester commented 4 years ago

Does this effectively change our eslint compatibility? With this, we're now increasing our required node level above what eslint 2 requires, I think?

edg2s commented 4 years ago

Hopefully not as we were already using Array.includes...

edg2s commented 4 years ago

Also we only claim to support ESLint 2.3, not the lowest version of Node that that supports. You can use 2.3 with Node 14...

edg2s commented 4 years ago

Array.includes requires Node 6, but so does Object.assign, and probably a bunch of our dependencies.

jdforrester commented 4 years ago

WFM. Maybe we can put the engine requirement in package.json for clarity?