From 5cc21c5498b758656cad3652d32329e021d11c97 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Mon, 20 Feb 2017 17:25:30 -0800 Subject: [PATCH] Correctly place trailing comments in conditionals (#754) Previously we would blindly put leading comments of the next expression, but we didn't account for a real trailing comment. By checking if the leadingNode is on the same line, we can correctly put it there when needed Fixes #685 --- src/comments.js | 12 ++++++++++-- tests/conditional/__snapshots__/jsfmt.spec.js.snap | 13 ++++++++++--- tests/conditional/comments.js | 4 ++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/comments.js b/src/comments.js index c630afa3..4897381e 100644 --- a/src/comments.js +++ b/src/comments.js @@ -161,8 +161,10 @@ function attach(comments, ast, text) { if ( handleConditionalExpressionComments( enclosingNode, + precedingNode, followingNode, - comment + comment, + text ) ) { // We're good @@ -382,10 +384,16 @@ function handleMemberExpressionComment(enclosingNode, followingNode, comment) { function handleConditionalExpressionComments( enclosingNode, + precedingNode, followingNode, - comment + comment, + text ) { + const isSameLineAsPrecedingNode = precedingNode && + !util.hasNewlineInRange(text, locEnd(precedingNode), locStart(comment)); + if ( + (!precedingNode || !isSameLineAsPrecedingNode) && enclosingNode && enclosingNode.type === "ConditionalExpression" && followingNode diff --git a/tests/conditional/__snapshots__/jsfmt.spec.js.snap b/tests/conditional/__snapshots__/jsfmt.spec.js.snap index 688b1095..3885c2a7 100644 --- a/tests/conditional/__snapshots__/jsfmt.spec.js.snap +++ b/tests/conditional/__snapshots__/jsfmt.spec.js.snap @@ -32,6 +32,10 @@ const extractTextPluginOptions = shouldUseRelativeAssetPaths const extractTextPluginOptions = shouldUseRelativeAssetPaths // Making sure that the publicPath goes back to to build folder. ? { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } : {}; + +const { configureStore } = process.env.NODE_ENV === \"production\" + ? require(\"./configureProdStore\") // a + : require(\"./configureDevStore\"); // b ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ var inspect = 4 === util.inspect.length ? // node <= 0.8.x @@ -63,9 +67,12 @@ const extractTextPluginOptions = shouldUseRelativeAssetPaths { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } : {}; -const extractTextPluginOptions = shouldUseRelativeAssetPaths - ? // Making sure that the publicPath goes back to to build folder. - { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } +const extractTextPluginOptions = shouldUseRelativeAssetPaths // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } : {}; + +const { configureStore } = process.env.NODE_ENV === \"production\" + ? require(\"./configureProdStore\") // a + : require(\"./configureDevStore\"); // b " `; diff --git a/tests/conditional/comments.js b/tests/conditional/comments.js index 78e9a024..e353b39f 100644 --- a/tests/conditional/comments.js +++ b/tests/conditional/comments.js @@ -31,3 +31,7 @@ const extractTextPluginOptions = shouldUseRelativeAssetPaths const extractTextPluginOptions = shouldUseRelativeAssetPaths // Making sure that the publicPath goes back to to build folder. ? { publicPath: Array(cssFilename.split("/").length).join("../") } : {}; + +const { configureStore } = process.env.NODE_ENV === "production" + ? require("./configureProdStore") // a + : require("./configureDevStore"); // b