Enable the unicorn/{prefer-includes,throw-new-error} linting rules#18571
Conversation
|
/botio-linux browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/cbbef2459b3343c/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/cbbef2459b3343c/output.txt Total script time: 20.63 mins
Image differences available at: http://54.241.84.105:8877/cbbef2459b3343c/reftest-analyzer.html#web=eq.log |
For more information refer to https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-includes.md. Fortunately this only required one change because we already use `.includes()` everywhere else. Note that that is mostly due to the `mozilla/use-includes-instead-of-indexOf` rule which we replace with this new rule now because it's more complete.
For more information refer to https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/throw-new-error.md. This didn't require any changes because we already do this correctly, but it ensures that new code remains consistent and explicit.
Snuffleupagus
left a comment
There was a problem hiding this comment.
Something for another patch might be to remove mozilla/avoid-removeChild, and thus the mozilla-plugin itself, since that one seems to be covered by unicorn/prefer-dom-node-remove now?
r=me, thank you!
|
Good idea; that should also help to get closer to ESLint 9. Unfortunately we use one more Mozilla rule at https://github.com/mozilla/pdf.js/blob/master/extensions/chromium/.eslintrc#L19, but if I remove that it only results in five failures nowadays (given that we moved away from most globals a while ago) so I wonder if we can maybe just go ahead and just explicitly document the globals we have left there? In any case, I'll document this idea on the ESLint 9 ticket (since |
The commit messages contain more information about the individual changes.