* Add `formatWithCursor` API with `cursorOffset` option
This addresses https://github.com/prettier/prettier/issues/93 by
adding a new option, `cursorOffset`, that tells prettier to determine
the location of the cursor after the code has been formatted. This is
accessible through the API via a new function, `formatWithCursor`, which
returns a `{formatted: string, cursorOffset: ?number}`.
Here's a usage example:
```js
require("prettier").formatWithCursor(" 1", { cursorOffset: 2 });
// -> { formatted: '1;\n', cursorOffset: 1 }
```
* Add `--cursor-offset` CLI option
It will print out the offset instead of the formatted output. This
makes it easier to test. For example:
echo ' 1' | prettier --stdin --cursor-offset 2
# prints 1
* Add basic test of cursor translation
* Document `cursorOffset` option and `formatWithCursor()`
* Print translated cursor offset to stderr when --cursor-offset is given
This lets us continue to print the formatted code, while also
communicating the updated cursor position.
See https://github.com/prettier/prettier/pull/1637#discussion_r119735496
* doc-print cursor placeholder in comments.printComments()
See https://github.com/prettier/prettier/pull/1637#discussion_r119735149
* Compare array index to -1 instead of >= 0 to determine element presence
See https://github.com/prettier/prettier/pull/1637#discussion_r119736623
* Return {formatted, cursor} from printDocToString() instead of mutating options
See https://github.com/prettier/prettier/pull/1637#discussion_r119737354
This tweaks our JSX formatting to only put a JSX whitespace `{" "}` on a line by itself when it comes before or after a multiline element.
When preceding a text or single line element it appear on the same line as that element.
* Fix unstable JSX output by ensuring we follow `fill` rules.
This changes makes us more strict about ensuring our JSX children follow the alternating content/whitespace format expected by the `fill` primitive.
Previously there were some cases where could get out of sync which would throw out the formatting.
* Simplify whitespace wrangling
# Conflicts:
# src/printer.js
This avoids making it seems like it is indented by 4 characters instead of two. The downside is that if the condition is multi-line it's not going to be properly aligned, but I feel it's a better trade-offs. If you are doing nested ternaries, you usually have small conditions.
The docs go over a bunch of edge cases, might as well have it as a test :)
http://lesscss.org/features/
I just had to remove
```css
.weird-element {
content: ^//* some horrible but needed css hack;
}
```
but i'm not sure if it's real less.
We use a heuristic to figure out if it's a SCSS or Less file. And if it doesn't work, we try again with the other one. We do the same for JSX and TypeScript.
Fixes#1784
Babylon has a bug (I guess) with locations for classes where decorators are involved. Instead of the class starting at the first decorator, it starts at the beginning of the `class` keyword. By moving the location to the first comment, it solves --some-- of the issues with decorator comments.
The issue is really that the media query parser fails to parse the inner queries and just gives a raw string for the expression, but it should be safe to remove extra spaces. I can't make it rmeove spaces inside () that way unfortunately :(
If you put a space, `{loose: true}` is going to parse them as word + paren instead of func. It doesn't impact correctness and i'm not really sure how to clean the ast, so let's just make the test pass. I haven't seen this anywhere in real code.
It's very annoying to have to have a static definition of the ast, we should instead just traverse the objects to discover it. We just need to make sure not to have any cycles, but it's also good for debugging anyway.
This fixed a few comments already :)
A long time ago I introduced the ability to break for import statements. Then, later on, we removed the ability for require to break and go over the 80 column mark. In order to be consistent, we should do the same for import statements as well.
There can often be something that breaks inside of `extends` so it's looking weird to break twice. It now only breaks on `implements` and make sure to put each element on its own line.
Fixes#1520
I didn't intend for putting parenthesis there in the first place but it slept through. Since it was going to trigger a ton of changes I held up changing it, but I feel like we need to do it sooner than later otherwise we're not going to be able to do it. A lot of people writing functional components are going to be very happy about this change :)
TypeScript doesn't have the concept of `?` for nullable options and instead you have to write `| null` and `| void`. This is annoying to have it use the long form, so we're now inlining them.
While working on this, I found out a few issues with the way we deal with those:
- We only align objects if the parent is a union. This means that if you have `Array<{ object }>`, the object is not aligned properly. The fix is to move the alignment logic to the union, and not the child.
- When doing so, it messes up with the comment alignment, so we have to manually handle children comment printing in the union code.
It doesn't yet fix#1727 because the hardcoded type names are different, i'll follow up in a PR.
This keeps being requested and we're not using it at Facebook, so I don't particularly care which way it should be printed. We now force multiline if there's at least one declaration with a value. We don't want to break all the variables that are just declared.
Fixes#1607
We don't always want to automatically forward this option but we can always forward it to `n.body`. If it's an arrow function, it's doing the intended behavior, otherwise, it's not going to ignore it and not forward it. What we don't want is for arrow -> blockStatement -> arrow to get it, but we're good.
Fixes#1652
The function isPreviousLineEmpty comment doesn't skip comments (on purpose, see comment above that method :P) so the detection is messed up. Turns out, it's easier to just use isNextLineEmpty like everywhere else.
Fixes#1708
We completely butcher comments inside of template literals. The fix is to not be able to attach comments to TemplateExpression nodes and to avoid comments that are ine one expression to be moved to another expression. We now have the boundary to prevent outputting invalid code.
Fixes#1617
This is very unfortunate that we have to change the generic function that prints code but we want the JSXElement node to handle its comments printing itself in order to write them inside of the parenthesis instead of outside.
Fixes#555
I implemented indenting based on any character that's before the `${` but it was not the right behavior. Instead people want to indent based on the indentation of this line. It's easy to fix :)
Fixes parts of #1626
I wanted to see how hard it would be to add support for CSS inside of prettier. Turns out, it's not that hard. I spent a few hours printing post-css values and getting all the stylefmt unit tests to not throw.
I kept adding places where comments could be but more use cases kept creeping in. So now this is a holistical approach, we check all the comments that can be between two nodes. This way we should be good to go :)
* Keep unusual unicode spaces
This no longer converts unusual unicode whitespace characters (such as a non-breaking space) into normal spaces.
* Tweak comment based on PR feedback
* Move range extension code into helper functions
* Add findNodeByOffset() helper
This was adapted from cbc1929c64
* Test extending formatted range to entire node
* Fix extending formatted range to entire node
* Fix style errors
* Add run_file test function
This makes it possible to use different options on a per-file basis,
which is useful for things like range formatting tests.
* Test extending the format range to nearest parseable node
This means you can select the range of a `catch` clause, attempt to
format it, and have the `try` formatted as well, rather than throwing an
error.
* Fix extending the format range to nearest parseable node
This means you can select the range of a `catch` clause, attempt to
format it, and have the `try` formatted as well, rather than throwing an
error.
* Test that external indentation is left alone when formatting a range
* Preserve external indentation when formatting a range
* Dedupe range formatting traversal callbacks
* Simplify range formatting traversal using ast-types
See https://github.com/prettier/prettier/pull/1659#issuecomment-302974798
* Make range formatting traversal more efficient
There's less unnecessary parsing now.
* Fix style errors
* Add test where range expanding fails
* Fix test where range expanding fails
This makes sure that the range contains the entirety of the nodes
containing each of the range's endpoints.
* Add test for expanding range to beginning of line
* Pass test for expanding range to beginning of line
This makes it so that indentation before the range is added to the
formatted range.
* Don't parse/stringify AST to detect pre-range indentation
See https://github.com/prettier/prettier/pull/1659#discussion_r117790671
* When formatting a range, find closest statement rather than parsing
The `isStatement` implementation came from `docs/prettier.min.js`.
See https://github.com/prettier/prettier/pull/1659#issuecomment-303154770
* Add test for range-formatting a FunctionDeclaration's argument object
* Include FunctionDeclaration when searching for nearest node to range-format
From the spec, a Program is a series of SourceElements, each of which is
either a Statement or a FunctionDeclaration. See
https://www.ecma-international.org/ecma-262/5.1/#sec-A.5
* Remove unnecessary try-catch
See https://github.com/prettier/prettier/pull/1659#discussion_r117810096
* Add tests with multiple statements
See https://github.com/prettier/prettier/pull/1659#discussion_r117810753
* Remove unnecessary arguments from findNodeByOffset()
* Contract format range to ensure it starts/ends on nodes
* Specify test ranges in the fixtures
See https://github.com/prettier/prettier/pull/1659#discussion_r117811186
* Remove unnecessary comments from range fixtures
* Remove run_file test function
It's no longer used. This essentially reverts
8241216e68f2e0da997a4f558b03658d642c89a2
* Update range formatting docs
Clarify that the range expands to the nearest statement, and not to the
end of the line.
* Don't overwrite test options when detecting range
Now that multiple files share the same object again, we shouldn't be
re-assigning to it.
* Reuse already-read fixtures for AST_COMPARE=1 tests
* Remove `run_file` global from test eslintrc
* Undo package.json churn
`yarn` reformatted it before, but the whitespace visually sets off the
comment, so let's put it back how it was before.
See https://github.com/prettier/prettier/pull/1659#discussion_r117864655
* Remove misleading comments from isSourceElement
See https://github.com/prettier/prettier/pull/1659#discussion_r117865196
* Loop backwards through string instead of reversing it
See https://github.com/prettier/prettier/pull/1659#discussion_r117865759
* Don't recompute indent string when formatting range
See https://github.com/prettier/prettier/pull/1659#discussion_r117867268
* Rename findNodeByOffset to findNodeAtOffset
"Find x by y" is the common usage for finding an `x` by a key `y`.
However, since "by" has positional meaning, let's use "at" instead.
See https://github.com/prettier/prettier/pull/1659#discussion_r117865121
* Always trimRight() in formatRange and explain why
See https://github.com/prettier/prettier/pull/1659#discussion_r117864635
* Test formatting a range that crosses AST levels
See https://github.com/prettier/prettier/pull/1659#issuecomment-303243688
* Fix formatting a range that crosses AST levels
See https://github.com/prettier/prettier/pull/1659#issuecomment-303243688
* Remove unnecessary try-catch
See e52db5e9f9 (r117878763)
* Add test demonstrating range formatting indent detection
* Detect alignment from text on line before range, but don't reformat it
This avoids reformatting non-indentation that happens to precede the
range on the same line, while still correctly indenting the range based
on it.
See https://github.com/prettier/prettier/pull/1659#discussion_r117881430
* Improve ESLint setup
- Uses `eslint:recommended` + a handful more rules.
- Uses .eslintignore so that editors can understand which files to lint.
- Uses .eslintrc<strong>.js</strong> so more editors get syntax highlighting.
* Fix ESLint errors
About half of them were fixed by `eslint . --fix`.
* Add `--range-start` and `--range-end` options to format only parts of the input
These options default to `0` and `Infinity`, respectively, so that the
entire input is formatted by default. However, if either option is
specified such that a node lies completely outside the resulting range,
the node will be treated as if it has a `// prettier-ignore` comment.
Related to https://github.com/prettier/prettier/pull/1577#issuecomment-300551179
Related to https://github.com/prettier/prettier/issues/1324
Related to https://github.com/prettier/prettier/issues/593
* printer: Extract hasPrettierIgnoreComment() helper
* Move isOutsideRange() to util
* Don't throw errors about comments outside range "not printing"
* Remove unnecessary check from isOutsideRange()
* Make --range-end exclusive
This lets it use the conventional way of specifying ranges in strings.
Note that if the rangeEnd in the tests is changed to 158, it will fail,
but it wouldn't have failed before this change.
* Change range formatting approach
NOTE: This doesn't pass its test yet. Note that since we're reading the
indentation from the first line, it is expected not to change. However,
a semicolon is added, and the lines outside the range are not changed.
The new approach is roughly:
* Require that the range exactly covers an integer number of lines of the input
* Detect the indentation of the line the range starts on
* Format the range's substring using `printAstToDoc`
* Add enough `indent`s to the doc to restore the detected indentation
* Format the doc to a string with `printDocToString`
* Prepend/append the original input before/after the range
See https://github.com/prettier/prettier/pull/1609#issuecomment-301582273
---
Given `tests/range/range.js`, run the following:
prettier tests/range/range.js --range-start 165 --range-end 246
See the range's text with:
dd if=tests/range/range.js ibs=1 skip=165 count=81 2>/dev/null
* Don't use default function parameters
Node v4 doesn't support them. See
http://node.green/#ES2015-syntax-default-function-parameters
* Hackily fix indentation of range formatting
See
https://github.com/prettier/prettier/pull/1609#issuecomment-301625368
Also update the snapshot to reflect that the indentation actually should
decrease by one space, since there were 13 spaces in the input and we
round down after dividing by tabWidth.
* Revert "printer: Extract hasPrettierIgnoreComment() helper"
See https://github.com/prettier/prettier/pull/1609#discussion_r116804853
This reverts commit 62bf068ca98f69d4a7fd0ae188b3554d409eee8d.
* Test automatically using the beginning of the rangeStart line and same for the end
See https://github.com/prettier/prettier/pull/1609#issuecomment-301862076
* Fix automatically using the beginning of the rangeStart line and same for the end
See https://github.com/prettier/prettier/pull/1609#issuecomment-301862076
* Propagate breaks after adding an indentation-triggering hardline
See c1a61ebde8 (r116805581)
* Extract getAlignmentSize(), use instead of countIndents()
See c1a61ebde8 (r116804694)
* Extract addAlignmentToDoc(), use instead of addIndentsToDoc()
See c1a61ebde8 (r116804694)
* Document that --range-start and --range-end include the entire line
* Fix rangeStart calculation
Before, it was incorrectly resulting in 1 when the originally provided
value was 0
* Extract formatRange() helper function
* Move getAlignmentSize() from printer to util
This addresses https://github.com/prettier/prettier/pull/1609#discussion_r117636241
* Move addAlignmentToDoc() from printer to doc-builders
This addresses https://github.com/prettier/prettier/pull/1609#discussion_r117636251
This commit updates `npm run format:all` to not only format .js files in
src/ and bin/, but also tests_config/ and scripts/, as well as all
jsfmt.spec.js files.
It also includes the result of running `npm run format:all`, except
changes to src/ and bin/.
* fix TSCallSignature
* fix TSTypeReference
* remove test with invalid syntax
* --wip--
* partially fix TSTypeReference
* get comments right for TypeScript interfaces
This makes it easier to format code snippets including a `return`
statement, without having to format the entire function. For example,
given the following code:
```js
function f() {
return someVeryLongStringA && someVeryLongStringB && someVeryLongStringC && someVeryLongStringD
}
```
a developer could select the line containing `return`, then use
prettier to format the code to:
```js
function f() {
return (
someVeryLongStringA &&
someVeryLongStringB &&
someVeryLongStringC &&
someVeryLongStringD
);
}
```
which can then be reindented by the editor.
---
Related to https://github.com/prettier/prettier/issues/593
* Add test for empty object in logical expression
* Add check for empty object and array in shouldInlineLogicalExpression
* Review fixes, add additional case with function call
* (Babylon) Fall back to non-strict mode
This makes Prettier a little less opinionated about linting. For
example, the following can now be formatted:
```js
function f(a,a){return a}
```
whereas before it would cause an error:
stdin: SyntaxError: Argument name clash in strict mode (1:13)
> 1 | function f(a,a){return a}
| ^
This also allows octal numbers to be parsed,
and therefore fixes https://github.com/prettier/prettier/issues/228
If the code parses neither as strict nor as non-strict, the error from the
strict parse is thrown (as it was before this change).
---
I noticed this while trying out [eslump] with prettier:
eslump | pbcopy; pbpaste | prettier
[eslump]: https://github.com/lydell/eslump
* Add missing test
* Use Object.assign() instead of mutating object
* Don't let trailing template literal comments escape
This might not be the best-looking solution, but it's a start, and it
passes `AST_COMPARE=1 npm test`
Fixes https://github.com/prettier/prettier/issues/1559
* Add trailing space when changing line comment to block
* Be more discerning about template literal comment placement
Try to maintain the previous behavior if possible.
* Revert "Add trailing space when changing line comment to block"
This reverts commit 1eb42c24819a296c93a79b92a358d30a2aacc16c.
* Add new fill primitive and use it to wrap text in JSX
This adds a new `fill` primitive that can be used to fill lines with as much code as possible before moving to a new line with the same indentation.
It is used here layout JSX children. This gives us nicer wrapping for JSX elements containings lots of text interspersed with tags.
* Quick fix for jsx whitespace regressions
* Fix a couple more bugs
* Tidy up the `fill` algorithm
Attempt to make the algorithm a little more regular, and improve the naming of variables to make it a little easier to understand (I hope!).
* Small tidy up of JSX whitespace declarations
* Remove unnecessary code
It turns out that `children` is only used in the case when the element is printed on a single line, in which case all the types of JSX whitespaces behave the same, so we don't need to special case leading/trailing/solitary whitespace.
* A little more tidy up based on PR feedback
* Fix up tests after rebasing
* Make it explicit that we keep multiple consecutive spaces
* Add an explanatory comment
* Fix broken snapshot in master
* Ignore existing commands when deciding whether content will fit when using fill
* Fix a bug where children would get incorrectly filled onto a line
* Tidy up JSX whitespace names
* Print directive literals verbatim
This addresses https://github.com/prettier/prettier/issues/1555,
but doesn't seem to pass the AST_COMPARE=1 tests:
AST_COMPARE=1 npm test -- tests/quotes -t strings
However, running `prettier --debug-check` on the relevant file *does*
work:
prettier tests/quotes/strings.js --debug-check
* Change directive literal quotes if it doesn't contain quotes
This addresses https://github.com/prettier/prettier/pull/1560#discussion_r115396257
From https://github.com/prettier/prettier/issues/1555#issue-227206837:
> It's okay to change the type of quotation marks used, but only if
doing so does not require changing any characters within the directive.
* Don't change directive literal quotes if it contains a backslash
This passes the `--debug-check` tests again:
prettier tests/quotes/strings.js --debug-check
* Try to add regression test for escaped directive literals
This seems not to work, despite the following command having the correct
output:
echo "'\''" | prettier
You can use the following to get an idea of how flow/typescript parse
this:
node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'flow'}), null, 2)"
node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'typescript'}), null, 2)"
* WIP Disable Flow/Typescript for ./tests/directives
We don't yet handle escaped directives for them, but Babylon works.
(similar to 90bf93713c (diff-0de18284f37da79ab8af4e4690919abaR1))
* Revert "WIP Disable Flow/Typescript for ./tests/directives"
This reverts commit 2aba6231271f6985a395c31e3df9323e8f3da115.
* Prevent test strings from being parsed as directives
See https://github.com/prettier/prettier/pull/1560#issue-227225960
* Add more escaped directive tests
* Infer DirectiveLiterals from Flow parser
* Don't test TypeScript on directives
See https://github.com/prettier/prettier/pull/1560#issuecomment-300296221
* fixup! Infer DirectiveLiterals from Flow parser
* Don't fake objects that look like a DirectiveLiteral
Instead, add a flag to nodeStr() that deals with the Flow node
accordingly. See https://github.com/prettier/prettier/pull/1560#discussion_r115605758
* Print preferred quotes around escaped DirectiveLiteral when it doesn't contain quotes
See https://github.com/prettier/prettier/pull/1560#discussion_r115606122
* Simplify `canChangeDirectiveQuotes` logic
* Add directive test with unnecessarily escaped non-quote character
* Fix boolean logic error
I thought that this would result in the following if-block executing, as
needed to pass the test case in the previous commit. However, it appears
that it's not actually needed to pass the test case, since `makeString`
doesn't unescape unnecessarily escaped non-quote characters.
Nevertheless, I think we should leave that if-block (`if (canChangeDirectiveQuotes)`)
there, in case `makeString` is updated.
See https://github.com/prettier/prettier/pull/1571#discussion_r115658398
* Make isFlowDirectiveLiteral a separate argument to nodeStr()
See https://github.com/prettier/prettier/pull/1571#discussion_r115810988
* Simplify isFlowDirectiveLiteral logic by passing n.expression to nodeStr()
See https://github.com/prettier/prettier/pull/1571#discussion_r115811216
* Print directive literals verbatim
This addresses https://github.com/prettier/prettier/issues/1555,
but doesn't seem to pass the AST_COMPARE=1 tests:
AST_COMPARE=1 npm test -- tests/quotes -t strings
However, running `prettier --debug-check` on the relevant file *does*
work:
prettier tests/quotes/strings.js --debug-check
* Change directive literal quotes if it doesn't contain quotes
This addresses https://github.com/prettier/prettier/pull/1560#discussion_r115396257
From https://github.com/prettier/prettier/issues/1555#issue-227206837:
> It's okay to change the type of quotation marks used, but only if
doing so does not require changing any characters within the directive.
* Don't change directive literal quotes if it contains a backslash
This passes the `--debug-check` tests again:
prettier tests/quotes/strings.js --debug-check
* Try to add regression test for escaped directive literals
This seems not to work, despite the following command having the correct
output:
echo "'\''" | prettier
You can use the following to get an idea of how flow/typescript parse
this:
node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'flow'}), null, 2)"
node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'typescript'}), null, 2)"
* WIP Disable Flow/Typescript for ./tests/directives
We don't yet handle escaped directives for them, but Babylon works.
(similar to 90bf93713c (diff-0de18284f37da79ab8af4e4690919abaR1))
* Revert "WIP Disable Flow/Typescript for ./tests/directives"
This reverts commit 2aba6231271f6985a395c31e3df9323e8f3da115.
* Prevent test strings from being parsed as directives
See https://github.com/prettier/prettier/pull/1560#issue-227225960
* Add more escaped directive tests
* Infer DirectiveLiterals from Flow parser
* Don't test TypeScript on directives
See https://github.com/prettier/prettier/pull/1560#issuecomment-300296221
* fixup! Infer DirectiveLiterals from Flow parser
* fix(typescript): improve handling of computed properties
* test(typescript): add Symbol computed property test
* fix(typescript): do not print brackets for literals
* fix(typescript): fix module block, add enum initializers and fix type parameters
* fix(typescript): use printStatementSequence for TSModuleBlock
* fix(type-params): move typeParameters out of printFunctionParams
* refactor(type-params): move typeParameters out of printArgumentList
I have no idea if it's even valid but it threw on 8 typescript tests, now it doesn't.
```js
TypeError: Cannot read property 'type' of null
at printMemberLookup (prettier/src/printer.js:3062:16)
```
In 1.3.0, we shipped a change that makes template literal always inlined as single arguments of a function. The problem with template literals is that they whitespace is significant so we can't change it. There are two cases:
```js
call(`
template
template
`);
```
and
```js
call(
`template
template`
);
```
If you always make the same decision to inline, you're going to be wrong for the other use case. The solution that I found that works is to figure out if there's a `\n` before the backtick `` ` ``. If that's the case, then don't inline, otherwise do. We're trying to avoid looking at the source as much as possible but this is one example where we actually don't have a choice if we want to keep the output sane.
1.3.0 made the jest codebase significantly worse because of this. The issue is that once things have been moved, this heuristic won't be able to undo it. So people need to have this fix applied before they run 1.3.0, otherwise it's going to damage their codebase unless they manually change everything back, which is a pain. So I'm going to land this as a hotfix in 1.3.1.
Fixes#1492
* Make slice simpler.
* Drop htmlEscapeInsideAngleBracket function.
Use child.extra.raw (Babylon) and child.raw (Flow) to get the value instead.
* Remove unused htmlEscapeInsideAngleBracket function.
* Update test cases accordingly.
* Fix merge conflict.
* Update tests via `npm test -- -u`.
* Fix multiple consecutive spaces preservation in printJSXChildren.
* Update tests accordingly.
* fix(typescript): fix handling of new keyword
* fix(typescript): fix handling of call signatures
* feat(typescript): share type parameter formatting with flow
* feat(typescript): #1480: implement *Keyword, namespace function and class heritage
* feat(typescript): add type params and modifiers to interfaces
* chore(style): add squigly wings to if/else blocks
* fix(typescript): remove hardline before declare
We never want to break on those at it looks very weird. The reason why I didn't add this yet is because whenever it triggered in the past, the root cause was something else and it helped identify things to fix. But now the remaining ones I'm seeing are no longer underlying issues and they're just about this breaking unnecessarily, so we should just fix it.
This was added in order to follow some eslint rule but it's only confusing when it doesn't break, when it breaks the indentation makes it clear what is happening and you don't need parenthesis.
Fixes#1379
All of the discussions around ternaries are for the form
```js
cond1 ? elem1_if : cond2 ? elem2_if : elem_else
```
but some of them are for the form
```js
cond1 ? cond2 ? elem2_if : elem2_else : elem1_else
```
which is more rare and would be good to call out by adding parenthesis.
```js
cond1 ? (cond2 ? elem2_if : elem2_else) : elem1_else
```
Note that we only want parenthesis if it's written inline, otherwise the indentation is good enough to understand the flow:
```js
cond1
? cond2 ? elem2_if : elem2_else
: elem1_else
```
Right now, expressions inside of template literals are have the level of indentation of the start of the template literal. But oftentimes, expressions are nested inside of template literals and therefore the expression should have this level of nesting.
The heuristic i'm using to decide when to use the template literal nesting is if it's not the first line and if there isn't another expression before on the same line.
* Add eslint as dev dep, reorder scripts.
* Add tests & docs to eslintignore.
* Add eslintcache to gitignore.
* Update yarn lock file 😽.
* Add linting step in the test pipeline.
* Add a set of really basic rules for linting.
* Fix linting 🚀.
* No need for .jsx files to be linted...
* Reorder rules alphabetically.
* Refine rules: drop styling ones, only keep what provides dead code elimination.
* Add no-console rule to be consistent along with the no-debugger one.
* Remove empty line.
* Add eslint-disable-next-line no-console where console log/warn/error are allowed.
* Drop no-console rule.
* Remove eslint-disable-next-line no-console comments.
* Remove linting step in Travis CI.
* Fix linting after merging current master.
* Run `npm test -- -u` after noticing one test was out of sync.
* Drop eslint references from previous implementation.
* Revert yarn lock file.
* Revert scripts ordering.
* Fix incorrect yarn lock file.
In #1251, we now have a proper whitelist of all the types that should have parenthesis. Turns out, it included NullableTypeAnnotation which is `?a`. For `?a => void` this wasn't needed but for `(?(a => b)) => c` it was! It's better to always put it anyway if it's not just a simple literal.
I've added tests for all the combinations I could think of, so we'll catch regressions if they happen.
Fixes#1353
We've had this issue since the beginning and I tagged it as 1.0 but haven't managed to fix it by then. We shouldn't allow things to break in the argument list if we are in the last argument expansion mode. It turns out that we now have all the building blocks needed to fix this:
- have a special way to flag when we are printing the last argument expansion in the code that prints the argument list
- have a way to remove all the softlines from the argument list
Fixes#1301
This is the second part of the fix for the performance regression seen in #1250. In #1217, for correctness reasons, we're now traversing all the conditional groups. This means that we're now in O(n^2). But, in practice, many of those groups are === between each others. So we only need to recurse through one of the instances to know if it's going to break.
This makes the first example go from not terminating to being instant. The second one going from not terminating to taking ~1s. We can also make it instant by tweaking the printing phase, but that's for another PR.
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
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.
I've tried a lot of places where to fix this and this is the only one that gives correct results. This is likely not exhaustive but works for that use case. It's been reported twice in issues and I've seen it happen a few other times so we probably want to get it fixed.
Fixes#922Fixes#797