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.

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

Jump to bottom

[jsx-wrap-multilines] Add new nodes for wrap #1384

Merged
merged 13 commits into from
Oct 30, 2017
Merged

[jsx-wrap-multilines] Add new nodes for wrap #1384

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
eb039b8
Add new line before/after parentheses when wrap
Aug 11, 2017
04f3b9f
Add new nodes for wrap
Aug 11, 2017
ed319dc
[jsx-wrap-multilines] Fix typo
yepninja Aug 19, 2017
2bbd79c
[jsx-wrap-multilines] Add tests
yepninja Aug 19, 2017
21d9a20
[jsx-wrap-multilines] Better tests
yepninja Aug 19, 2017
657e77e
[jsx-wrap-multilines] Better tests for attributes
yepninja Aug 20, 2017
252c3ba
[jsx-wrap-multilines] Add docs
yepninja Aug 20, 2017
cea8305
[jsx-wrap-multilines] Fix example
yepninja Aug 22, 2017
a05c589
[jsx-wrap-multilines] Return tests
yepninja Aug 22, 2017
dc729fa
[jsx-wrap-multilines] Return invalid tests
yepninja Aug 22, 2017
41a29c6
[jsx-wrap-multilines] Fix tests. Check conditions by default
yepninja Aug 22, 2017
ccb9132
[jsx-wrap-multilines] Rename attr to prop
yepninja Sep 1, 2017
cfcb2bb
[jsx-wrap-multilines] Сompatibility with current default parametrs
yepninja Sep 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 63 additions & 1 deletion docs/rules/jsx-wrap-multilines.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ Wrapping multiline JSX in parentheses can improve readability and/or convenience

## Rule Details

This rule optionally takes a second parameter in the form of an object, containing places to apply the rule. By default, all the syntax listed below will be checked, but these can be explicitly disabled. Any syntax type missing in the object will follow the default behavior (become enabled).
This rule optionally takes a second parameter in the form of an object, containing places to apply the rule. By default, all the syntax listed below will be checked except the conditional expressions, logical expressions and JSX attributes, but these can be explicitly disabled. Any syntax type missing in the object will follow the default behavior (become enabled).

There are the possible syntax available:

* `declaration`
* `assignment`
* `return`
* `arrow`
* `condition` (not enabled by default)
* `logical` (not enabled by default)
* `attr` (not enabled by default)
Copy link
Member

Choose a reason for hiding this comment

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

this should probably be "prop" - in jsx, elements have props, and "attributes" are something HTML elements have (which jsx isn't)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!
I've renamed to prop


The following patterns are considered warnings:

Expand Down Expand Up @@ -118,3 +121,62 @@ var hello = () => (
</div>
);
```

The following patterns are considered warnings when configured `{condition: true}`.

```jsx
<div>
{foo ? <div>
<p>Hello</p>
</div> : null}
</div>
```

The following patterns are not considered warnings when configured `{condition: true}`.

```jsx
<div>
{foo ? (<div>
<p>Hello</p>
</div>) : null}
</div>
```


The following patterns are considered warnings when configured `{logical: true}`.

```jsx
<div>
{foo &&
<div>
<p>Hello World</p>
</div>
}
</div>
```

The following patterns are not considered warnings when configured `{logical: true}`.

```jsx
<div> not
Copy link
Member

Choose a reason for hiding this comment

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

?

```

The following patterns are considered warnings when configured `{attr: true}`.

```jsx
<div attr={<div>
<p>Hello</p>
</div>}>
<p>Hello</p>
</div>;
```

The following patterns are not considered warnings when configured `{attr: true}`.

```jsx
<div attr={(<div>
<p>Hello</p>
</div>)}>
<p>Hello</p>
</div>;
```
38 changes: 32 additions & 6 deletions lib/rules/jsx-wrap-multilines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ const DEFAULTS = {
declaration: true,
assignment: true,
return: true,
arrow: true
arrow: true,
condition: false,
logical: false,
attr: false
};

// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -44,6 +47,15 @@ module.exports = {
},
arrow: {
type: 'boolean'
},
condition: {
type: 'boolean'
},
logical: {
type: 'boolean'
},
attr: {
type: 'boolean'
}
},
additionalProperties: false
Expand Down Expand Up @@ -100,11 +112,6 @@ module.exports = {
if (!isEnabled('declaration')) {
return;
}
if (node.init && node.init.type === 'ConditionalExpression') {
check(node.init.consequent);
check(node.init.alternate);
return;
}
check(node.init);
},

Expand Down Expand Up @@ -132,6 +139,25 @@ module.exports = {
if (isEnabled('arrow') && arrowBody.type !== 'BlockStatement') {
check(arrowBody);
}
},

ConditionalExpression: function(node) {
if (isEnabled('condition')) {
check(node.consequent);
check(node.alternate);
}
},

LogicalExpression: function(node) {
if (isEnabled('logical')) {
check(node.right);
}
},

JSXAttribute: function (node) {
if (isEnabled('attr') && node.value && node.value.type === 'JSXExpressionContainer') {
check(node.value.expression);
}
}
};
}
Expand Down
164 changes: 98 additions & 66 deletions tests/lib/rules/jsx-wrap-multilines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,44 +52,6 @@ const RETURN_NO_PAREN = `
});
`;

const DECLARATION_TERNARY_SINGLE_LINE = 'var hello = foo ? <p>Hello</p> : <p>Hi</p>;';

const DECLARATION_TERNARY_PAREN = `
var hello = foo ? (<div>
<p>Hello</p>
</div>) : (<div>
<p>Hi</p>
</div>);
`;

const DECLARATION_TERNARY_NO_PAREN = `
var hello = foo ? <div>
<p>Hello</p>
</div> : <div>
<p>Hi</p>
</div>;
`;

const ASSIGNMENT_TERNARY_SINGLE_LINE = 'var hello; hello = foo ? <p>Hello</p> : <p>Hi</p>;';

const ASSIGNMENT_TERNARY_PAREN = `
var hello;
hello = foo ? (<div>
<p>Hello</p>
</div>) : (<div>
<p>Hi</p>
</div>);
`;

const ASSIGNMENT_TERNARY_NO_PAREN = `
var hello;
hello = foo ? <div>
<p>Hello</p>
</div> : <div>
<p>Hi</p>
</div>;
`;

const DECLARATION_SINGLE_LINE = 'var hello = <p>Hello</p>;';

const DECLARATION_PAREN = `
Expand Down Expand Up @@ -134,6 +96,68 @@ const ARROW_NO_PAREN = `
</div>;
`;

