From 8613046fd264be4bc44dc3a5649c836169f6400f Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Wed, 3 May 2017 14:35:58 -0700 Subject: [PATCH] Respect template inline-ness (#1497) 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 --- src/printer.js | 24 +++- .../strings/__snapshots__/jsfmt.spec.js.snap | 36 ++++-- .../template/__snapshots__/jsfmt.spec.js.snap | 111 ++++++++++++++++++ tests/template/inline.js | 58 +++++++++ 4 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 tests/template/inline.js diff --git a/src/printer.js b/src/printer.js index 718d3472e..91ecfdc7e 100644 --- a/src/printer.js +++ b/src/printer.js @@ -377,8 +377,7 @@ function genericPrintNoParens(path, options, print, args) { n.body.type === "ObjectExpression" || n.body.type === "JSXElement" || n.body.type === "BlockStatement" || - n.body.type === "TaggedTemplateExpression" || - n.body.type === "TemplateLiteral" || + isTemplateOnItsOwnLine(n.body, options.originalText) || n.body.type === "ArrowFunctionExpression" ) { return group(collapsed); @@ -706,7 +705,8 @@ function genericPrintNoParens(path, options, print, args) { // We want to keep require calls as a unit (n.callee.type === "Identifier" && n.callee.name === "require") || // Template literals as single arguments - n.arguments.length === 1 && n.arguments[0].type === "TemplateLiteral" || + (n.arguments.length === 1 && + isTemplateOnItsOwnLine(n.arguments[0], options.originalText)) || // Keep test declarations on a single line // e.g. `it('long name', () => {` (n.callee.type === "Identifier" && @@ -2084,7 +2084,7 @@ function genericPrintNoParens(path, options, print, args) { return concat([path.call(print, "elementType"), "[]"]); case "TSPropertySignature": parts.push(path.call(print, "name")); - + if (n.typeAnnotation) { parts.push(": "); parts.push(path.call(print, "typeAnnotation")); @@ -4011,6 +4011,22 @@ function shouldHugArguments(fun) { ); } +function templateLiteralHasNewLines(template) { + return template.quasis.some(quasi => quasi.value.raw.includes('\n')); +} + +function isTemplateOnItsOwnLine(n, text) { + return ( + (n.type === "TemplateLiteral" && templateLiteralHasNewLines(n) || + n.type === "TaggedTemplateExpression" && templateLiteralHasNewLines(n.quasi)) && + !util.hasNewline( + text, + util.locStart(n), + {backwards: true} + ) + ); +} + function printArrayItems(path, options, printPath, print) { const printedElements = []; let separatorParts = []; diff --git a/tests/strings/__snapshots__/jsfmt.spec.js.snap b/tests/strings/__snapshots__/jsfmt.spec.js.snap index 25833c930..57190faef 100644 --- a/tests/strings/__snapshots__/jsfmt.spec.js.snap +++ b/tests/strings/__snapshots__/jsfmt.spec.js.snap @@ -123,19 +123,25 @@ pipe.write( \`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\`, ); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -foo(\`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3} with expr\`); +foo( + \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3} with expr\` +); const x = \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { return 3; })() + 3 + 2 + 3 + 2 + 3} with expr\`; -foo(\`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { - const x = 5; +foo( + \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { + const x = 5; - return x; - })() + 3 + 2 + 3 + 2 + 3} with expr\`); + return x; + })() + 3 + 2 + 3 + 2 + 3} with expr\` +); -pipe.write(\`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\`); +pipe.write( + \`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\` +); `; @@ -154,18 +160,24 @@ pipe.write( \`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\`, ); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -foo(\`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3} with expr\`); +foo( + \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3} with expr\`, +); const x = \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { return 3; })() + 3 + 2 + 3 + 2 + 3} with expr\`; -foo(\`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { - const x = 5; +foo( + \`a long string \${1 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + 3 + 2 + (function() { + const x = 5; - return x; - })() + 3 + 2 + 3 + 2 + 3} with expr\`); + return x; + })() + 3 + 2 + 3 + 2 + 3} with expr\`, +); -pipe.write(\`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\`); +pipe.write( + \`\\n \${chalk.dim(\`\\u203A and \${more} more \${more} more \${more} more \${more}\`)}\`, +); `; diff --git a/tests/template/__snapshots__/jsfmt.spec.js.snap b/tests/template/__snapshots__/jsfmt.spec.js.snap index d20917b97..39668647d 100644 --- a/tests/template/__snapshots__/jsfmt.spec.js.snap +++ b/tests/template/__snapshots__/jsfmt.spec.js.snap @@ -140,6 +140,117 @@ module.exports = Relay.createContainer( `; +exports[`inline.js 1`] = ` +this._pipe.write(\`\\n\\n Pattern matches \${total} \${pluralizeTest}\`); +this._pipe.write( + \`\\n\\n Pattern matches \${total} \${pluralizeTest}\` +); +this._pipe + .write( + \`\\n\\n Pattern matches \${total} \${pluralizeTest}\` + ); + +this._pipe.write(\`\\n\\n Pattern matches \${total} \${pluralizeTest} but that's long\`); + +this._pipe.write( + \`\\n\\n Pattern matches \${total} \${pluralizeTest} but that's long\` +); + +this._pipe.write(\` + \\n\\n Pattern matches \${total} \${pluralizeTest} but that's long +\`); + + +() => \` + a +\`; + +() => + \` + a + \`; + +(): { + someLong: boolean, + t: boolean +} => \` + a +\`; + +(): { + someLong: boolean, + t: boolean +} => + \` + a + \`; + +( + someLong: boolean, + t: boolean +) => \` + a + \`; + +( + someLong: boolean, + t: boolean +) => + \` + a + \`; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +this._pipe.write(\`\\n\\n Pattern matches \${total} \${pluralizeTest}\`); +this._pipe.write(\`\\n\\n Pattern matches \${total} \${pluralizeTest}\`); +this._pipe.write(\`\\n\\n Pattern matches \${total} \${pluralizeTest}\`); + +this._pipe.write( + \`\\n\\n Pattern matches \${total} \${pluralizeTest} but that's long\` +); + +this._pipe.write( + \`\\n\\n Pattern matches \${total} \${pluralizeTest} but that's long\` +); + +this._pipe.write(\` + \\n\\n Pattern matches \${total} \${pluralizeTest} but that's long +\`); + +() => \` + a +\`; + +() => + \` + a + \`; + +(): { + someLong: boolean, + t: boolean +} => \` + a +\`; + +(): { + someLong: boolean, + t: boolean +} => + \` + a + \`; + +(someLong: boolean, t: boolean) => \` + a + \`; + +(someLong: boolean, t: boolean) => + \` + a + \`; + +`; + exports[`parenthesis.js 1`] = ` // "ArrowFunctionExpression" (() => {})\`\`; diff --git a/tests/template/inline.js b/tests/template/inline.js new file mode 100644 index 000000000..24bd0c651 --- /dev/null +++ b/tests/template/inline.js @@ -0,0 +1,58 @@ +this._pipe.write(`\n\n Pattern matches ${total} ${pluralizeTest}`); +this._pipe.write( + `\n\n Pattern matches ${total} ${pluralizeTest}` +); +this._pipe + .write( + `\n\n Pattern matches ${total} ${pluralizeTest}` + ); + +this._pipe.write(`\n\n Pattern matches ${total} ${pluralizeTest} but that's long`); + +this._pipe.write( + `\n\n Pattern matches ${total} ${pluralizeTest} but that's long` +); + +this._pipe.write(` + \n\n Pattern matches ${total} ${pluralizeTest} but that's long +`); + + +() => ` + a +`; + +() => + ` + a + `; + +(): { + someLong: boolean, + t: boolean +} => ` + a +`; + +(): { + someLong: boolean, + t: boolean +} => + ` + a + `; + +( + someLong: boolean, + t: boolean +) => ` + a + `; + +( + someLong: boolean, + t: boolean +) => + ` + a + `;