Hug elements inside of JSXExpressionContainer #213
Conversation
| @@ -1098,6 +1098,14 @@ function genericPrintNoParens(path, options, print) { | |||
| case "JSXSpreadAttribute": | |||
| return concat([ "{...", path.call(print, "argument"), "}" ]); | |||
| case "JSXExpressionContainer": | |||
| const shouldIndent = | |||
| n.expression.type !== 'ArrayExpression' && | |||
| n.expression.type !== 'ObjectExpression'; | |||
There was a problem hiding this comment.
We'll probably want to add more things here over time
vjeux
force-pushed
the
hug_jsx_expression
branch
2 times, most recently
from
03ae06b
to
0438805
Compare
|
|
||
| <Something> | ||
| {items.map( | ||
| item => ( |
There was a problem hiding this comment.
this one will be fixed by #211
vjeux
changed the title
vjeux
changed the title
|
|
||
| if (!shouldIndent) { | ||
| return concat(["{", path.call(print, "expression"), "}"]); | ||
| } |
There was a problem hiding this comment.
I'm not sure I get what the shouldIndent logic is? Take this example:
<Something>
{() => (
<SomethingElse>
<span />
</SomethingElse>
)}
</Something>;I thought that the indent below is the indentation for the opening { on the line 2. It's not clear to me why this bug is happening, but it's also getting late so I might not be thinking clearly... Thanks for fixing these JSX bugs, they've annoyed me as well!
I'd like to look at this again tomorrow before merging so I can understand what's happening.
There was a problem hiding this comment.
Yeah, I'm not really sure what's going on here and it would be nice to find out, instead of blindly sending a fix that happens to fix it.
There was a problem hiding this comment.
This code is good. The comment below about the group will fix other bugs.
|
Sorry I haven't merged this yet. I will go through this again tomorrow (there's something I want to understand here). I'm finishing up some other work right now. |
|
What's happening is the JSX code has hard lines, so it breaks across lines. The previous code assumed all code was on one line, and added indentation for itself when it breaks. This pattern doesn't work when the inner contents forces itself across lines because it'll inherit an extra indentation level. This bug will happen with anything that breaks itself up. For example, blocks have hard lines so we can repro with this: <Something>
{() => {
return 5;
}}
</Something>;If the inner contents may break themselves up, you need to use Otherwise, your PR looks good. It's a bit unrelated actually, just exposed this bug. |
| if (!shouldIndent) { | ||
| return concat(["{", path.call(print, "expression"), "}"]); | ||
| } | ||
|
|
||
| return group( |
There was a problem hiding this comment.
This needs to be a multilineGroup.
But this has me wondering: Should JSXExpressionContainer ever include lines and break the expression to next line?
There was a problem hiding this comment.
Great find, it also fixes a few more snapshots :)
vjeux
force-pushed
the
hug_jsx_expression
branch
from
0438805
to
e44b071
Compare
| @@ -200,13 +200,13 @@ var Example = React.createClass({ | |||
| var ok_void = ( | |||
| <Example | |||
| func={() => { | |||
| }} | |||
lock
bot
added
the
locked-due-to-inactivity
Fixes #197
Fixes #196