expiring-todo-comments: Fix compatibility with ESLint 9.15
#2497
+14
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
changed the title
defaultOptions to expiring-todo-comments ruledefaultOptions to expiring-todo-comments rule
|
@sindresorhus I see you opened #2498, this PR is related to that |
|
Would love to see this merged, this prevents us from bumping eslint |
…arcalexiei/eslint-plugin-unicorn into expiring-todo-comments-default-options
fisker
approved these changes
Nov 19, 2024
sindresorhus
changed the title
defaultOptions to expiring-todo-comments ruleexpiring-todo-comments: Fix compatibility with ESLint 9.15
1 task
voxpelli
added a commit
to voxpelli/eslint
that referenced
this pull request
Nov 25, 2024
Seems like eslint#17656 introduced an unintended change in `no-warnings-comments.js` where before it roughly looked like: ```js var configuration = context.options[0] || {}; var decoration = [...configuration.decoration || []].join(""); ``` And afterwards it looks like: ```js const [{ decoration, location, terms: warningTerms }] = context.options; ``` Which unintentionally(?) removes the `context.options[0] || {}` default, which eg. causes `eslint-plugin-unicorn@<15.0.1` to fail (see sindresorhus/eslint-plugin-unicorn#2497) Since this happened in a minor `eslint` release but the fix for `eslint-plugin-unicorn` is only available as a patch to its latest major, this is causing me some issues. My guess is that you won't be wanting to merge this though as `eslint-plugin-unicorn` is [extending that rule](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/8b7c5fcf5c9743db4afde0bb9c90cc51782d47ef/rules/utils/get-builtin-rule.js#L4) through: ```js require('eslint/use-at-your-own-risk').builtinRules.get('no-warning-comments') ``` And as discussed in eslint#19013 you don't consider that a good practise: > It's in unstable APIs because rule APIs are indeed not stable > Core rules are not intended to extended any you do so at your own risk Perhaps eslint#19013 or similar can suggest an alternative for eg. `eslint-plugin-unicorn` to avoid this happening again in the future, but my guess is you consider such suggestions to be out of scope of `eslint` and as such to not be something you ant to get involved in. Here's a PR fixing the regression at least, you decide if you want to accept it or reject it 🤷♂️
1 task
|
Added a PR to fix this in upstream as well: eslint/eslint#19166 To avoid failures for those using unicorn <15.x I guess backporting of this PR to 14.x is not something that you would be interested in? |
|
That issue was closed referencing eslint/eslint#19169 that will document that extension of built in rules are discouraged, they should be forked instead. |
7 tasks
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #2496.
I added the default options to
expiring-todo-commentsrule to avoid error when running this config with[email protected].I also added
IteratortolanguageOptions.globalsotherwise the recommended config test will fail.I tested that the patch works also with
[email protected]but let me know if want that I try to test multiple version ofESLintinCIworkflowWarning
I cloned and checkout
mainbranch then I installed the dependencies usingnpm i.After that I ran
npm run lintI gotis this expected or I have missed some step during setup?