From 8d03423d17ffe4190a0b5207433e87e7a3d7b2d4 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Tue, 18 Apr 2017 08:40:08 -0700 Subject: [PATCH] Avoid breaking arguments for last arg expansion (#1305) 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 --- src/printer.js | 32 +++++++++---------- .../__snapshots__/jsfmt.spec.js.snap | 31 ++++++++++++++++++ tests/last_argument_expansion/break.js | 9 ++++++ 3 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 tests/last_argument_expansion/break.js diff --git a/src/printer.js b/src/printer.js index 991c1862b..bc31fab36 100644 --- a/src/printer.js +++ b/src/printer.js @@ -1437,21 +1437,6 @@ function genericPrintNoParens(path, options, print, args) { case "TemplateLiteral": var expressions = path.map(print, "expressions"); - function removeLines(doc) { - // Force this doc into flat mode by statically converting all - // lines into spaces (or soft lines into nothing). Hard lines - // should still output because there's too great of a chance - // of breaking existing assumptions otherwise. - return docUtils.mapDoc(doc, d => { - if (d.type === "line" && !d.hard) { - return d.soft ? "" : " "; - } else if (d.type === "if-break") { - return d.flatContents || ""; - } - return d; - }); - } - parts.push("`"); path.each(function(childPath) { @@ -2291,7 +2276,7 @@ function printFunctionParams(path, print, options, expandArg) { // }) ) => { // }) if (expandArg) { - return group(concat(["(", join(", ", printed), ")"])); + return group(concat(["(", join(", ", printed.map(removeLines)), ")"])); } // Single object destructuring should hug @@ -3579,6 +3564,21 @@ function printArrayItems(path, options, printPath, print) { return concat(printedElements); } +function removeLines(doc) { + // Force this doc into flat mode by statically converting all + // lines into spaces (or soft lines into nothing). Hard lines + // should still output because there's too great of a chance + // of breaking existing assumptions otherwise. + return docUtils.mapDoc(doc, d => { + if (d.type === "line" && !d.hard) { + return d.soft ? "" : " "; + } else if (d.type === "if-break") { + return d.flatContents || ""; + } + return d; + }); +} + function printAstToDoc(ast, options) { function printGenerically(path, args) { return comments.printComments( diff --git a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap b/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap index 96e5329a0..d56fb1305 100644 --- a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap +++ b/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap @@ -39,6 +39,37 @@ export default function searchUsers(action$) { `; +exports[`break.js 1`] = ` +export default class AddAssetHtmlPlugin { + apply(compiler: WebpackCompilerType) { + compiler.plugin('compilation', (compilation: WebpackCompilationType) => { + compilation.plugin('html-webpack-plugin-before-html', (callback: Callback) => { + addAllAssetsToCompilation(this.assets, compilation, htmlPluginData, callback); + }); + }); + } +} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +export default class AddAssetHtmlPlugin { + apply(compiler: WebpackCompilerType) { + compiler.plugin("compilation", (compilation: WebpackCompilationType) => { + compilation.plugin( + "html-webpack-plugin-before-html", + (callback: Callback) => { + addAllAssetsToCompilation( + this.assets, + compilation, + htmlPluginData, + callback + ); + } + ); + }); + } +} + +`; + exports[`break-parent.js 1`] = ` ({ processors: [ diff --git a/tests/last_argument_expansion/break.js b/tests/last_argument_expansion/break.js new file mode 100644 index 000000000..cc736b0c3 --- /dev/null +++ b/tests/last_argument_expansion/break.js @@ -0,0 +1,9 @@ +export default class AddAssetHtmlPlugin { + apply(compiler: WebpackCompilerType) { + compiler.plugin('compilation', (compilation: WebpackCompilationType) => { + compilation.plugin('html-webpack-plugin-before-html', (callback: Callback) => { + addAllAssetsToCompilation(this.assets, compilation, htmlPluginData, callback); + }); + }); + } +}