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
Docs: add more examples for no-sequences
rule
#14578
Conversation
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.
LGTM, thanks!
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.
The added examples LGTM, but since we already had a similar example with arrow function (right above the new examples) I think the idea in #14572 (comment) was to add a note. @nzakas ?
Yes, I think we should add a note that explicitly states this exception case as it’s different from anywhere else in the rule. |
@snitin315 are you still working on this? |
Yes, thanks for the ping. I will push an update today. |
Done ✅ |
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.
I was more thinking we should have a specific paragraph in the page saying something like:
Note about arrow function bodies
If an arrow function body is a statement rather than a block, and that statement contains a sequence, you need to use double parentheses around the statement to indicate that the sequence is intentional.
(include examples here)
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.
Exactly what I was looking for. Thank you!
/*eslint no-sequences: "error"*/ | ||
const foo = (val) => (console.log('bar'), val); | ||
|
||
const foo = () => ((bar = 123), 10)); |
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.
These is an extra )
in this example.
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.
Good catch, thanks. I will send a fix.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Add more examples.
Fixes #14572
Is there anything you'd like reviewers to focus on?
Nope