Add no-invalid-remove-event-listener rule
#1216
Conversation
|
@bschlenk Friendly bump :) |
| const MESSAGE_ID = 'no-invalid-remove-event-listener'; | ||
| const messages = { | ||
| [MESSAGE_ID]: | ||
| 'Store the result of this expression in a variable that is passed to both `removeEventListener` the corresponding `addEventListener`.' |
There was a problem hiding this comment.
How about The listener agument should be a function reference.?
This comment has been minimized.
This comment has been minimized.
fisker
changed the title
no-invalid-event-listener ruleno-invalid-remove-event-listener rule
I commonly use |
|
Looks good, I'll make some adjustment. |
|
@bschlenk I can't push to your fork, please merge this https://github.com/bschlenk/eslint-plugin-unicorn/pull/1. Next time try not to use the main branch. |
|
Can you improve the report location a little bit, if it's a function, use |
| const isBindCall = node => | ||
| node.type === 'CallExpression' && | ||
| node.callee.type === 'MemberExpression' && | ||
| node.callee.property.name === 'bind' && | ||
| !node.callee.computed; |
There was a problem hiding this comment.
This function isn't used anymore. Do the below matchers handle when the callee is computed?
There was a problem hiding this comment.
Yes, remove it, all tested.
I didn't realize GitHub let you do that. When can you push to someone else's fork? |
|
Yes, people do this all the time. There is checkbox "allow maintainer edit"(something like that) on right side of this page, it should be checked. I'm not sure if it because you used default branch. |
|
That's cool! It does say you should be able to push to the main branch
|
|
Need fix style |
|
@bschlenk Can you rebase this and run |
|
Running |
|
You probably need to rebase from the main branch first. |
|
I forgot to npm install, I had an outdated xo 🤦♂️ |
|
@bschlenk Can you ensure the tests are passing? |
readme.md
Outdated
| @@ -167,6 +168,7 @@ Each rule has emojis denoting: | |||
| | [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. | ✅ | 🔧 | | | |||
| | [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. | ✅ | 🔧 | | | |||
| | [no-instanceof-array](docs/rules/no-instanceof-array.md) | Require `Array.isArray()` instead of `instanceof Array`. | ✅ | 🔧 | | | |||
| | [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling removeEventListener with the result of an expression | ✅ | | | | |||
There was a problem hiding this comment.
The description here needs to be regenerated.
There was a problem hiding this comment.
Is this what you mean? Make this description match the one from docs/rules/no-invalid-remove-event-listener.md?
There was a problem hiding this comment.
Run npm run generate-rules-table and npm run generate-usage-example
|
Thanks for helping, it's hard to keep up with all the changes in this repo. |
|
@sindresorhus I think this rule is ready for review, the code part looks good to me. |
|
Looks good :) |
Closes #682