const CONDITION_SINGLE_LINE = 'foo ? <p>Hello</p> : null;';

const CONDITION_PAREN = `
<div>
{foo ? (<div>
<p>Hello</p>
</div>) : null}
</div>
`;

const CONDITION_NO_PAREN = `
<div>
{foo ? <div>
<p>Hello</p>
</div> : null}
</div>
`;

const LOGICAL_SINGLE_LINE = 'foo && <p>Hello</p>;';

const LOGICAL_PAREN = `
<div>
{foo &&
(<div>
<p>Hello World</p>
</div>)
}
</div>
`;

const LOGICAL_NO_PAREN = `
<div>
{foo &&
<div>
<p>Hello World</p>
</div>
}
</div>
`;

const ATTR_SINGLE_LINE = '<div attr={<p>Hello</p>}></div>';

const ATTR_PAREN = `
<div attr={
(<div>
<p>Hello</p>
</div>)
}>
<p>Hello</p>
</div>
`;

const ATTR_NO_PAREN = `
<div attr={
<div>
<p>Hello</p>
</div>
}>
<p>Hello</p>
</div>
`;

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------
Expand All @@ -149,20 +173,6 @@ ruleTester.run('jsx-wrap-multilines', rule, {
}, {
code: RETURN_NO_PAREN,
options: [{return: false}]
}, {
code: DECLARATION_TERNARY_SINGLE_LINE
}, {
code: DECLARATION_TERNARY_PAREN
}, {
code: DECLARATION_TERNARY_NO_PAREN,
options: [{declaration: false}]
}, {
code: ASSIGNMENT_TERNARY_SINGLE_LINE
}, {
code: ASSIGNMENT_TERNARY_PAREN
}, {
code: ASSIGNMENT_TERNARY_NO_PAREN,
options: [{assignment: false}]
}, {
code: DECLARATION_SINGLE_LINE
}, {
Expand All @@ -187,6 +197,27 @@ ruleTester.run('jsx-wrap-multilines', rule, {
}, {
code: ARROW_NO_PAREN,
options: [{arrow: false}]
}, {
code: CONDITION_SINGLE_LINE
}, {
code: CONDITION_NO_PAREN
}, {
code: CONDITION_PAREN,
options: [{condition: true}]
}, {
code: LOGICAL_SINGLE_LINE
}, {
code: LOGICAL_NO_PAREN
}, {
code: LOGICAL_PAREN,
options: [{logical: true}]
}, {
code: ATTR_SINGLE_LINE
}, {
code: ATTR_NO_PAREN
}, {
code: ATTR_PAREN,
options: [{attr: true}]
}
],

Expand All @@ -200,20 +231,6 @@ ruleTester.run('jsx-wrap-multilines', rule, {
output: RETURN_PAREN,
options: [{return: true}],
errors: [{message: 'Missing parentheses around multilines JSX'}]
}, {
code: DECLARATION_TERNARY_NO_PAREN,
Copy link
Member

Choose a reason for hiding this comment

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

why do these tests need to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same tests as tests for condition rule. I can return them if you think that they are useful. The more tests the better

Copy link
Member

Choose a reason for hiding this comment

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

For semver-minor PRs, it makes me happier when tests are only added :-)

output: DECLARATION_TERNARY_PAREN,
errors: [
{message: 'Missing parentheses around multilines JSX'},
{message: 'Missing parentheses around multilines JSX'}
]
}, {
code: ASSIGNMENT_TERNARY_NO_PAREN,
output: ASSIGNMENT_TERNARY_PAREN,
errors: [
{message: 'Missing parentheses around multilines JSX'},
{message: 'Missing parentheses around multilines JSX'}
]
}, {
code: DECLARATION_NO_PAREN,
output: DECLARATION_PAREN,
Expand All @@ -237,6 +254,21 @@ ruleTester.run('jsx-wrap-multilines', rule, {
output: ARROW_PAREN,
options: [{arrow: true}],
errors: [{message: 'Missing parentheses around multilines JSX'}]
}, {
code: CONDITION_NO_PAREN,
output: CONDITION_PAREN,
options: [{condition: true}],
errors: [{message: 'Missing parentheses around multilines JSX'}]
}, {
code: LOGICAL_NO_PAREN,
output: LOGICAL_PAREN,
options: [{logical: true}],
errors: [{message: 'Missing parentheses around multilines JSX'}]
}, {
code: ATTR_NO_PAREN,
output: ATTR_PAREN,
options: [{attr: true}],
errors: [{message: 'Missing parentheses around multilines JSX'}]
}
]
});