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

Don't break self closing elements with no attributes #3250

Merged
merged 5 commits into from Nov 13, 2017
Merged

Don't break self closing elements with no attributes #3250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5663ee6
Refactor JSXOpeningElement code
duailibe Nov 11, 2017
2cf85e0
Prevent breaking self closing elements
duailibe Nov 11, 2017
49cfd04
Update snapshot
duailibe Nov 11, 2017
3f4dbd5
Refactor
duailibe Nov 11, 2017
950861e
Fix refactor bug
duailibe Nov 12, 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
28 changes: 15 additions & 13 deletions src/printer.js
View file
Open in desktop
Expand Up @@ -1791,6 +1791,14 @@ function genericPrintNoParens(path, options, print, args) {
case "JSXOpeningElement": {
const n = path.getValue();

const nameHasComments =
n.name && n.name.comments && n.name.comments.length > 0;

// Don't break self-closing elements with no attributes and no comments
if (n.selfClosing && !n.attributes.length && !nameHasComments) {
return concat(["<", path.call(print, "name"), " />"]);
}

// don't break up opening elements with a single long text attribute
if (
n.attributes.length === 1 &&
Expand All @@ -1805,10 +1813,8 @@ function genericPrintNoParens(path, options, print, args) {
// attr="value"
// // comment
// >
!(
(n.name && n.name.comments && n.name.comments.length) ||
(n.attributes[0].comments && n.attributes[0].comments.length)
)
!nameHasComments &&
(!n.attributes[0].comments || !n.attributes[0].comments.length)
) {
return group(
concat([
Expand All @@ -1821,6 +1827,9 @@ function genericPrintNoParens(path, options, print, args) {
);
}

const lastAttrHasTrailingComments =
n.attributes.length && hasTrailingComment(util.getLast(n.attributes));

const bracketSameLine =
options.jsxBracketSameLine &&
// We should print the bracket in a new line for the following cases:
Expand All @@ -1830,15 +1839,8 @@ function genericPrintNoParens(path, options, print, args) {
// <div
// attr // comment
// >
!(
(n.name &&
!(n.attributes && n.attributes.length) &&
n.name.comments &&
n.name.comments.length) ||
(n.attributes &&
n.attributes.length &&
hasTrailingComment(util.getLast(n.attributes)))
);
(!nameHasComments || n.attributes.length) &&
!lastAttrHasTrailingComments;

return group(
concat([
Expand Down
6 changes: 6 additions & 0 deletions tests/comments_jsx_same_line/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Expand Up @@ -26,6 +26,9 @@ exports[`jsx_same_line.js 1`] = `
>
{foo}
</div>;

<br // comment
/>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<div
// comment
Expand All @@ -52,4 +55,7 @@ exports[`jsx_same_line.js 1`] = `
{foo}
</div>;

<br // comment
/>;

`;
3 changes: 3 additions & 0 deletions tests/comments_jsx_same_line/jsx_same_line.js
View file
Open in desktop
Expand Up @@ -23,3 +23,6 @@
>
{foo}
</div>;

<br // comment
/>;
16 changes: 16 additions & 0 deletions tests/jsx-text-wrap/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Expand Up @@ -308,6 +308,13 @@ x =
<div>
{" "}{" "}{" "}
</div>

// Don't break a self-closing element without attributes
// ----------
x =
<p>
text text text text text text text text text text text text text text text<br />text text text text text text
</p>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Wrapping text
x = (
Expand Down Expand Up @@ -702,4 +709,13 @@ x = (
// NOTE: Multiple JSX whitespaces are collapsed into a single space.
x = <div> </div>;

// Don't break a self-closing element without attributes
// ----------
x = (
<p>
text text text text text text text text text text text text text text text<br />text
Copy link
Member Author

Choose a reason for hiding this comment

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

To help review, the print width is in the middle of <br />

text text text text text
</p>
);

`;
7 changes: 7 additions & 0 deletions tests/jsx-text-wrap/test.js
View file
Open in desktop
Expand Up @@ -305,3 +305,10 @@ x =
<div>
{" "}{" "}{" "}
</div>

// Don't break a self-closing element without attributes
// ----------
x =
<p>
text text text text text text text text text text text text text text text<br />text text text text text text
</p>;