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

Incorrect pass on jsx-closing-bracket-location (tag-aligned) for multi-line prop #889

Closed
msrose opened this issue Oct 6, 2016 · 3 comments
Labels

Comments

@msrose
Copy link

msrose commented Oct 6, 2016

For jsx-closing-bracket-location (tag-aligned), if the last prop is declared on the same line as the opening bracket, the rule expects the closing bracket to be after the last prop even when the last prop spans multiple lines:

const MyComponent = () => (
    <div>

        <div className={[
            "some",
            "stuff",
            2 ]}>           {/* expected error, but passes lint */}
            Some text
        </div>

        <div className={[
            "some",
            "stuff",
            2 ]}
        >                   {/* expected to pass, but produces error */}
            Some text
        </div>

    </div>
);

The error is correctly reported when the prop is moved to the next line:

<div 
    className={[
        "some",
        "stuff",
        2 ]}>       {/* errors here, as expected (closing bracket should be aligned with opening bracket) */}
        Some text
</div>

Here's my eslint config:

// eslint@3.7.1
// eslint-plugin-react@6.3.0
module.exports = {
    parserOptions: {
        ecmaVersion: 6,
        ecmaFeatures: {
            "jsx": true
        }
    },
    plugins: ['react'],
    rules: {
        'react/jsx-closing-bracket-location': ['error', 'tag-aligned']
    }
};

I've done a quick look at the source with @captbaritone - if I find a solution I'll make a PR.

@ljharb
Copy link
Member

ljharb commented Oct 6, 2016

<div className={[
            "some",
            "stuff",
            2 ]}
        >

should not in fact pass - the following should:

<div
  className={[
    "some",
    "stuff",
    2
  ]}
>

However, I agree your first example should not pass.

@tuures
Copy link
Contributor

tuures commented Nov 3, 2016

I mostly agree with the examples above. However I think it should also be possible to configure following to be valid which is no longer possible with the change (e2f72f1):

// I want to write:
<Comp flag={true} style={{
  color: 'blue'
}}>
  child
</Comp>

// eslint wants:
<Comp flag={true} style={{
  color: 'blue'
  }}
>
  child
</Comp>

// I agree should not be valid:
<Comp flag={true} style={{
  color: 'blue'
  }}>
  child
</Comp>

I like to follow the rule that opening braces that are on the same line should have their corresponding closing braces on the same line. non-JSX analogy:

// good:
_.forEach(list, l => ({
  prop: l.prop
}));

// not so good:
_.forEach(list, l => ({
  prop: l.prop
  })
);

@ljharb
Copy link
Member

ljharb commented Nov 3, 2016

Although I don't use that style I agree that contents of braces/parens/brackets should always be indented one level deeper than the lines containing the marks, when multiline.

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

No branches or pull requests

4 participants