-
Notifications
You must be signed in to change notification settings - Fork 10.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.