Skip to content

Commit

Permalink
sort-comp rule: fix bug affecting instance-methods group
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michaelhogg committed Apr 21, 2018
1 parent d68ef96 commit 1870758
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
7 changes: 4 additions & 3 deletions lib/rules/sort-comp.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,10 +384,11 @@ module.exports = {
node.value.type !== 'ArrowFunctionExpression' &&
node.value.type !== 'FunctionExpression',
instanceMethod: !node.static &&
node.type === 'ClassProperty' &&
node.value &&
(node.value.type === 'ArrowFunctionExpression' ||
node.value.type === 'FunctionExpression'),
(
(node.type === 'ClassProperty' && node.value.type === 'ArrowFunctionExpression') ||
(node.type === 'MethodDefinition' && node.value.type === 'FunctionExpression')
),
typeAnnotation: !!node.typeAnnotation && node.value === null
}));

Expand Down
4 changes: 2 additions & 2 deletions tests/lib/rules/sort-comp.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,8 @@ ruleTester.run('sort-comp', rule, {
code: [
'class Hello extends React.Component {',
' foo = () => {}',
' constructor() {}',
' classMethod() {}',
' constructor() {}',
' static bar = () => {}',
' render() {',
' return <div>{this.props.text}</div>;',
Expand Down Expand Up @@ -573,7 +573,7 @@ ruleTester.run('sort-comp', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{message: 'foo should be placed before constructor'}],
errors: [{message: 'classMethod should be placed before constructor'}],
options: [{
order: [
'instance-methods',
Expand Down

0 comments on commit 1870758

Please sign in to comment.