In #1250, @jeresig reported that adding // prettier-ignore on a file that has a ton of conditional group still took 1.7s. The issue is that we're printing before checking for the comment. Swaping the two had the unfortunate side effect of not marking the comments as being printed, so I had to skip that safety check if there's a prettier-ignore.
With this change, adding prettier-ignore makes the file be printed instantly!
The implementation was checking if the comment was inside of the expression range, which seems like a good idea. Unfortunately, the expression range is not what's inside of `${}` but the actual AST node, which incidentally doesn't include comments. So the logic was off and returned `undefined` which threw afterwards.
Another solution is to find the first quasi where start is > comment start. This means that the comment appeared between the quasi before and this one... therefore in the expression before!
The flow parser has issues with unicode where it makes node location invalid, there are likely other places where node locations are off. So instead of throwing with a weird error, let's attach it to the first one if it doesn't work.
Fixes#1293
As I was debugging #1248, I found out that the code to fix was actually making things worse. The other two branches are for decorators and deleting some random value of a function. I ran all the tests and the flow object is actually now preserving empty lines and didn't change anything else.
I'd rather remove all those and if something comes up then fix it properly upstream than having those crutches that we don't know why they exist anymore.
In #1257, I discovered that if there's a `""` doc at the end, it's not going to trim the previous one correctly. It also happens to fix a few existing things.
This one is pretty crazy. In #927, I changed
```js
concat(["(", join(concat([", "]), printed), ")"]),
```
into
```js
concat(["(", join(concat([", "]), printedLastArgExpanded), ")"]),
```
which makes the example in #1203 look ugly. The crazy thing is that `JSON.stringify(printed) === JSON.stringify(printedLastArgExpanded)`. So they are deep equal but not the same object. In a non-mutative world, this should cause any problem, but we actually mutate those to propagate breaks.
In the break propagation, we only looked at the first one in case of a conditional group. But, because the two were the same object then it also applied to the second one and happened to be the correct behavior! When I changed that piece of code to be two distinct objects, it no longer worked by accident and caused a bunch of weird issues where breaks didn't propagate correctly.
The solution for this case is to propagate the breaks on all the conditions. I'm pretty sure that this is the expected behavior, we want to deeply recurse in all of them, we don't propagate it up the boundary anyway.
The other use case for `traverseInDoc()` is `findInDoc()`, right now it searches for the first conditional group but it seems very arbitrary. I changed it to not search on any and none of the tests are failing, so I think it's safe(tm). If it triggers weird behavior, then it'll at least be expected and not randomly explode at us if we move groups around.
I tried to go through all the conditions for `findInDoc()` but it triggers a few failures (the output look bad). I'm not really sure why. https://gist.github.com/vjeux/5fb7566cc3d65974817d512d1ef6abe1Fix#1203
We shouldn't be reading from the printed document but instead look at the AST. This was really easy, it can either be EmptyStatement, BlockStatement or anything else.
In #847, I used a heuristic to find if the element was going to be expanded. But, it wasn't 100% accurate because we couldn't know in which conditionalGroup we would land. We added a way for the parent to tell that function if we should be in `expandLastArg`. By replacing the condition by this variable, it now fixes the issues!
This is so good that adding the right abstraction fixes problems across the board :)
Fixes#997
It turns out that we can't reliably detect with the ast if a comment is before or after the ). In order to fix this same problem with `if` I added the `getNextNonSpaceNonCommentCharacter` function. We can use the same here to fix the problem.
Fixes#933
* Hug template literals inside of JSXExpressionContainer
We already hug a bunch of things inside of `{}`. It seems that it's a good idea to do it for template literals as well. I don't think I've seen anyone actually indent them.
Fixes#1090
* Inline jsx elements with single template literal expression
If there is a single expression and a single template literal, then a lot of people in the jsx-style world inline it. I've also myself used this for markdown and printed it that way. So we probably should print it that way.
Note that I'm checking for children.length === 1, this means that if there's any whitespace, this is not going to be true and will not enter this case. So it WON'T reformat
```js
<style>
{`
color: red;
`}
</style>
```
into
```js
<style>{`
color: red;
`}</style>
```
which is great. You have to opt-in to the second style in order to get it.
Fixes#1090
I'm unclear whether anyone was ever confused by this but the eslint page is kind of compelling
```js
// The intent is not clear
var x = a => 1 ? 2 : 3;
// Did the author mean this
var x = function (a) { return 1 ? 2 : 3 };
// Or this
var x = a <= 1 ? 2 : 3;
```
Adding a parenthesis makes it valid with `{"allowParens": true}` rule. Note that if this option is not enabled, the code would not pass lint in the first place.
Christopher Chedeau