Improve JSX Formatting

* Arrow Function Expressions returning JSX will now add parens when the JSX breaks
* Conditional expressions within (or containing) JSX are formatted in a more natural way (for JSX), especiall when chained
* JSX in logical expressions (|| or &&) is always wrapped in parens

Fixes #2208
master
Stephen Scott 2017-07-05 00:45:06 -06:00
parent 1691b85322
commit 757037948a
9 changed files with 573 additions and 123 deletions

View File

@ -462,6 +462,7 @@ function genericPrintNoParens(path, options, print, args) {
(n.body.type === "ArrayExpression" ||
n.body.type === "ObjectExpression" ||
n.body.type === "BlockStatement" ||
n.body.type === "JSXElement" ||
isTemplateOnItsOwnLine(n.body, options.originalText) ||
n.body.type === "ArrowFunctionExpression")
) {
@ -1224,21 +1225,81 @@ function genericPrintNoParens(path, options, print, args) {
return concat(parts);
case "ConditionalExpression": {
const parent = path.getParentNode();
const printed = concat([
line,
"? ",
n.consequent.type === "ConditionalExpression" ? ifBreak("", "(") : "",
align(2, path.call(print, "consequent")),
n.consequent.type === "ConditionalExpression" ? ifBreak("", ")") : "",
line,
": ",
align(2, path.call(print, "alternate"))
]);
let forceNoIndent = false;
let firstNonConditionalParent;
let i = 0;
do {
firstNonConditionalParent = path.getParentNode(i);
i++;
} while (
firstNonConditionalParent &&
firstNonConditionalParent.type === "ConditionalExpression"
);
// Conditional expressions are intentionally formatted differently when
// they appear within JSX or contain JSX. Instead of:
//
// test
// ? consequent
// : alternate;
//
// They look like:
//
// test ? (
// consequent
// ) : (
// alternate
// )
//
if (
n.consequent.type === "JSXElement" ||
n.alternate.type === "JSXElement" ||
parent.type === "JSXExpressionContainer" ||
firstNonConditionalParent.type === "JSXExpressionContainer"
) {
forceNoIndent = true;
// Even though they don't need parens, we wrap (almost) everything in
// parens when using ?: within JSX, because the parens are analagous to
// curly braces in an if statement.
const wrap = node =>
concat(["(", indent(concat([hardline, node])), hardline, ")"]);
const NO_WRAP_TYPES = {
ConditionalExpression: true,
// JSXElements are always wrapped when they're the child of a ConditionalExpression.
JSXElement: true
};
parts.push(
" ? ",
NO_WRAP_TYPES[n.consequent.type]
? path.call(print, "consequent")
: wrap(path.call(print, "consequent")),
" : ",
NO_WRAP_TYPES[n.alternate.type]
? path.call(print, "alternate")
: wrap(path.call(print, "alternate"))
);
} else {
parts.push(
line,
"? ",
n.consequent.type === "ConditionalExpression" ? ifBreak("", "(") : "",
align(2, path.call(print, "consequent")),
n.consequent.type === "ConditionalExpression" ? ifBreak("", ")") : "",
line,
": ",
align(2, path.call(print, "alternate"))
);
}
return group(
concat([
path.call(print, "test"),
parent.type === "ConditionalExpression" ? printed : indent(printed)
parent.type === "ConditionalExpression" || forceNoIndent
? concat(parts)
: indent(concat(parts))
])
);
}
@ -4018,15 +4079,20 @@ function maybeWrapJSXElementInParens(path, elem) {
JSXElement: true,
JSXExpressionContainer: true,
ExpressionStatement: true,
CallExpression: true,
ConditionalExpression: true,
LogicalExpression: true,
ArrowFunctionExpression: true
CallExpression: true
};
if (NO_WRAP_PARENTS[parent.type]) {
return elem;
}
const ALWAYS_WRAP_PARENTS = {
LogicalExpression: true,
ConditionalExpression: true
};
if (ALWAYS_WRAP_PARENTS[parent.type]) {
return concat(["(", indent(concat([hardline, elem])), hardline, ")"]);
}
return group(
concat([
ifBreak("("),
@ -4060,6 +4126,10 @@ function shouldInlineLogicalExpression(node) {
return true;
}
if (node.right.type === "JSXElement") {
return true;
}
return false;
}

View File

@ -11,7 +11,7 @@ const render2 = ({ styles }) => <div style={styles} key="something">
Create wrapping parens.
</div>;
const render3 = ({ styles }) => <div style={styles} key="something">Bump to next line without parens</div>;
const render3 = ({ styles }) => <div style={styles} key="something">Create wrapping parens.</div>;
const render4 = ({ styles }) => <div style={styles} key="something">Create wrapping parens and indent <strong>all the things</strong>.</div>;
@ -75,29 +75,33 @@ const renderTernary = (props) =>
{props.showTheOtherThing ? <div>I am here!!</div> : null}
</BaseForm>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
const render1 = ({ styles }) =>
const render1 = ({ styles }) => (
<div style={styles} key="something">
Keep the wrapping parens. Put each key on its own line.
</div>;
</div>
);
const render2 = ({ styles }) =>
const render2 = ({ styles }) => (
<div style={styles} key="something">
Create wrapping parens.
</div>;
</div>
);
const render3 = ({ styles }) =>
const render3 = ({ styles }) => (
<div style={styles} key="something">
Bump to next line without parens
</div>;
Create wrapping parens.
</div>
);
const render4 = ({ styles }) =>
const render4 = ({ styles }) => (
<div style={styles} key="something">
Create wrapping parens and indent <strong>all the things</strong>.
</div>;
</div>
);
const render5 = ({ styles }) => <div>Keep it on one line.</div>;
const render6 = ({ styles }) =>
const render6 = ({ styles }) => (
<div attr1="aaaaaaaaaaaaaaaaa" attr2="bbbbbbbbbbb" attr3="cccccccccccc">
<div
attr1="aaaaaaaaaaaaaaaaa"
@ -131,23 +135,26 @@ const render6 = ({ styles }) =>
</div>{" "}
<strong>hello</strong>
</div>
</div>;
</div>
);
const render7 = () =>
const render7 = () => (
<div>
<span />
<span>Dont break each elem onto its own line.</span> <span />
<div /> <div />
</div>;
</div>
);
const render7A = () =>
const render7A = () => (
<div>
<div />
<div />
<div />
</div>;
</div>
);
const render7B = () =>
const render7B = () => (
<div>
<span>
{" "}
@ -159,15 +166,18 @@ const render7B = () =>
<span>
Dont break plz<span />
</span>
</div>;
</div>
);
const render8 = props => <div>{props.text}</div>;
const render9 = props =>
<div>{props.looooooooooooooooooooooooooooooong_text}</div>;
const render10 = props =>
const render9 = props => (
<div>{props.looooooooooooooooooooooooooooooong_text}</div>
);
const render10 = props => (
<div>
{props.even_looooooooooooooooooooooooooooooooooooooooooonger_contents}
</div>;
</div>
);
const notJSX = (aaaaaaaaaaaaaaaaa, bbbbbbbbbbb) =>
this.someLongCallWithParams(aaaaaa, bbbbbbb).anotherLongCallWithParams(
@ -186,7 +196,7 @@ React.render(
document.querySelector("#react-root")
);
const renderTernary = props =>
const renderTernary = props => (
<BaseForm
url="/auth/google"
method="GET"
@ -194,41 +204,56 @@ const renderTernary = props =>
size="large"
submitLabel="Sign in with Google"
>
{props.showTheThing
? <BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>
: "hello " + "howdy! "}
{props.showTheThing
? <BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>
: null}
{props.showTheThing
? null
: <BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>}
{props.showTheOtherThing ? <div>I am here</div> : <div attr="blah" />}
{props.showTheOtherThing ? <div>I am here!!</div> : null}
</BaseForm>;
{props.showTheThing ? (
<BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>
) : (
"hello " + "howdy! "
)}
{props.showTheThing ? (
<BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>
) : (
null
)}
{props.showTheThing ? (
null
) : (
<BaseForm
url="/auth/google"
method="GET"
colour="blue"
size="large"
submitLabel="Sign in with Google"
>
Hello world
</BaseForm>
)}
{props.showTheOtherThing ? (
<div>I am here</div>
) : (
<div attr="blah" />
)}
{props.showTheOtherThing ? (
<div>I am here!!</div>
) : (
null
)}
</BaseForm>
);
`;

View File

@ -8,7 +8,7 @@ const render2 = ({ styles }) => <div style={styles} key="something">
Create wrapping parens.
</div>;
const render3 = ({ styles }) => <div style={styles} key="something">Bump to next line without parens</div>;
const render3 = ({ styles }) => <div style={styles} key="something">Create wrapping parens.</div>;
const render4 = ({ styles }) => <div style={styles} key="something">Create wrapping parens and indent <strong>all the things</strong>.</div>;

View File

@ -1,5 +1,84 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`conditional-expression.js 1`] = `
<div>
{isCool ? "cool" : "very nice"}
</div>;
// should get formatted JSX style
const a = something ? "string" : <div />;
// should not get formatted JSX style
const a = something ? "string" : "other string";
<div>
{reallyLongChainOfConditionalExpressions ? (
"why"
) : yes ? (
"it"
) : is ? (
"!"
) : (
":)"
)}
</div>;
<div>
{reallyLongChainOfConditionalExpressions ? (
<span>why</span>
) : yes ? (
<span>it</span>
) : is ? (
<span>!</span>
) : (
<span>:)</span>
)}
</div>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<div>
{isCool ? (
"cool"
) : (
"very nice"
)}
</div>;
// should get formatted JSX style
const a = something ? (
"string"
) : (
<div />
);
// should not get formatted JSX style
const a = something ? "string" : "other string";
<div>
{reallyLongChainOfConditionalExpressions ? (
"why"
) : yes ? (
"it"
) : is ? (
"!"
) : (
":)"
)}
</div>;
<div>
{reallyLongChainOfConditionalExpressions ? (
<span>why</span>
) : yes ? (
<span>it</span>
) : is ? (
<span>!</span>
) : (
<span>:)</span>
)}
</div>;
`;
exports[`expression.js 1`] = `
<View
style={
@ -89,20 +168,21 @@ exports[`expression.js 1`] = `
/>;
<Something>
{() =>
<SomethingElse>
<span />
</SomethingElse>}
</Something>;
<Something>
{items.map(item =>
{() => (
<SomethingElse>
<span />
</SomethingElse>
)}
</Something>;
<Something>
{items.map(item => (
<SomethingElse>
<span />
</SomethingElse>
))}
</Something>;
<Something>
{function() {
return (
@ -193,32 +273,35 @@ exports[`hug.js 1`] = `
</div>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<div>
{__DEV__
? this.renderDevApp()
: <div>
{routes.map(route =>
<MatchAsync
key={\`\${route.to}-async\`}
pattern={route.to}
exactly={route.to === "/"}
getComponent={routeES6Modules[route.value]}
/>
)}
</div>}
</div>;
<div>
{__DEV__ &&
{__DEV__ ? (
this.renderDevApp()
) : (
<div>
{routes.map(route =>
{routes.map(route => (
<MatchAsync
key={\`\${route.to}-async\`}
pattern={route.to}
exactly={route.to === "/"}
getComponent={routeES6Modules[route.value]}
/>
)}
</div>}
))}
</div>
)}
</div>;
<div>
{__DEV__ && (
<div>
{routes.map(route => (
<MatchAsync
key={\`\${route.to}-async\`}
pattern={route.to}
exactly={route.to === "/"}
getComponent={routeES6Modules[route.value]}
/>
))}
</div>
)}
</div>;
<div>
@ -228,6 +311,58 @@ exports[`hug.js 1`] = `
`;
exports[`logical-expression.js 1`] = `
<div>
{a || "b"}
</div>;
<div>
{a && "b"}
</div>;
<div>
{a || <span></span>}
</div>;
<div>
{a && <span></span>}
</div>;
<div>
{a && <span>
<div>
<div></div>
</div>
</span>}
</div>;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<div>{a || "b"}</div>;
<div>{a && "b"}</div>;
<div>
{a || (
<span />
)}
</div>;
<div>
{a && (
<span />
)}
</div>;
<div>
{a && (
<span>
<div>
<div />
</div>
</span>
)}
</div>;
`;
exports[`object-property.js 1`] = `
const tabs = [
{
@ -330,6 +465,111 @@ exports[`quotes.js 1`] = `
`;
exports[`return-statement.js 1`] = `
const NonBreakingArrowExpression = () => <div />;
const BreakingArrowExpression = () => <div>
<div>
bla bla bla
</div>
</div>;
const NonBreakingArrowExpressionWBody = () => {
return (
<div />
);
};
const BreakingArrowExpressionWBody = () => {
return <div>
<div>
bla bla bla
</div>
</div>
};
const NonBreakingFunction = function() {
return (
<div />
);
};
const BreakingFunction = function() {
return <div>
<div>
bla bla bla
</div>
</div>
};
class NonBreakingClass extends React.component {
render() {
return (
<div />
);
}
}
class BreakingClass extends React.component {
render() {
return <div>
<div>
bla bla bla
</div>
</div>;
}
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
const NonBreakingArrowExpression = () => <div />;
const BreakingArrowExpression = () => (
<div>
<div>bla bla bla</div>
</div>
);
const NonBreakingArrowExpressionWBody = () => {
return <div />;
};
const BreakingArrowExpressionWBody = () => {
return (
<div>
<div>bla bla bla</div>
</div>
);
};
const NonBreakingFunction = function() {
return <div />;
};
const BreakingFunction = function() {
return (
<div>
<div>bla bla bla</div>
</div>
);
};
class NonBreakingClass extends React.component {
render() {
return <div />;
}
}
class BreakingClass extends React.component {
render() {
return (
<div>
<div>bla bla bla</div>
</div>
);
}
}
`;
exports[`spacing.js 1`] = `
const Labels = {
label1: (

View File

@ -0,0 +1,33 @@
<div>
{isCool ? "cool" : "very nice"}
</div>;
// should get formatted JSX style
const a = something ? "string" : <div />;
// should not get formatted JSX style
const a = something ? "string" : "other string";
<div>
{reallyLongChainOfConditionalExpressions ? (
"why"
) : yes ? (
"it"
) : is ? (
"!"
) : (
":)"
)}
</div>;
<div>
{reallyLongChainOfConditionalExpressions ? (
<span>why</span>
) : yes ? (
<span>it</span>
) : is ? (
<span>!</span>
) : (
<span>:)</span>
)}
</div>;

View File

@ -0,0 +1,23 @@
<div>
{a || "b"}
</div>;
<div>
{a && "b"}
</div>;
<div>
{a || <span></span>}
</div>;
<div>
{a && <span></span>}
</div>;
<div>
{a && <span>
<div>
<div></div>
</div>
</span>}
</div>;

View File

@ -0,0 +1,53 @@
const NonBreakingArrowExpression = () => <div />;
const BreakingArrowExpression = () => <div>
<div>
bla bla bla
</div>
</div>;
const NonBreakingArrowExpressionWBody = () => {
return (
<div />
);
};
const BreakingArrowExpressionWBody = () => {
return <div>
<div>
bla bla bla
</div>
</div>
};
const NonBreakingFunction = function() {
return (
<div />
);
};
const BreakingFunction = function() {
return <div>
<div>
bla bla bla
</div>
</div>
};
class NonBreakingClass extends React.component {
render() {
return (
<div />
);
}
}
class BreakingClass extends React.component {
render() {
return <div>
<div>
bla bla bla
</div>
</div>;
}
}

View File

@ -109,19 +109,21 @@ true
]
});
true
? test({
a: 1
})
: <div
a={123412342314}
b={123412341234}
c={123412341234}
d={123412341234}
e={123412341234}
f={123412341234}
g={123412341234}
/>;
true ? (
test({
a: 1
})
) : (
<div
a={123412342314}
b={123412341234}
c={123412341234}
d={123412341234}
e={123412341234}
f={123412341234}
g={123412341234}
/>
);
`;
@ -282,11 +284,11 @@ const els = items.map(item => (
</div>
));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
const els = items.map(item =>
const els = items.map(item => (
<div className="whatever">
<span>{children}</span>
</div>
);
));
`;

View File

@ -17,9 +17,11 @@ room = room.map((row, rowIndex) => (
const funnelSnapshotCard =
(report === MY_OVERVIEW && !ReportGK.xar_metrics_active_capitol_v2) ||
(report === COMPANY_OVERVIEW &&
!ReportGK.xar_metrics_active_capitol_v2_company_metrics)
? <ReportMetricsFunnelSnapshotCard metrics={metrics} />
: null;
!ReportGK.xar_metrics_active_capitol_v2_company_metrics) ? (
<ReportMetricsFunnelSnapshotCard metrics={metrics} />
) : (
null
);
room = room.map((row, rowIndex) =>
row.map(
@ -52,9 +54,11 @@ room = room.map((row, rowIndex) => (
const funnelSnapshotCard =
(report === MY_OVERVIEW && !ReportGK.xar_metrics_active_capitol_v2) ||
(report === COMPANY_OVERVIEW &&
!ReportGK.xar_metrics_active_capitol_v2_company_metrics)
? <ReportMetricsFunnelSnapshotCard metrics={metrics} />
: null;
!ReportGK.xar_metrics_active_capitol_v2_company_metrics) ? (
<ReportMetricsFunnelSnapshotCard metrics={metrics} />
) : (
null
);
room = room.map((row, rowIndex) =>
row.map(