Skip to content
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

Merged
merged 1 commit into from May 28, 2021
Merged

Docs: add more examples for no-sequences rule #14578

merged 1 commit into from May 28, 2021

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented May 8, 2021

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

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label May 8, 2021
Copy link
Member

@aladdin-add aladdin-add left a comment

LGTM, thanks!

@aladdin-add aladdin-add added accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules enhancement This change enhances an existing feature of ESLint and removed triage An ESLint team member will look at this issue soon labels May 12, 2021
Copy link
Member

@mdjermanovic mdjermanovic left a comment

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 ?

@nzakas
Copy link
Member

nzakas commented May 18, 2021

Yes, I think we should add a note that explicitly states this exception case as it’s different from anywhere else in the rule.

@nzakas
Copy link
Member

nzakas commented May 22, 2021

@snitin315 are you still working on this?

@snitin315
Copy link
Contributor Author

snitin315 commented May 22, 2021

Yes, thanks for the ping. I will push an update today.

@snitin315
Copy link
Contributor Author

snitin315 commented May 23, 2021

Done

Copy link
Member

@nzakas nzakas left a comment

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)

nzakas
nzakas approved these changes May 28, 2021
Copy link
Member

@nzakas nzakas left a comment

Exactly what I was looking for. Thank you!

@aladdin-add aladdin-add merged commit 958ff4e into eslint:master May 28, 2021
14 checks passed
/*eslint no-sequences: "error"*/
const foo = (val) => (console.log('bar'), val);

const foo = () => ((bar = 123), 10));
Copy link
Contributor

@fisker fisker May 31, 2021

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.

Copy link
Contributor Author

@snitin315 snitin315 May 31, 2021

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.

@snitin315 snitin315 deleted the docs-no-seq branch May 31, 2021
@snitin315 snitin315 mentioned this pull request May 31, 2021
1 task
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 25, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-sequences doesn't work correctly with assignment in arrow function expression
5 participants