Skip to content

Commit

Permalink
Avoid breaking arguments for last arg expansion (#1305)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vjeux committed Apr 18, 2017
1 parent 5995af2 commit 8d03423
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 16 deletions.
32 changes: 16 additions & 16 deletions src/printer.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
31 changes: 31 additions & 0 deletions tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -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<any>) => {
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<any>) => {
addAllAssetsToCompilation(
this.assets,
compilation,
htmlPluginData,
callback
);
}
);
});
}
}
`;
exports[`break-parent.js 1`] = `
({
processors: [
Expand Down
9 changes: 9 additions & 0 deletions 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<any>) => {
addAllAssetsToCompilation(this.assets, compilation, htmlPluginData, callback);
});
});
}
}

0 comments on commit 8d03423

Please sign in to comment.