Skip to content

jsx-first-prop-new-line: add multiline-multiprop option #883

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 1, 2016
Merged

jsx-first-prop-new-line: add multiline-multiprop option #883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2d6d7f
jsx-first-prop-new-line: add multiline-multiprop option
kentor Oct 4, 2016
b341300
Use jsx instead of js for jsx fence blocks
kentor Oct 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions docs/rules/jsx-first-prop-new-line.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ Ensure correct position of the first property.
This rule checks whether the first property of all JSX elements is correctly placed. There are three possible configurations:
* `always`: The first property should always be placed on a new line.
* `never` : The first property should never be placed on a new line, e.g. should always be on the same line as the Component opening tag.
* `multiline`: The first property should always be placed on a new line when the properties are spread over multiple lines.
* `multiline`: The first property should always be placed on a new line when the JSX tag takes up multiple lines.
* `multiline-multiprop`: The first property should always be placed on a new line if the JSX tag takes up multiple lines and there are multiple properties.

The following patterns are considered warnings when configured `"always"`:

```js
```jsx
<Hello personal={true} />

<Hello personal={true}
Expand All @@ -21,7 +22,7 @@ The following patterns are considered warnings when configured `"always"`:

The following patterns are not considered warnings when configured `"always"`:

```js
```jsx
<Hello
personal />

Expand All @@ -32,7 +33,7 @@ The following patterns are not considered warnings when configured `"always"`:

The following patterns are considered warnings when configured `"never"`:

```js
```jsx
<Hello
personal />

Expand All @@ -43,7 +44,7 @@ The following patterns are considered warnings when configured `"never"`:

The following patterns are not considered warnings when configured `"never"`:

```js
```jsx
<Hello personal={true} />

<Hello personal={true}
Expand All @@ -53,14 +54,19 @@ The following patterns are not considered warnings when configured `"never"`:

The following patterns are considered warnings when configured `"multiline"`:

```js
```jsx
<Hello personal
prop />
```

```jsx
<Hello foo={{
}} />
```

The following patterns are not considered warnings when configured `"multiline"`:

```js
```jsx
<Hello personal={true} />

<Hello
Expand All @@ -69,6 +75,27 @@ The following patterns are not considered warnings when configured `"multiline"`
/>
```

The following patterns are considered warnings when configured `"multiline-multiprop"`:

```jsx
<Hello foo={{
}}
bar />
```

The following patterns are not considered warnings when configured `"multiline-multiprop"`:

```jsx
<Hello foo={{
Copy link
Member

Choose a reason for hiding this comment

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

wait, why is this not a warning? The JSX tag is spanning multiple lines, so it should look like:

<Hello
  foo={{
    …
  }}
/>

Copy link
Contributor Author

@kentor kentor Oct 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

That would be handled by the existing multiline option. This is the multiline-multiprop option which does not warn because there is only 1 prop, but would if there are >= 2 props

I wanted this option to allow something like this:

<Hello style={{
  'background': 'black',
}} />

but not

<Hello style={{
  'background': 'black',
}} secondProp />

to fix the second case one would do (at a minimum):

<Hello 
  style={{
    'background': 'black',
  }} secondProp />

if the jsx-max-props-per-line rule is also enforced, then one would additionally have to move secondProp to its own line

<Hello 
  style={{
    'background': 'black',
  }} 
  secondProp 
/>

Copy link
Member

Choose a reason for hiding this comment

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

hmm, i misunderstood the purpose of this option - it seems like Airbnb's config already matches multiline, and we wouldn't use this particular option.

This thus seems like it does match what you're looking for.

}} />

<Hello
foo={{
}}
bar
/>
```

## When not to use

If you are not using JSX then you can disable this rule.
8 changes: 6 additions & 2 deletions lib/rules/jsx-first-prop-new-line.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = {
},

schema: [{
enum: ['always', 'never', 'multiline']
enum: ['always', 'never', 'multiline', 'multiline-multiprop']
}]
},

Expand All @@ -30,7 +30,11 @@ module.exports = {

return {
JSXOpeningElement: function (node) {
if ((configuration === 'multiline' && isMultilineJSX(node)) || (configuration === 'always')) {
if (
(configuration === 'multiline' && isMultilineJSX(node)) ||
(configuration === 'multiline-multiprop' && isMultilineJSX(node) && node.attributes.length > 1) ||
(configuration === 'always')
) {
node.attributes.forEach(function(decl) {
if (decl.loc.start.line === node.loc.start.line) {
context.report({
Expand Down
51 changes: 51 additions & 0 deletions tests/lib/rules/jsx-first-prop-new-line.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,39 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
options: ['multiline'],
parser: parserOptions
},
{
code: [
'<Foo bar />'
].join('\n'),
options: ['multiline-multiprop'],
parser: parserOptions
},
{
code: [
'<Foo bar baz />'
].join('\n'),
options: ['multiline-multiprop'],
parser: parserOptions
},
{
code: [
'<Foo prop={{',
'}} />'
].join('\n'),
options: ['multiline-multiprop'],
parser: parserOptions
},
{
code: [
'<Foo ',
' foo={{',
' }}',
' bar',
'/>'
].join('\n'),
options: ['multiline-multiprop'],
parser: parserOptions
},
{
code: '<Foo />',
options: ['always'],
Expand Down Expand Up @@ -144,6 +177,24 @@ ruleTester.run('jsx-first-prop-new-line', rule, {
options: ['never'],
errors: [{message: 'Property should be placed on the same line as the component declaration'}],
parser: parserOptions
},
{
code: [
'<Foo prop={{',
'}} />'
].join('\n'),
options: ['multiline'],
errors: [{message: 'Property should be placed on a new line'}],
parser: parserOptions
},
{
code: [
'<Foo bar={{',
'}} baz />'
].join('\n'),
options: ['multiline-multiprop'],
errors: [{message: 'Property should be placed on a new line'}],
parser: parserOptions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding tests!

}
]
});