Skip to content

Commit

Permalink
jsx-max-props-per-line: Fix when prop spans multiple lines
Browse files Browse the repository at this point in the history
In this patch we now consider props to be on the same "line" if the line
of where one prop ends matches the line of where the next prop begins.

e.g.
```
<App foo={{
}} bar />
```

`foo` and `bar` are now considered to be on the same line. Previously,
we would only look at the line in which the prop begins.
  • Loading branch information
kentor committed Jan 29, 2017
1 parent 30e0206 commit bd20a79
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 18 deletions.
6 changes: 5 additions & 1 deletion docs/rules/jsx-max-props-per-line.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@ Limiting the maximum of props on a single line can improve readability.

## Rule Details

This rule checks all JSX elements and verifies that the number of props per line do not exceed the maximum allowed. A spread attribute counts as one prop. This rule is off by default and when on the default maximum of props on one line is `1`.
This rule checks all JSX elements and verifies that the number of props per line do not exceed the maximum allowed. Props are considered to be in a new line if there is a line break between the start of the prop and the end of the previous prop. A spread attribute counts as one prop. This rule is off by default and when on the default maximum of props on one line is `1`.

The following patterns are considered warnings:

```jsx
<Hello lastName="Smith" firstName="John" />;

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

The following patterns are not considered warnings:
Expand Down
33 changes: 16 additions & 17 deletions lib/rules/jsx-max-props-per-line.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

'use strict';

var has = require('has');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -45,30 +43,31 @@ module.exports = {

return {
JSXOpeningElement: function (node) {
var props = {};
if (!node.attributes.length) {
return;
}

var firstProp = node.attributes[0];
var linePartitionedProps = [[firstProp]];

node.attributes.forEach(function(decl) {
var line = decl.loc.start.line;
if (props[line]) {
props[line].push(decl);
node.attributes.reduce(function(last, decl) {
if (last.loc.end.line === decl.loc.start.line) {
linePartitionedProps[linePartitionedProps.length - 1].push(decl);
} else {
props[line] = [decl];
linePartitionedProps.push([decl]);
}
return decl;
});

for (var line in props) {
if (!has(props, line)) {
continue;
}
if (props[line].length > maximum) {
var name = getPropName(props[line][maximum]);
linePartitionedProps.forEach(function(propsInLine) {
if (propsInLine.length > maximum) {
var name = getPropName(propsInLine[maximum]);
context.report({
node: props[line][maximum],
node: propsInLine[maximum],
message: 'Prop `' + name + '` must be placed on a new line'
});
break;
}
}
});
}
};
}
Expand Down
19 changes: 19 additions & 0 deletions tests/lib/rules/jsx-max-props-per-line.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,24 @@ ruleTester.run('jsx-max-props-per-line', rule, {
].join('\n'),
errors: [{message: 'Prop `this.props` must be placed on a new line'}],
parserOptions: parserOptions
}, {
code: [
'<App',
' foo={{',
' }} bar',
'/>'
].join('\n'),
errors: [{message: 'Prop `bar` must be placed on a new line'}],
parserOptions: parserOptions
}, {
code: [
'<App',
' foo={{',
' }} bar baz',
'/>'
].join('\n'),
options: [{maximum: 2}],
errors: [{message: 'Prop `baz` must be placed on a new line'}],
parserOptions: parserOptions
}]
});

0 comments on commit bd20a79

Please sign in to comment.