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

Add instance-methods and instance-variables to sort-comp. #1470

Merged
merged 4 commits into from
Nov 27, 2017
Merged

Add instance-methods and instance-variables to sort-comp. #1470

merged 4 commits into from
Nov 27, 2017

Conversation

RDGthree
Copy link
Contributor

Allow sorting of instance methods and instance variables optionally with the rules instance-methods and instance-variables respectively. Rules are optional, and are applied immediately before everything-else to avoid overriding other rules.

Adjusting static-methods and adding static-variables would have been breaking so I skipped that for now.

Fixes #599.

@danieljvdm
Copy link

Hey guys, changes LGTM, let's get the ball rolling on merging this.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@GerritSe
Copy link

Any updates on the merge?

@ljharb ljharb merged commit c7dd755 into jsx-eslint:master Nov 27, 2017
michaelhogg added a commit to michaelhogg/eslint-plugin-react that referenced this pull request Apr 21, 2018
Regarding this issue:

    jsx-eslint#599
    sort-comp: Consider separating static fields & instance fields into separate groups

and this PR, released in v7.6.0:

    jsx-eslint#1470
    Add instance-methods and instance-variables to sort-comp.

there's a bug in the implementation of the `instance-methods` group.

Class instance methods can be declared in two different ways:

    class Main extends React.Component {

        // MethodDefinition -> FunctionExpression
        foo() {}

        // ClassProperty -> ArrowFunctionExpression
        bar = () => {}

    }

Using the `babel-eslint` parser, if a class instance method is
declared as a `FunctionExpression`, then the parent AST node is
a `MethodDefinition`, but the `sort-comp` rule is expecting the
parent node to be a `ClassProperty`, which is wrong.
@michaelhogg
Copy link

FYI: discussion of this PR is continuing in #1774.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants