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

Support private properties for semi rule #11748

Merged
merged 2 commits into from
Jun 29, 2020
Merged

Support private properties for semi rule #11748

merged 2 commits into from
Jun 29, 2020

Conversation

simonkotwicz
Copy link
Contributor

@simonkotwicz simonkotwicz commented Jun 26, 2020

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Corresponding PR for eslint-plugin-babel repo: babel/eslint-plugin-babel#194 (comment)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 26, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c10e795:

Sandbox Source
cocky-agnesi-h76dk Configuration
delicate-dust-u6r2d Configuration

@existentialism existentialism added area: eslint PR: New Feature 🚀 A type of pull request used for our changelog categories labels Jun 26, 2020
}
}
}

export default ruleComposer.joinReports([
rule,
context => ({
ClassProperty(node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaicataldo Does matches selector support

":matches(ClassProperty, ClassPrivateProperty)"

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! As an example, you can see this core rule: https://github.com/eslint/eslint/blob/master/lib/rules/func-call-spacing.js#L171

We should be able to combine both of these selectors into one, e.g. "ClassProperty, ClassPrivateProperty"(node) {}.

Copy link
Contributor

@JLHwung JLHwung Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know that A, B is supported, guess we should update https://eslint.org/docs/developer-guide/selectors#what-syntax-can-selectors-have.

Update: estools/esquery#113

}
}
}

export default ruleComposer.joinReports([
rule,
context => ({
ClassProperty(node) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! As an example, you can see this core rule: https://github.com/eslint/eslint/blob/master/lib/rules/func-call-spacing.js#L171

We should be able to combine both of these selectors into one, e.g. "ClassProperty, ClassPrivateProperty"(node) {}.

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaicataldo 's concern has been addressed.

@JLHwung JLHwung merged commit 379e1c5 into babel:main Jun 29, 2020
@simonkotwicz simonkotwicz deleted the dev branch June 29, 2020 21:43
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Sep 29, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: eslint outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants