Skip to content

Commit

Permalink
Improve union types with leading comments (#5575)
Browse files Browse the repository at this point in the history
We're running into this issue at Facebook because `// flowfixme` ignores the very next line, but in this case an empty line is added in-between which breaks the fixme.

Ideally the solution is to avoid adding a newline and we'd call it a day. Unfortunately it is tricky to implement in this particular case. The comment is already printed, including its \n. Yet, we want to indent the block and the only way to convince the doc printer to do so is to add an indent group --before-- the \n is printed, otherwise it's just going to indent on the next \n.

So this PR changes the output from one bad way to another, but the new way has the benefit of not breaking flow for Facebook, which makes our internal teams happier.

Note that the way we print the same construct with `&` is broken in the same way as with this PR. https://prettier.io/playground/#N4Igxg9gdgLgprEAuc0DOMAEBbAngNQEMAbAVzkwF5MAKME4+4xhpl5tmgDgHYA2LgE5+g3gP5CRY6cL6iJE2fL4BKJAB0ombTt17tAegOYAwhGzYEMTftu7oFAGSYYAdwjOYACwBOTzABWACYA1gAeXgBmxCFowVEA5jEBaEGJyV6pIdEBCZkhQQGJIcQpXkGRCSFFWaWRXlWlaOWVwZElCUUlAZgqANwgADQgEAAOMACW6MighD4+EK4ACnMIaMgghABuEBNBQyAARj6EYCFwMADKo6cTUAnIMD7kw14w2MQA6l4T8Gg3YDglzWvwmW1+uA2YDQ62GdzQcB8MCWJwS2EIyEiJARwxSYQAQiczhdLoRLAAZO5wTHYuC4tBhS53JJwACKpAg8BpxBxIBuPgRPg2h0Ih1wxGgB1GPjuME+e28yC4AAZhtKIAjPidRhtpXBBVtqcM-ABHUgTPwowhojFILE8ukgBHYCaPZ6OtDM4hsjlcu204YwUXyoKKpAAJkDJwmxGZZgstpAUAcB1ICIAKqL1v6HQBfXNAA

Fixes #5572
  • Loading branch information
vjeux committed Nov 28, 2018
1 parent bd38340 commit 0af81c7
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/language-js/printer-estree.js
Expand Up @@ -2839,8 +2839,12 @@ function printPathNoParens(path, options, print, args) {
return join(" | ", printed);
}

const shouldAddStartLine =
shouldIndent &&
!hasLeadingOwnLineComment(options.originalText, n, options);

const code = concat([
ifBreak(concat([shouldIndent ? line : "", "| "])),
ifBreak(concat([shouldAddStartLine ? line : "", "| "])),
join(concat([line, "| "]), printed)
]);

Expand Down
22 changes: 22 additions & 0 deletions tests/flow_union/__snapshots__/jsfmt.spec.js.snap
@@ -1,5 +1,27 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`comment.js 1`] = `
====================================options=====================================
parsers: ["flow", "babylon"]
printWidth: 80
| printWidth
=====================================input======================================
const myValue = (callcallcallcallcallcall(87689769876876897698768768976987687689769876):
// Comment
one | two| thre | jdkxhflksjdhfglkjsdhfglkjhsdkfljghskdjhfgkljshdfgkjhsdkljfhgkljshdfgjdfklgjhklj );
=====================================output=====================================
const myValue = (callcallcallcallcallcall(
87689769876876897698768768976987687689769876
): // Comment
| one
| two
| thre
| jdkxhflksjdhfglkjsdhfglkjhsdkfljghskdjhfgkljshdfgkjhsdkljfhgkljshdfgjdfklgjhklj);
================================================================================
`;

exports[`union.js 1`] = `
====================================options=====================================
parsers: ["flow", "babylon"]
Expand Down
3 changes: 3 additions & 0 deletions tests/flow_union/comment.js
@@ -0,0 +1,3 @@
const myValue = (callcallcallcallcallcall(87689769876876897698768768976987687689769876):
// Comment
one | two| thre | jdkxhflksjdhfglkjsdhfglkjhsdkfljghskdjhfgkljshdfgkjhsdkljfhgkljshdfgjdfklgjhklj );

0 comments on commit 0af81c7

Please sign in to comment.