From c5a7febe3cdbf74e83941d772e74ff7b23d2262a Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Mon, 29 Oct 2018 00:19:47 +0800 Subject: [PATCH 01/13] [ImportParserPlugin] when need creating context, should return false instead of true --- lib/dependencies/ImportParserPlugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dependencies/ImportParserPlugin.js b/lib/dependencies/ImportParserPlugin.js index edabf5bb03c..e7b9473b86a 100644 --- a/lib/dependencies/ImportParserPlugin.js +++ b/lib/dependencies/ImportParserPlugin.js @@ -254,7 +254,7 @@ class ImportParserPlugin { dep.loc = expr.loc; dep.optional = !!parser.scope.inTry; parser.state.current.addDependency(dep); - return true; + return false; } }); } From 1d78d914676b47eb9448092fefca8951e649b061 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Mon, 29 Oct 2018 00:21:58 +0800 Subject: [PATCH 02/13] add tests --- .../parsing/issue-8293/dir/file.js | 1 + test/configCases/parsing/issue-8293/index.js | 8 ++++++++ test/configCases/parsing/issue-8293/other.js | 1 + .../parsing/issue-8293/webpack.config.js | 19 +++++++++++++++++++ 4 files changed, 29 insertions(+) create mode 100644 test/configCases/parsing/issue-8293/dir/file.js create mode 100644 test/configCases/parsing/issue-8293/index.js create mode 100644 test/configCases/parsing/issue-8293/other.js create mode 100644 test/configCases/parsing/issue-8293/webpack.config.js diff --git a/test/configCases/parsing/issue-8293/dir/file.js b/test/configCases/parsing/issue-8293/dir/file.js new file mode 100644 index 00000000000..020923cbec7 --- /dev/null +++ b/test/configCases/parsing/issue-8293/dir/file.js @@ -0,0 +1 @@ +export const test = 'test code'; diff --git a/test/configCases/parsing/issue-8293/index.js b/test/configCases/parsing/issue-8293/index.js new file mode 100644 index 00000000000..59be7808e71 --- /dev/null +++ b/test/configCases/parsing/issue-8293/index.js @@ -0,0 +1,8 @@ +const fs = require("fs"); +const path = require("path"); + +it("should be able to replace import param in DefinePlugin", function() { + const source = fs.readFileSync(path.join(__dirname, "bundle1.js"), "utf-8"); + expect(source).toContain(`(\`./\${foobar}.js\`)`); + expect(source).not.toContain(`(\`./\${NAME}.js\`)`); +}); diff --git a/test/configCases/parsing/issue-8293/other.js b/test/configCases/parsing/issue-8293/other.js new file mode 100644 index 00000000000..678213d0a6c --- /dev/null +++ b/test/configCases/parsing/issue-8293/other.js @@ -0,0 +1 @@ +import(`./dir/${NAME}.js`); diff --git a/test/configCases/parsing/issue-8293/webpack.config.js b/test/configCases/parsing/issue-8293/webpack.config.js new file mode 100644 index 00000000000..31a5c2eba97 --- /dev/null +++ b/test/configCases/parsing/issue-8293/webpack.config.js @@ -0,0 +1,19 @@ +const webpack = require("../../../../"); + +module.exports = { + entry: { + bundle0: "./index.js", + bundle1: "./other.js" + }, + output: { + filename: "[name].js" + }, + node: { + __dirname: false + }, + plugins: [ + new webpack.DefinePlugin({ + NAME: "foobar" + }) + ] +}; From d3041bc816ddd3653f5f73ea26bb2dadc86cc618 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 00:33:58 +0800 Subject: [PATCH 03/13] only walk expressions that are not replaced by ContextDependency --- .../AMDDefineDependencyParserPlugin.js | 28 ++++++++++++----- ...AMDRequireDependenciesBlockParserPlugin.js | 31 +++++++++++-------- .../CommonJsRequireDependencyParserPlugin.js | 6 ++-- lib/dependencies/ContextDependencyHelpers.js | 14 ++++++++- lib/dependencies/ImportParserPlugin.js | 4 ++- .../RequireResolveDependencyParserPlugin.js | 16 +++++++--- 6 files changed, 69 insertions(+), 30 deletions(-) diff --git a/lib/dependencies/AMDDefineDependencyParserPlugin.js b/lib/dependencies/AMDDefineDependencyParserPlugin.js index 95e862d1282..7dbbffee90f 100644 --- a/lib/dependencies/AMDDefineDependencyParserPlugin.js +++ b/lib/dependencies/AMDDefineDependencyParserPlugin.js @@ -49,7 +49,7 @@ class AMDDefineDependencyParserPlugin { ); } - processArray(parser, expr, param, identifiers, namedModule) { + processArray(parser, expr, param, paramExpr, identifiers, namedModule) { if (param.isArray()) { param.items.forEach((param, idx) => { if ( @@ -57,9 +57,16 @@ class AMDDefineDependencyParserPlugin { ["require", "module", "exports"].includes(param.string) ) identifiers[idx] = param.string; - const result = this.processItem(parser, expr, param, namedModule); + const subParamExpr = paramExpr.elements[idx]; + const result = this.processItem( + parser, + expr, + param, + subParamExpr, + namedModule + ); if (result === undefined) { - this.processContext(parser, expr, param); + this.processContext(parser, expr, param, subParamExpr); } }); return true; @@ -98,12 +105,14 @@ class AMDDefineDependencyParserPlugin { return true; } } - processItem(parser, expr, param, namedModule) { + processItem(parser, expr, param, paramExpr, namedModule) { if (param.isConditional()) { - param.options.forEach(param => { - const result = this.processItem(parser, expr, param); + param.options.forEach((param, idx) => { + const subParamExpr = + idx === 0 ? paramExpr.consequent : paramExpr.alternate; + const result = this.processItem(parser, expr, param, subParamExpr); if (result === undefined) { - this.processContext(parser, expr, param); + this.processContext(parser, expr, param, subParamExpr); } }); return true; @@ -130,12 +139,14 @@ class AMDDefineDependencyParserPlugin { return true; } } - processContext(parser, expr, param) { + processContext(parser, expr, param, paramExpr) { const dep = ContextDependencyHelpers.create( AMDRequireContextDependency, param.range, param, + paramExpr, expr, + parser, this.options ); if (!dep) return; @@ -231,6 +242,7 @@ class AMDDefineDependencyParserPlugin { parser, expr, param, + array, identifiers, namedModule ); diff --git a/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js b/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js index e6826722e75..40e87a26efb 100644 --- a/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js +++ b/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js @@ -56,14 +56,15 @@ class AMDRequireDependenciesBlockParserPlugin { ); } - processArray(parser, expr, param) { + processArray(parser, expr, param, paramExpr) { if (param.isArray()) { - for (const p of param.items) { - const result = this.processItem(parser, expr, p); + param.items.forEach((p, idx) => { + const subParamExpr = paramExpr.elements[idx]; + const result = this.processItem(parser, expr, p, subParamExpr); if (result === undefined) { - this.processContext(parser, expr, p); + this.processContext(parser, expr, p, subParamExpr); } - } + }); return true; } else if (param.isConstArray()) { const deps = []; @@ -97,14 +98,16 @@ class AMDRequireDependenciesBlockParserPlugin { return true; } } - processItem(parser, expr, param) { + processItem(parser, expr, param, paramExpr) { if (param.isConditional()) { - for (const p of param.options) { - const result = this.processItem(parser, expr, p); + param.options.forEach((p, idx) => { + const subParamExpr = + idx === 0 ? paramExpr.consequent : paramExpr.alternate; + const result = this.processItem(parser, expr, p, subParamExpr); if (result === undefined) { - this.processContext(parser, expr, p); + this.processContext(parser, expr, p, subParamExpr); } - } + }); return true; } else if (param.isString()) { let dep, localModule; @@ -136,12 +139,14 @@ class AMDRequireDependenciesBlockParserPlugin { return true; } } - processContext(parser, expr, param) { + processContext(parser, expr, param, paramExpr) { const dep = ContextDependencyHelpers.create( AMDRequireContextDependency, param.range, param, + paramExpr, expr, + parser, this.options ); if (!dep) return; @@ -196,7 +201,7 @@ class AMDRequireDependenciesBlockParserPlugin { if (expr.arguments.length === 1) { parser.inScope([], () => { - result = this.processArray(parser, expr, param); + result = this.processArray(parser, expr, param, expr.arguments[0]); }); parser.state.current = old; if (!result) return; @@ -207,7 +212,7 @@ class AMDRequireDependenciesBlockParserPlugin { if (expr.arguments.length === 2 || expr.arguments.length === 3) { try { parser.inScope([], () => { - result = this.processArray(parser, expr, param); + result = this.processArray(parser, expr, param, expr.arguments[0]); }); if (!result) { dep = new UnsupportedDependency("unsupported", expr.range); diff --git a/lib/dependencies/CommonJsRequireDependencyParserPlugin.js b/lib/dependencies/CommonJsRequireDependencyParserPlugin.js index 875657348a7..8d528dbdbfd 100644 --- a/lib/dependencies/CommonJsRequireDependencyParserPlugin.js +++ b/lib/dependencies/CommonJsRequireDependencyParserPlugin.js @@ -29,12 +29,14 @@ class CommonJsRequireDependencyParserPlugin { return true; } }; - const processContext = (expr, param) => { + const processContext = (expr, param, paramExpr) => { const dep = ContextDependencyHelpers.create( CommonJsRequireContextDependency, expr.range, param, + paramExpr, expr, + parser, options ); if (!dep) return; @@ -110,7 +112,7 @@ class CommonJsRequireDependencyParserPlugin { } else { const result = processItem(expr, param); if (result === undefined) { - processContext(expr, param); + processContext(expr, param, expr.arguments[0]); } else { const dep = new RequireHeaderDependency(expr.callee.range); dep.loc = expr.loc; diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index 2dabc3345a5..72b4115a6b1 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -45,7 +45,9 @@ ContextDependencyHelpers.create = ( Dep, range, param, + paramExpr, expr, + parser, options, contextOptions ) => { @@ -84,7 +86,17 @@ ContextDependencyHelpers.create = ( ); dep.loc = expr.loc; const replaces = []; - if (prefixRange && prefix !== prefixRaw) { + const templateLiteral = + paramExpr.type === "TaggedTemplateExpression" + ? paramExpr.quasi + : paramExpr; + templateLiteral.expressions.forEach(e => { + // expressions in postfix are needed to be walked + if (e.range[0] > prefixRange[1]) { + parser.walkExpression(e); + } + }); + if (prefix !== prefixRaw) { replaces.push({ range: prefixRange, value: prefix diff --git a/lib/dependencies/ImportParserPlugin.js b/lib/dependencies/ImportParserPlugin.js index e7b9473b86a..9ce8013a8a5 100644 --- a/lib/dependencies/ImportParserPlugin.js +++ b/lib/dependencies/ImportParserPlugin.js @@ -237,7 +237,9 @@ class ImportParserPlugin { ImportContextDependency, expr.range, param, + expr.arguments[0], expr, + parser, this.options, { chunkName, @@ -254,7 +256,7 @@ class ImportParserPlugin { dep.loc = expr.loc; dep.optional = !!parser.scope.inTry; parser.state.current.addDependency(dep); - return false; + return true; } }); } diff --git a/lib/dependencies/RequireResolveDependencyParserPlugin.js b/lib/dependencies/RequireResolveDependencyParserPlugin.js index 4c51ddf6a49..a325e29a66a 100644 --- a/lib/dependencies/RequireResolveDependencyParserPlugin.js +++ b/lib/dependencies/RequireResolveDependencyParserPlugin.js @@ -21,12 +21,16 @@ class RequireResolveDependencyParserPlugin { if (expr.arguments.length !== 1) return; const param = parser.evaluateExpression(expr.arguments[0]); if (param.isConditional()) { - for (const option of param.options) { + param.options.forEach((option, idx) => { + const subParamExpr = + idx === 0 + ? expr.arguments[0].consequent + : expr.arguments[0].alternate; const result = processItem(expr, option, weak); if (result === undefined) { - processContext(expr, option, weak); + processContext(expr, option, subParamExpr, weak); } - } + }); const dep = new RequireResolveHeaderDependency(expr.callee.range); dep.loc = expr.loc; parser.state.current.addDependency(dep); @@ -34,7 +38,7 @@ class RequireResolveDependencyParserPlugin { } else { const result = processItem(expr, param, weak); if (result === undefined) { - processContext(expr, param, weak); + processContext(expr, param, expr.arguments[0], weak); } const dep = new RequireResolveHeaderDependency(expr.callee.range); dep.loc = expr.loc; @@ -52,12 +56,14 @@ class RequireResolveDependencyParserPlugin { return true; } }; - const processContext = (expr, param, weak) => { + const processContext = (expr, param, paramExpr, weak) => { const dep = ContextDependencyHelpers.create( RequireResolveContextDependency, param.range, param, + paramExpr, expr, + parser, options, { mode: weak ? "weak" : "sync" From 0cbb36f38fb6bf1fc63eaeefd8c21e63ec132fcc Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 00:34:12 +0800 Subject: [PATCH 04/13] enhance tests --- test/configCases/parsing/issue-8293/index.js | 6 ++++-- test/configCases/parsing/issue-8293/other.js | 2 +- .../issue-8293/{dir/file.js => prefix/folder/suffix.js} | 0 test/configCases/parsing/issue-8293/webpack.config.js | 4 +++- 4 files changed, 8 insertions(+), 4 deletions(-) rename test/configCases/parsing/issue-8293/{dir/file.js => prefix/folder/suffix.js} (100%) diff --git a/test/configCases/parsing/issue-8293/index.js b/test/configCases/parsing/issue-8293/index.js index 59be7808e71..7e29c86cae5 100644 --- a/test/configCases/parsing/issue-8293/index.js +++ b/test/configCases/parsing/issue-8293/index.js @@ -3,6 +3,8 @@ const path = require("path"); it("should be able to replace import param in DefinePlugin", function() { const source = fs.readFileSync(path.join(__dirname, "bundle1.js"), "utf-8"); - expect(source).toContain(`(\`./\${foobar}.js\`)`); - expect(source).not.toContain(`(\`./\${NAME}.js\`)`); + expect(source).toContain(`(\`./\${foobar}/\${"suffix"}\`)`); + expect(source).not.toContain( + `(\`./\${DEFINED_EXPRESSION}/\${CONST_SUFFIX}\`)` + ); }); diff --git a/test/configCases/parsing/issue-8293/other.js b/test/configCases/parsing/issue-8293/other.js index 678213d0a6c..5888f41d7c7 100644 --- a/test/configCases/parsing/issue-8293/other.js +++ b/test/configCases/parsing/issue-8293/other.js @@ -1 +1 @@ -import(`./dir/${NAME}.js`); +import(`./${CONST_PREFIX}/${DEFINED_EXPRESSION}/${CONST_SUFFIX}`); diff --git a/test/configCases/parsing/issue-8293/dir/file.js b/test/configCases/parsing/issue-8293/prefix/folder/suffix.js similarity index 100% rename from test/configCases/parsing/issue-8293/dir/file.js rename to test/configCases/parsing/issue-8293/prefix/folder/suffix.js diff --git a/test/configCases/parsing/issue-8293/webpack.config.js b/test/configCases/parsing/issue-8293/webpack.config.js index 31a5c2eba97..3f29a7135e7 100644 --- a/test/configCases/parsing/issue-8293/webpack.config.js +++ b/test/configCases/parsing/issue-8293/webpack.config.js @@ -13,7 +13,9 @@ module.exports = { }, plugins: [ new webpack.DefinePlugin({ - NAME: "foobar" + CONST_PREFIX: JSON.stringify("prefix"), + CONST_SUFFIX: JSON.stringify("suffix"), + DEFINED_EXPRESSION: "foobar" }) ] }; From 16ff8009850eb14d5db831c85dd0fc4b9c24b88f Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 16:22:09 +0800 Subject: [PATCH 05/13] handle conditional evaluated to template string --- lib/dependencies/ContextDependencyHelpers.js | 24 ++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index 72b4115a6b1..da19dcb9b8b 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -4,6 +4,8 @@ */ "use strict"; +const ConstDependency = require("./ConstDependency"); + const ContextDependencyHelpers = exports; /** @@ -86,10 +88,24 @@ ContextDependencyHelpers.create = ( ); dep.loc = expr.loc; const replaces = []; - const templateLiteral = - paramExpr.type === "TaggedTemplateExpression" - ? paramExpr.quasi - : paramExpr; + let templateLiteral = paramExpr; + if (paramExpr.type === "ConditionalExpression") { + const test = parser.evaluateExpression(paramExpr.test); + let replace; + if (test.asBool()) { + templateLiteral = paramExpr.consequent; + replace = paramExpr.alternate; + } else { + templateLiteral = paramExpr.alternate; + replace = paramExpr.consequent; + } + const dep = new ConstDependency(JSON.stringify(""), replace.range); + dep.loc = replace.loc; + parser.state.current.addDependency(dep); + } + if (templateLiteral.type === "TaggedTemplateExpression") { + templateLiteral = templateLiteral.quasi; + } templateLiteral.expressions.forEach(e => { // expressions in postfix are needed to be walked if (e.range[0] > prefixRange[1]) { From 84f92abcb95da9e7b3fa3545533ed8b2dd74646d Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 16:22:31 +0800 Subject: [PATCH 06/13] add tests --- .../parsing/issue-8293/amd-define.js | 9 ++++++++ .../parsing/issue-8293/amd-require.js | 9 ++++++++ .../parsing/issue-8293/commonjs.js | 7 ++++++ test/configCases/parsing/issue-8293/import.js | 7 ++++++ test/configCases/parsing/issue-8293/index.js | 23 +++++++++++++------ test/configCases/parsing/issue-8293/other.js | 1 - .../issue-8293/prefix/folder/suffix.js | 1 - .../issue-8293/prefix0/folder/suffix0.js | 1 + .../issue-8293/prefix1/folder/suffix1.js | 1 + .../issue-8293/prefix2/folder/suffix2.js | 1 + .../issue-8293/prefix3/folder/suffix3.js | 1 + .../issue-8293/prefix4/folder/suffix4.js | 1 + .../parsing/issue-8293/require.resolve.js | 11 +++++++++ .../parsing/issue-8293/webpack.config.js | 21 ++++++++++++++--- 14 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 test/configCases/parsing/issue-8293/amd-define.js create mode 100644 test/configCases/parsing/issue-8293/amd-require.js create mode 100644 test/configCases/parsing/issue-8293/commonjs.js create mode 100644 test/configCases/parsing/issue-8293/import.js delete mode 100644 test/configCases/parsing/issue-8293/other.js delete mode 100644 test/configCases/parsing/issue-8293/prefix/folder/suffix.js create mode 100644 test/configCases/parsing/issue-8293/prefix0/folder/suffix0.js create mode 100644 test/configCases/parsing/issue-8293/prefix1/folder/suffix1.js create mode 100644 test/configCases/parsing/issue-8293/prefix2/folder/suffix2.js create mode 100644 test/configCases/parsing/issue-8293/prefix3/folder/suffix3.js create mode 100644 test/configCases/parsing/issue-8293/prefix4/folder/suffix4.js create mode 100644 test/configCases/parsing/issue-8293/require.resolve.js diff --git a/test/configCases/parsing/issue-8293/amd-define.js b/test/configCases/parsing/issue-8293/amd-define.js new file mode 100644 index 00000000000..d499d97165b --- /dev/null +++ b/test/configCases/parsing/issue-8293/amd-define.js @@ -0,0 +1,9 @@ +define([ + `./${CONST_PREFIX0}/${DEFINED_EXPRESSION}/${CONST_SUFFIX0}`, + window.baz + ? `./${CONST_PREFIX1}/${DEFINED_EXPRESSION}/${CONST_SUFFIX1}` + : `./${CONST_PREFIX2}/${DEFINED_EXPRESSION}/${CONST_SUFFIX2}`, + typeof require === "function" + ? `./${CONST_PREFIX3}/${DEFINED_EXPRESSION}/${CONST_SUFFIX3}` + : `./${CONST_PREFIX4}/${DEFINED_EXPRESSION}/${CONST_SUFFIX4}` +], () => {}); diff --git a/test/configCases/parsing/issue-8293/amd-require.js b/test/configCases/parsing/issue-8293/amd-require.js new file mode 100644 index 00000000000..d3007c3c1e8 --- /dev/null +++ b/test/configCases/parsing/issue-8293/amd-require.js @@ -0,0 +1,9 @@ +require([ + `./${CONST_PREFIX0}/${DEFINED_EXPRESSION}/${CONST_SUFFIX0}`, + window.baz + ? `./${CONST_PREFIX1}/${DEFINED_EXPRESSION}/${CONST_SUFFIX1}` + : `./${CONST_PREFIX2}/${DEFINED_EXPRESSION}/${CONST_SUFFIX2}`, + typeof require === "function" + ? `./${CONST_PREFIX3}/${DEFINED_EXPRESSION}/${CONST_SUFFIX3}` + : `./${CONST_PREFIX4}/${DEFINED_EXPRESSION}/${CONST_SUFFIX4}` +], () => {}); diff --git a/test/configCases/parsing/issue-8293/commonjs.js b/test/configCases/parsing/issue-8293/commonjs.js new file mode 100644 index 00000000000..4e0cbe38ecd --- /dev/null +++ b/test/configCases/parsing/issue-8293/commonjs.js @@ -0,0 +1,7 @@ +require(`./${CONST_PREFIX0}/${DEFINED_EXPRESSION}/${CONST_SUFFIX0}`); +require(window.baz + ? `./${CONST_PREFIX1}/${DEFINED_EXPRESSION}/${CONST_SUFFIX1}` + : `./${CONST_PREFIX2}/${DEFINED_EXPRESSION}/${CONST_SUFFIX2}`); +require(typeof require === "function" + ? `./${CONST_PREFIX3}/${DEFINED_EXPRESSION}/${CONST_SUFFIX3}` + : `./${CONST_PREFIX4}/${DEFINED_EXPRESSION}/${CONST_SUFFIX4}`); diff --git a/test/configCases/parsing/issue-8293/import.js b/test/configCases/parsing/issue-8293/import.js new file mode 100644 index 00000000000..632a98aff33 --- /dev/null +++ b/test/configCases/parsing/issue-8293/import.js @@ -0,0 +1,7 @@ +import(`./${CONST_PREFIX0}/${DEFINED_EXPRESSION}/${CONST_SUFFIX0}`); +import(window.baz + ? `./${CONST_PREFIX1}/${DEFINED_EXPRESSION}/${CONST_SUFFIX1}` + : `./${CONST_PREFIX2}/${DEFINED_EXPRESSION}/${CONST_SUFFIX2}`); +import(typeof require === "function" + ? `./${CONST_PREFIX3}/${DEFINED_EXPRESSION}/${CONST_SUFFIX3}` + : `./${CONST_PREFIX4}/${DEFINED_EXPRESSION}/${CONST_SUFFIX4}`); diff --git a/test/configCases/parsing/issue-8293/index.js b/test/configCases/parsing/issue-8293/index.js index 7e29c86cae5..f61da630567 100644 --- a/test/configCases/parsing/issue-8293/index.js +++ b/test/configCases/parsing/issue-8293/index.js @@ -1,10 +1,19 @@ const fs = require("fs"); const path = require("path"); -it("should be able to replace import param in DefinePlugin", function() { - const source = fs.readFileSync(path.join(__dirname, "bundle1.js"), "utf-8"); - expect(source).toContain(`(\`./\${foobar}/\${"suffix"}\`)`); - expect(source).not.toContain( - `(\`./\${DEFINED_EXPRESSION}/\${CONST_SUFFIX}\`)` - ); -}); +["import", "amd-require", "amd-define", "commonjs", "require.resolve"].forEach( + method => { + it(`should be able to replace ${method} param in DefinePlugin`, function() { + const source = fs.readFileSync( + path.join(__dirname, `bundle-${method}.js`), + "utf-8" + ); + expect(source).toContain(`\`./\${foobar}/\${"suffix0"}\``); + // buggy for param.isConditional() cases, big work, resolve in later PRs + // expect(source).toContain(`\`./\${foobar}/\${"suffix1"}\``); + // expect(source).toContain(`\`./\${foobar}/\${"suffix2"}\``); + expect(source).toContain(`\`./\${foobar}/\${"suffix3"}\``); + expect(source).not.toContain(`\`./\${foobar}/\${"suffix4"}\``); + }); + } +); diff --git a/test/configCases/parsing/issue-8293/other.js b/test/configCases/parsing/issue-8293/other.js deleted file mode 100644 index 5888f41d7c7..00000000000 --- a/test/configCases/parsing/issue-8293/other.js +++ /dev/null @@ -1 +0,0 @@ -import(`./${CONST_PREFIX}/${DEFINED_EXPRESSION}/${CONST_SUFFIX}`); diff --git a/test/configCases/parsing/issue-8293/prefix/folder/suffix.js b/test/configCases/parsing/issue-8293/prefix/folder/suffix.js deleted file mode 100644 index 020923cbec7..00000000000 --- a/test/configCases/parsing/issue-8293/prefix/folder/suffix.js +++ /dev/null @@ -1 +0,0 @@ -export const test = 'test code'; diff --git a/test/configCases/parsing/issue-8293/prefix0/folder/suffix0.js b/test/configCases/parsing/issue-8293/prefix0/folder/suffix0.js new file mode 100644 index 00000000000..9004e941be7 --- /dev/null +++ b/test/configCases/parsing/issue-8293/prefix0/folder/suffix0.js @@ -0,0 +1 @@ +export const test = 'test code 0'; diff --git a/test/configCases/parsing/issue-8293/prefix1/folder/suffix1.js b/test/configCases/parsing/issue-8293/prefix1/folder/suffix1.js new file mode 100644 index 00000000000..41a18990840 --- /dev/null +++ b/test/configCases/parsing/issue-8293/prefix1/folder/suffix1.js @@ -0,0 +1 @@ +export const test = 'test code 1'; diff --git a/test/configCases/parsing/issue-8293/prefix2/folder/suffix2.js b/test/configCases/parsing/issue-8293/prefix2/folder/suffix2.js new file mode 100644 index 00000000000..c4c6df02b59 --- /dev/null +++ b/test/configCases/parsing/issue-8293/prefix2/folder/suffix2.js @@ -0,0 +1 @@ +export const test = 'test code 2'; diff --git a/test/configCases/parsing/issue-8293/prefix3/folder/suffix3.js b/test/configCases/parsing/issue-8293/prefix3/folder/suffix3.js new file mode 100644 index 00000000000..454521432be --- /dev/null +++ b/test/configCases/parsing/issue-8293/prefix3/folder/suffix3.js @@ -0,0 +1 @@ +export const test = 'test code 3'; diff --git a/test/configCases/parsing/issue-8293/prefix4/folder/suffix4.js b/test/configCases/parsing/issue-8293/prefix4/folder/suffix4.js new file mode 100644 index 00000000000..130fcfbc881 --- /dev/null +++ b/test/configCases/parsing/issue-8293/prefix4/folder/suffix4.js @@ -0,0 +1 @@ +export const test = 'test code 4'; diff --git a/test/configCases/parsing/issue-8293/require.resolve.js b/test/configCases/parsing/issue-8293/require.resolve.js new file mode 100644 index 00000000000..2940a9535ac --- /dev/null +++ b/test/configCases/parsing/issue-8293/require.resolve.js @@ -0,0 +1,11 @@ +require.resolve(`./${CONST_PREFIX0}/${DEFINED_EXPRESSION}/${CONST_SUFFIX0}`); +require.resolve( + window.baz + ? `./${CONST_PREFIX1}/${DEFINED_EXPRESSION}/${CONST_SUFFIX1}` + : `./${CONST_PREFIX2}/${DEFINED_EXPRESSION}/${CONST_SUFFIX2}` +); +require.resolve( + typeof require === "function" + ? `./${CONST_PREFIX3}/${DEFINED_EXPRESSION}/${CONST_SUFFIX3}` + : `./${CONST_PREFIX4}/${DEFINED_EXPRESSION}/${CONST_SUFFIX4}` +); diff --git a/test/configCases/parsing/issue-8293/webpack.config.js b/test/configCases/parsing/issue-8293/webpack.config.js index 3f29a7135e7..da6af6d2013 100644 --- a/test/configCases/parsing/issue-8293/webpack.config.js +++ b/test/configCases/parsing/issue-8293/webpack.config.js @@ -3,18 +3,33 @@ const webpack = require("../../../../"); module.exports = { entry: { bundle0: "./index.js", - bundle1: "./other.js" + "bundle-import": "./import.js", + "bundle-amd-require": "./amd-define.js", + "bundle-amd-define": "./amd-require.js", + "bundle-commonjs": "./commonjs.js", + "bundle-require.resolve": "./require.resolve.js" }, output: { filename: "[name].js" }, + module: { + exprContextCritical: false + }, node: { __dirname: false }, plugins: [ new webpack.DefinePlugin({ - CONST_PREFIX: JSON.stringify("prefix"), - CONST_SUFFIX: JSON.stringify("suffix"), + CONST_PREFIX0: JSON.stringify("prefix0"), + CONST_SUFFIX0: JSON.stringify("suffix0"), + CONST_PREFIX1: JSON.stringify("prefix1"), + CONST_SUFFIX1: JSON.stringify("suffix1"), + CONST_PREFIX2: JSON.stringify("prefix2"), + CONST_SUFFIX2: JSON.stringify("suffix2"), + CONST_PREFIX3: JSON.stringify("prefix3"), + CONST_SUFFIX3: JSON.stringify("suffix3"), + CONST_PREFIX4: JSON.stringify("prefix4"), + CONST_SUFFIX4: JSON.stringify("suffix4"), DEFINED_EXPRESSION: "foobar" }) ] From 25af83f288cdcc59367be689682c9957e4eb6f44 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 19:39:43 +0800 Subject: [PATCH 07/13] fix after comments --- lib/BasicEvaluatedExpression.js | 10 ++- lib/Parser.js | 66 +++++++++----- .../AMDDefineDependencyParserPlugin.js | 32 +++---- ...AMDRequireDependenciesBlockParserPlugin.js | 35 ++++---- .../CommonJsRequireDependencyParserPlugin.js | 10 +-- lib/dependencies/ContextDependencyHelpers.js | 86 ++++++++++++------- lib/dependencies/ImportParserPlugin.js | 5 +- .../RequireResolveDependencyParserPlugin.js | 19 ++-- 8 files changed, 146 insertions(+), 117 deletions(-) diff --git a/lib/BasicEvaluatedExpression.js b/lib/BasicEvaluatedExpression.js index 65db864f9b0..280590e182a 100644 --- a/lib/BasicEvaluatedExpression.js +++ b/lib/BasicEvaluatedExpression.js @@ -29,11 +29,13 @@ class BasicEvaluatedExpression { this.regExp = null; this.string = null; this.quasis = null; + this.parts = null; this.array = null; this.items = null; this.options = null; this.prefix = null; this.postfix = null; + this.expression = null; } isNull() { @@ -184,9 +186,10 @@ class BasicEvaluatedExpression { return this; } - setTemplateString(quasis) { + setTemplateString(quasis, parts) { this.type = TypeTemplateString; this.quasis = quasis; + this.parts = parts; return this; } @@ -206,6 +209,11 @@ class BasicEvaluatedExpression { this.range = range; return this; } + + setExpression(expression) { + this.expression = expression; + return this; + } } module.exports = BasicEvaluatedExpression; diff --git a/lib/Parser.js b/lib/Parser.js index 8be3bea88ab..1c4f4c0f9f8 100644 --- a/lib/Parser.js +++ b/lib/Parser.js @@ -602,24 +602,36 @@ class Parser extends Tapable { /** * @param {string} kind "cooked" | "raw" - * @param {TODO[]} quasis quasis - * @param {TODO[]} expressions expressions + * @param {TemplateLiteral} templateLiteralExpr TemplateLiteral expr * @returns {BasicEvaluatedExpression[]} Simplified template */ - const getSimplifiedTemplateResult = (kind, quasis, expressions) => { + const getSimplifiedTemplateResult = (kind, templateLiteralExpr) => { + const quasis = []; const parts = []; - for (let i = 0; i < quasis.length; i++) { + for (let i = 0; i < templateLiteralExpr.quasis.length; i++) { + const quasiExpr = templateLiteralExpr.quasis[i]; + quasis.push( + // expression is meaningless here + // as it may correspond to multiple expressions + new BasicEvaluatedExpression() + .setString(quasiExpr.value[kind]) + .setRange(quasiExpr.range) + ); parts.push( new BasicEvaluatedExpression() - .setString(quasis[i].value[kind]) - .setRange(quasis[i].range) + .setString(quasiExpr.value[kind]) + .setRange(quasiExpr.range) + .setExpression(quasiExpr) ); if (i > 0) { - const prevExpr = parts[parts.length - 2], - lastExpr = parts[parts.length - 1]; - const expr = this.evaluateExpression(expressions[i - 1]); + const prevExpr = quasis[quasis.length - 2], + lastExpr = quasis[quasis.length - 1]; + const expr = this.evaluateExpression( + templateLiteralExpr.expressions[i - 1] + ); + parts.push(expr); if (!(expr.isString() || expr.isNumber())) continue; prevExpr.setString( @@ -628,36 +640,37 @@ class Parser extends Tapable { lastExpr.string ); prevExpr.setRange([prevExpr.range[0], lastExpr.range[1]]); - parts.pop(); + quasis.pop(); } } - return parts; + return { + quasis, + parts + }; }; this.hooks.evaluate.for("TemplateLiteral").tap("Parser", node => { - const parts = getSimplifiedTemplateResult.call( + const { quasis, parts } = getSimplifiedTemplateResult.call( this, "cooked", - node.quasis, - node.expressions + node ); - if (parts.length === 1) { - return parts[0].setRange(node.range); + if (quasis.length === 1) { + return quasis[0].setRange(node.range); } return new BasicEvaluatedExpression() - .setTemplateString(parts) + .setTemplateString(quasis, parts) .setRange(node.range); }); this.hooks.evaluate.for("TaggedTemplateExpression").tap("Parser", node => { if (this.evaluateExpression(node.tag).identifier !== "String.raw") return; - const parts = getSimplifiedTemplateResult.call( + const { quasis, parts } = getSimplifiedTemplateResult.call( this, "raw", - node.quasi.quasis, - node.quasi.expressions + node.quasi ); return new BasicEvaluatedExpression() - .setTemplateString(parts) + .setTemplateString(quasis, parts) .setRange(node.range); }); @@ -1926,13 +1939,20 @@ class Parser extends Tapable { const hook = this.hooks.evaluate.get(expression.type); if (hook !== undefined) { const result = hook.call(expression); - if (result !== undefined) return result; + if (result !== undefined) { + if (result) { + result.setExpression(expression); + } + return result; + } } } catch (e) { console.warn(e); // ignore error } - return new BasicEvaluatedExpression().setRange(expression.range); + return new BasicEvaluatedExpression() + .setRange(expression.range) + .setExpression(expression); } parseString(expression) { diff --git a/lib/dependencies/AMDDefineDependencyParserPlugin.js b/lib/dependencies/AMDDefineDependencyParserPlugin.js index 7dbbffee90f..99a1528a55b 100644 --- a/lib/dependencies/AMDDefineDependencyParserPlugin.js +++ b/lib/dependencies/AMDDefineDependencyParserPlugin.js @@ -49,7 +49,7 @@ class AMDDefineDependencyParserPlugin { ); } - processArray(parser, expr, param, paramExpr, identifiers, namedModule) { + processArray(parser, expr, param, identifiers, namedModule) { if (param.isArray()) { param.items.forEach((param, idx) => { if ( @@ -57,16 +57,9 @@ class AMDDefineDependencyParserPlugin { ["require", "module", "exports"].includes(param.string) ) identifiers[idx] = param.string; - const subParamExpr = paramExpr.elements[idx]; - const result = this.processItem( - parser, - expr, - param, - subParamExpr, - namedModule - ); + const result = this.processItem(parser, expr, param, namedModule); if (result === undefined) { - this.processContext(parser, expr, param, subParamExpr); + this.processContext(parser, expr, param); } }); return true; @@ -105,14 +98,12 @@ class AMDDefineDependencyParserPlugin { return true; } } - processItem(parser, expr, param, paramExpr, namedModule) { + processItem(parser, expr, param, namedModule) { if (param.isConditional()) { - param.options.forEach((param, idx) => { - const subParamExpr = - idx === 0 ? paramExpr.consequent : paramExpr.alternate; - const result = this.processItem(parser, expr, param, subParamExpr); + param.options.forEach(param => { + const result = this.processItem(parser, expr, param); if (result === undefined) { - this.processContext(parser, expr, param, subParamExpr); + this.processContext(parser, expr, param); } }); return true; @@ -139,15 +130,15 @@ class AMDDefineDependencyParserPlugin { return true; } } - processContext(parser, expr, param, paramExpr) { + processContext(parser, expr, param) { const dep = ContextDependencyHelpers.create( AMDRequireContextDependency, param.range, param, - paramExpr, expr, - parser, - this.options + this.options, + {}, + parser ); if (!dep) return; dep.loc = expr.loc; @@ -242,7 +233,6 @@ class AMDDefineDependencyParserPlugin { parser, expr, param, - array, identifiers, namedModule ); diff --git a/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js b/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js index 40e87a26efb..ea52d819ad5 100644 --- a/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js +++ b/lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js @@ -56,15 +56,14 @@ class AMDRequireDependenciesBlockParserPlugin { ); } - processArray(parser, expr, param, paramExpr) { + processArray(parser, expr, param) { if (param.isArray()) { - param.items.forEach((p, idx) => { - const subParamExpr = paramExpr.elements[idx]; - const result = this.processItem(parser, expr, p, subParamExpr); + for (const p of param.items) { + const result = this.processItem(parser, expr, p); if (result === undefined) { - this.processContext(parser, expr, p, subParamExpr); + this.processContext(parser, expr, p); } - }); + } return true; } else if (param.isConstArray()) { const deps = []; @@ -98,16 +97,14 @@ class AMDRequireDependenciesBlockParserPlugin { return true; } } - processItem(parser, expr, param, paramExpr) { + processItem(parser, expr, param) { if (param.isConditional()) { - param.options.forEach((p, idx) => { - const subParamExpr = - idx === 0 ? paramExpr.consequent : paramExpr.alternate; - const result = this.processItem(parser, expr, p, subParamExpr); + for (const p of param.options) { + const result = this.processItem(parser, expr, p); if (result === undefined) { - this.processContext(parser, expr, p, subParamExpr); + this.processContext(parser, expr, p); } - }); + } return true; } else if (param.isString()) { let dep, localModule; @@ -139,15 +136,15 @@ class AMDRequireDependenciesBlockParserPlugin { return true; } } - processContext(parser, expr, param, paramExpr) { + processContext(parser, expr, param) { const dep = ContextDependencyHelpers.create( AMDRequireContextDependency, param.range, param, - paramExpr, expr, - parser, - this.options + this.options, + {}, + parser ); if (!dep) return; dep.loc = expr.loc; @@ -201,7 +198,7 @@ class AMDRequireDependenciesBlockParserPlugin { if (expr.arguments.length === 1) { parser.inScope([], () => { - result = this.processArray(parser, expr, param, expr.arguments[0]); + result = this.processArray(parser, expr, param); }); parser.state.current = old; if (!result) return; @@ -212,7 +209,7 @@ class AMDRequireDependenciesBlockParserPlugin { if (expr.arguments.length === 2 || expr.arguments.length === 3) { try { parser.inScope([], () => { - result = this.processArray(parser, expr, param, expr.arguments[0]); + result = this.processArray(parser, expr, param); }); if (!result) { dep = new UnsupportedDependency("unsupported", expr.range); diff --git a/lib/dependencies/CommonJsRequireDependencyParserPlugin.js b/lib/dependencies/CommonJsRequireDependencyParserPlugin.js index 8d528dbdbfd..b0faa1a338d 100644 --- a/lib/dependencies/CommonJsRequireDependencyParserPlugin.js +++ b/lib/dependencies/CommonJsRequireDependencyParserPlugin.js @@ -29,15 +29,15 @@ class CommonJsRequireDependencyParserPlugin { return true; } }; - const processContext = (expr, param, paramExpr) => { + const processContext = (expr, param) => { const dep = ContextDependencyHelpers.create( CommonJsRequireContextDependency, expr.range, param, - paramExpr, expr, - parser, - options + options, + {}, + parser ); if (!dep) return; dep.loc = expr.loc; @@ -112,7 +112,7 @@ class CommonJsRequireDependencyParserPlugin { } else { const result = processItem(expr, param); if (result === undefined) { - processContext(expr, param, expr.arguments[0]); + processContext(expr, param); } else { const dep = new RequireHeaderDependency(expr.callee.range); dep.loc = expr.loc; diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index da19dcb9b8b..a95c370b828 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -4,8 +4,6 @@ */ "use strict"; -const ConstDependency = require("./ConstDependency"); - const ContextDependencyHelpers = exports; /** @@ -47,11 +45,11 @@ ContextDependencyHelpers.create = ( Dep, range, param, - paramExpr, expr, - parser, options, - contextOptions + contextOptions, + // when parser is not passed in, expressions won't be walked + parser = null ) => { if (param.isTemplateString()) { let prefixRaw = param.quasis[0].string; @@ -88,37 +86,54 @@ ContextDependencyHelpers.create = ( ); dep.loc = expr.loc; const replaces = []; - let templateLiteral = paramExpr; - if (paramExpr.type === "ConditionalExpression") { - const test = parser.evaluateExpression(paramExpr.test); - let replace; - if (test.asBool()) { - templateLiteral = paramExpr.consequent; - replace = paramExpr.alternate; - } else { - templateLiteral = paramExpr.alternate; - replace = paramExpr.consequent; - } - const dep = new ConstDependency(JSON.stringify(""), replace.range); - dep.loc = replace.loc; - parser.state.current.addDependency(dep); - } - if (templateLiteral.type === "TaggedTemplateExpression") { - templateLiteral = templateLiteral.quasi; - } - templateLiteral.expressions.forEach(e => { - // expressions in postfix are needed to be walked - if (e.range[0] > prefixRange[1]) { - parser.walkExpression(e); + + if (parser) { + let paramExpr = param.expression; + while (paramExpr.type !== "TemplateLiteral") { + if (paramExpr.type === "ConditionalExpression") { + const test = parser.evaluateExpression(paramExpr.test); + let replace; + if (test.asBool()) { + replace = paramExpr.alternate; + paramExpr = paramExpr.consequent; + } else { + replace = paramExpr.consequent; + paramExpr = paramExpr.alternate; + } + replaces.push({ + range: replace.range, + value: JSON.stringify("") + }); + } else if (paramExpr.type === "TaggedTemplateExpression") { + paramExpr = paramExpr.quasi; + } else { + break; // there are other cases, just ignore for now + } } - }); - if (prefix !== prefixRaw) { - replaces.push({ - range: prefixRange, - value: prefix + param.parts.forEach(part => { + if ( + part.range[0] > prefixRange[1] && + (!postfixRange || part.range[1] < postfixRange[0]) + ) { + if (part.isString()) { + replaces.push({ + range: part.range, + value: part.string + }); + } else { + parser.walkExpression(part.expression); + } + } }); } - if (postfixRange && postfix !== postfixRaw) { + + // replace even prefix === prefixRaw as it's evaluated + replaces.push({ + range: prefixRange, + value: prefix + }); + // replace even postfix === postfixRaw as it's evaluated + if (postfixRange) { replaces.push({ range: postfixRange, value: postfix @@ -200,6 +215,11 @@ ContextDependencyHelpers.create = ( dep.critical = options.exprContextCritical && "the request of a dependency is an expression"; + + if (parser) { + parser.walkExpression(param.expression); + } + return dep; } }; diff --git a/lib/dependencies/ImportParserPlugin.js b/lib/dependencies/ImportParserPlugin.js index 9ce8013a8a5..db22574f693 100644 --- a/lib/dependencies/ImportParserPlugin.js +++ b/lib/dependencies/ImportParserPlugin.js @@ -237,9 +237,7 @@ class ImportParserPlugin { ImportContextDependency, expr.range, param, - expr.arguments[0], expr, - parser, this.options, { chunkName, @@ -250,7 +248,8 @@ class ImportParserPlugin { namespaceObject: parser.state.module.buildMeta.strictHarmonyModule ? "strict" : true - } + }, + parser ); if (!dep) return; dep.loc = expr.loc; diff --git a/lib/dependencies/RequireResolveDependencyParserPlugin.js b/lib/dependencies/RequireResolveDependencyParserPlugin.js index a325e29a66a..0d356cf5cd0 100644 --- a/lib/dependencies/RequireResolveDependencyParserPlugin.js +++ b/lib/dependencies/RequireResolveDependencyParserPlugin.js @@ -21,16 +21,12 @@ class RequireResolveDependencyParserPlugin { if (expr.arguments.length !== 1) return; const param = parser.evaluateExpression(expr.arguments[0]); if (param.isConditional()) { - param.options.forEach((option, idx) => { - const subParamExpr = - idx === 0 - ? expr.arguments[0].consequent - : expr.arguments[0].alternate; + for (const option of param.options) { const result = processItem(expr, option, weak); if (result === undefined) { - processContext(expr, option, subParamExpr, weak); + processContext(expr, option, weak); } - }); + } const dep = new RequireResolveHeaderDependency(expr.callee.range); dep.loc = expr.loc; parser.state.current.addDependency(dep); @@ -38,7 +34,7 @@ class RequireResolveDependencyParserPlugin { } else { const result = processItem(expr, param, weak); if (result === undefined) { - processContext(expr, param, expr.arguments[0], weak); + processContext(expr, param, weak); } const dep = new RequireResolveHeaderDependency(expr.callee.range); dep.loc = expr.loc; @@ -56,18 +52,17 @@ class RequireResolveDependencyParserPlugin { return true; } }; - const processContext = (expr, param, paramExpr, weak) => { + const processContext = (expr, param, weak) => { const dep = ContextDependencyHelpers.create( RequireResolveContextDependency, param.range, param, - paramExpr, expr, - parser, options, { mode: weak ? "weak" : "sync" - } + }, + parser ); if (!dep) return; dep.loc = expr.loc; From ff3bfa157de69159d64785ce8187708d4579a08a Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 19:45:16 +0800 Subject: [PATCH 08/13] fix tests --- test/cases/parsing/issue-7778/index.js | 2 +- test/configCases/parsing/issue-8293/index.js | 31 ++++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/test/cases/parsing/issue-7778/index.js b/test/cases/parsing/issue-7778/index.js index 0f8dda218e5..2e4ac5dc4c9 100644 --- a/test/cases/parsing/issue-7778/index.js +++ b/test/cases/parsing/issue-7778/index.js @@ -34,7 +34,7 @@ it("should detect query strings in dynamic import as a static value 2", function ]); }); -it("should detect query strings in dynamic import as a static value 2", function() { +it("should detect query strings in dynamic import as a static value 3", function() { var testFileName = "a"; return Promise.all([ diff --git a/test/configCases/parsing/issue-8293/index.js b/test/configCases/parsing/issue-8293/index.js index f61da630567..cc01b6c5646 100644 --- a/test/configCases/parsing/issue-8293/index.js +++ b/test/configCases/parsing/issue-8293/index.js @@ -8,12 +8,31 @@ const path = require("path"); path.join(__dirname, `bundle-${method}.js`), "utf-8" ); - expect(source).toContain(`\`./\${foobar}/\${"suffix0"}\``); - // buggy for param.isConditional() cases, big work, resolve in later PRs - // expect(source).toContain(`\`./\${foobar}/\${"suffix1"}\``); - // expect(source).toContain(`\`./\${foobar}/\${"suffix2"}\``); - expect(source).toContain(`\`./\${foobar}/\${"suffix3"}\``); - expect(source).not.toContain(`\`./\${foobar}/\${"suffix4"}\``); + expect(source).toContain(`\`./\${foobar}/suffix0`); + expect(source).toContain(`\`./\${foobar}/suffix3`); + expect(source).not.toContain(`\`./\${foobar}/suffix4`); }); } ); + +["import", "commonjs"].forEach(method => { + it(`should be able to replace ${method} param in DefinePlugin for conditional expression`, function() { + const source = fs.readFileSync( + path.join(__dirname, `bundle-${method}.js`), + "utf-8" + ); + expect(source).toContain(`\`./\${"prefix1"}/\${foobar}/\${"suffix1"}`); + expect(source).toContain(`\`./\${"prefix2"}/\${foobar}/\${"suffix2"}`); + }); +}); + +["amd-require", "amd-define", "require.resolve"].forEach(method => { + it(`should be able to replace ${method} param in DefinePlugin for conditional expression`, function() { + const source = fs.readFileSync( + path.join(__dirname, `bundle-${method}.js`), + "utf-8" + ); + expect(source).toContain(`\`./\${foobar}/suffix1`); + expect(source).toContain(`\`./\${foobar}/suffix2`); + }); +}); From c740380694a44859da2cf847abe9081a104895c2 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 23:43:44 +0800 Subject: [PATCH 09/13] fix jsdoc --- lib/Parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Parser.js b/lib/Parser.js index 1c4f4c0f9f8..4b75cf91166 100644 --- a/lib/Parser.js +++ b/lib/Parser.js @@ -602,7 +602,7 @@ class Parser extends Tapable { /** * @param {string} kind "cooked" | "raw" - * @param {TemplateLiteral} templateLiteralExpr TemplateLiteral expr + * @param {TODO} templateLiteralExpr TemplateLiteral expr * @returns {BasicEvaluatedExpression[]} Simplified template */ const getSimplifiedTemplateResult = (kind, templateLiteralExpr) => { From 88aab1e3cb11cdfd1a74ca534d9cd7b1228dcd7a Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Tue, 30 Oct 2018 23:50:09 +0800 Subject: [PATCH 10/13] fix jsdoc --- lib/Parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Parser.js b/lib/Parser.js index 4b75cf91166..73cd90e41e3 100644 --- a/lib/Parser.js +++ b/lib/Parser.js @@ -603,7 +603,7 @@ class Parser extends Tapable { /** * @param {string} kind "cooked" | "raw" * @param {TODO} templateLiteralExpr TemplateLiteral expr - * @returns {BasicEvaluatedExpression[]} Simplified template + * @returns {{quasis: BasicEvaluatedExpression[], parts: BasicEvaluatedExpression[]}} Simplified template */ const getSimplifiedTemplateResult = (kind, templateLiteralExpr) => { const quasis = []; From f0dfc45b9f6c7507d48bf5c500015f0f0008d119 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Sat, 3 Nov 2018 11:14:15 +0100 Subject: [PATCH 11/13] merge parts too when simplifying fix bug which keeps const conditional expression in bundle remove parsing code from ContextDependencyHelpers --- lib/BasicEvaluatedExpression.js | 35 ++++++- lib/Parser.js | 67 ++++++------ lib/dependencies/ContextDependencyHelpers.js | 104 +++++++++---------- test/configCases/parsing/issue-8293/index.js | 2 + 4 files changed, 114 insertions(+), 94 deletions(-) diff --git a/lib/BasicEvaluatedExpression.js b/lib/BasicEvaluatedExpression.js index 280590e182a..559c521142a 100644 --- a/lib/BasicEvaluatedExpression.js +++ b/lib/BasicEvaluatedExpression.js @@ -107,10 +107,36 @@ class BasicEvaluatedExpression { : undefined; } if (this.isTemplateString()) { - for (const quasi of this.quasis) { - if (quasi.asBool()) return true; + const str = this.asString(); + if (typeof str === "string") return str !== ""; + } + return undefined; + } + + asString() { + if (this.isBoolean()) return `${this.bool}`; + if (this.isNull()) return "null"; + if (this.isString()) return this.string; + if (this.isNumber()) return `${this.number}`; + if (this.isRegExp()) return `${this.regExp}`; + if (this.isArray()) { + let array = []; + for (const item of this.items) { + const itemStr = item.asString(); + if (itemStr === undefined) return undefined; + array.push(itemStr); + } + return `${array}`; + } + if (this.isConstArray()) return `${this.array}`; + if (this.isTemplateString()) { + let str = ""; + for (const part of this.parts) { + const partStr = part.asString(); + if (partStr === undefined) return undefined; + str += partStr; } - // can't tell if string will be empty without executing + return str; } return undefined; } @@ -186,10 +212,11 @@ class BasicEvaluatedExpression { return this; } - setTemplateString(quasis, parts) { + setTemplateString(quasis, parts, kind) { this.type = TypeTemplateString; this.quasis = quasis; this.parts = parts; + this.templateStringKind = kind; return this; } diff --git a/lib/Parser.js b/lib/Parser.js index 73cd90e41e3..0ff49bb7b76 100644 --- a/lib/Parser.js +++ b/lib/Parser.js @@ -611,37 +611,33 @@ class Parser extends Tapable { for (let i = 0; i < templateLiteralExpr.quasis.length; i++) { const quasiExpr = templateLiteralExpr.quasis[i]; - quasis.push( - // expression is meaningless here - // as it may correspond to multiple expressions - new BasicEvaluatedExpression() - .setString(quasiExpr.value[kind]) - .setRange(quasiExpr.range) - ); - parts.push( - new BasicEvaluatedExpression() - .setString(quasiExpr.value[kind]) - .setRange(quasiExpr.range) - .setExpression(quasiExpr) - ); + const quasi = quasiExpr.value[kind]; if (i > 0) { - const prevExpr = quasis[quasis.length - 2], - lastExpr = quasis[quasis.length - 1]; + const prevExpr = parts[parts.length - 1]; const expr = this.evaluateExpression( templateLiteralExpr.expressions[i - 1] ); + const exprAsString = expr.asString(); + if (typeof exprAsString === "string") { + // We can merge quasi + expr + quasi when expr + // is a const string + + prevExpr.setString(prevExpr.string + exprAsString + quasi); + prevExpr.setRange([prevExpr.range[0], quasiExpr.range[1]]); + // We unset the expression as it doesn't match to a single expression + prevExpr.setExpression(undefined); + continue; + } parts.push(expr); - if (!(expr.isString() || expr.isNumber())) continue; - - prevExpr.setString( - prevExpr.string + - (expr.isString() ? expr.string : expr.number) + - lastExpr.string - ); - prevExpr.setRange([prevExpr.range[0], lastExpr.range[1]]); - quasis.pop(); } + + const part = new BasicEvaluatedExpression() + .setString(quasi) + .setRange(quasiExpr.range) + .setExpression(quasiExpr); + quasis.push(part); + parts.push(part); } return { quasis, @@ -650,27 +646,22 @@ class Parser extends Tapable { }; this.hooks.evaluate.for("TemplateLiteral").tap("Parser", node => { - const { quasis, parts } = getSimplifiedTemplateResult.call( - this, - "cooked", - node - ); - if (quasis.length === 1) { - return quasis[0].setRange(node.range); + const { quasis, parts } = getSimplifiedTemplateResult("cooked", node); + if (parts.length === 1) { + return parts[0].setRange(node.range); } return new BasicEvaluatedExpression() - .setTemplateString(quasis, parts) + .setTemplateString(quasis, parts, "cooked") .setRange(node.range); }); this.hooks.evaluate.for("TaggedTemplateExpression").tap("Parser", node => { if (this.evaluateExpression(node.tag).identifier !== "String.raw") return; - const { quasis, parts } = getSimplifiedTemplateResult.call( - this, - "raw", - node.quasi - ); + const { quasis, parts } = getSimplifiedTemplateResult("raw", node.quasi); + if (parts.length === 1) { + return parts[0].setRange(node.range); + } return new BasicEvaluatedExpression() - .setTemplateString(quasis, parts) + .setTemplateString(quasis, parts, "raw") .setRange(node.range); }); diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index a95c370b828..ce2d3ab0369 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -57,19 +57,30 @@ ContextDependencyHelpers.create = ( param.quasis.length > 1 ? param.quasis[param.quasis.length - 1].string : ""; - const prefixRange = [param.quasis[0].range[0], param.quasis[0].range[1]]; - const postfixRange = - param.quasis.length > 1 - ? param.quasis[param.quasis.length - 1].range - : ""; + const valueRange = param.range; const { context, prefix } = splitContextFromPrefix(prefixRaw); const { postfix, query } = splitQueryFromPostfix(postfixRaw); - // If there are more than two quasis, maybe the generated RegExp can be more precise? + + // When there are more than two quasis, the generated RegExp can be more precise + // We join the quasis with the expression regexp + const innerQuasis = param.quasis.slice(1, param.quasis.length - 1); + const innerRegExp = + options.wrappedContextRegExp.source + + innerQuasis + .map(q => quotemeta(q.string) + options.wrappedContextRegExp.source) + .join(""); + + // Example: `./context/pre${e}inner${e}inner2${e}post?query` + // context: "./context" + // prefix: "./pre" + // innerQuasis: [BEE("inner"), BEE("inner2")] + // (BEE = BasicEvaluatedExpression) + // postfix: "post" + // query: "?query" + // regExp: /^\.\/pre.*inner.*inner2.*post$/ const regExp = new RegExp( - `^${quotemeta(prefix)}${options.wrappedContextRegExp.source}${quotemeta( - postfix - )}$` + `^${quotemeta(prefix)}${innerRegExp}${quotemeta(postfix)}$` ); const dep = new Dep( Object.assign( @@ -88,57 +99,46 @@ ContextDependencyHelpers.create = ( const replaces = []; if (parser) { - let paramExpr = param.expression; - while (paramExpr.type !== "TemplateLiteral") { - if (paramExpr.type === "ConditionalExpression") { - const test = parser.evaluateExpression(paramExpr.test); - let replace; - if (test.asBool()) { - replace = paramExpr.alternate; - paramExpr = paramExpr.consequent; - } else { - replace = paramExpr.consequent; - paramExpr = paramExpr.alternate; + param.parts.forEach((part, i) => { + if (i % 2 === 0) { + // Quasis or merged quasi + let range = part.range; + let value = part.string; + if (param.templateStringKind === "cooked") { + value = JSON.stringify(value); + value = value.slice(1, value.length - 1); + } + if (i === 0) { + // prefix + value = prefix; + range = [param.range[0], part.range[1]]; + value = + (param.templateStringKind === "cooked" ? "`" : "String.raw`") + + value; + } else if (i === param.parts.length - 1) { + // postfix + value = postfix; + range = [part.range[0], param.range[1]]; + value = value + "`"; + } else if ( + param.expression && + param.expression.type === "TemplateElement" && + param.expression.value[param.templateStringKind] === value + ) { + // Shortcut when it's a single quasi and doesn't need to be replaced + return; } replaces.push({ - range: replace.range, - value: JSON.stringify("") + range, + value }); - } else if (paramExpr.type === "TaggedTemplateExpression") { - paramExpr = paramExpr.quasi; } else { - break; // there are other cases, just ignore for now - } - } - param.parts.forEach(part => { - if ( - part.range[0] > prefixRange[1] && - (!postfixRange || part.range[1] < postfixRange[0]) - ) { - if (part.isString()) { - replaces.push({ - range: part.range, - value: part.string - }); - } else { - parser.walkExpression(part.expression); - } + // Expression + parser.walkExpression(part.expression); } }); } - // replace even prefix === prefixRaw as it's evaluated - replaces.push({ - range: prefixRange, - value: prefix - }); - // replace even postfix === postfixRaw as it's evaluated - if (postfixRange) { - replaces.push({ - range: postfixRange, - value: postfix - }); - } dep.replaces = replaces; dep.critical = options.wrappedContextCritical && diff --git a/test/configCases/parsing/issue-8293/index.js b/test/configCases/parsing/issue-8293/index.js index cc01b6c5646..bb60bef0efa 100644 --- a/test/configCases/parsing/issue-8293/index.js +++ b/test/configCases/parsing/issue-8293/index.js @@ -11,6 +11,8 @@ const path = require("path"); expect(source).toContain(`\`./\${foobar}/suffix0`); expect(source).toContain(`\`./\${foobar}/suffix3`); expect(source).not.toContain(`\`./\${foobar}/suffix4`); + expect(source).not.toContain(`\`./\${DEFINED_EXPRESSION}/\${CONST_SUFFIX4}`); + expect(source).not.toContain(`typeof require ===`); }); } ); From 1a541e12cef4f31437ad1831f92911306e57ae9f Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Sat, 3 Nov 2018 11:19:37 +0100 Subject: [PATCH 12/13] Perform replacmenents even without parser --- lib/dependencies/ContextDependencyHelpers.js | 76 ++++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index ce2d3ab0369..16f5de55200 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -98,46 +98,46 @@ ContextDependencyHelpers.create = ( dep.loc = expr.loc; const replaces = []; - if (parser) { - param.parts.forEach((part, i) => { - if (i % 2 === 0) { - // Quasis or merged quasi - let range = part.range; - let value = part.string; - if (param.templateStringKind === "cooked") { - value = JSON.stringify(value); - value = value.slice(1, value.length - 1); - } - if (i === 0) { - // prefix - value = prefix; - range = [param.range[0], part.range[1]]; - value = - (param.templateStringKind === "cooked" ? "`" : "String.raw`") + - value; - } else if (i === param.parts.length - 1) { - // postfix - value = postfix; - range = [part.range[0], param.range[1]]; - value = value + "`"; - } else if ( - param.expression && - param.expression.type === "TemplateElement" && - param.expression.value[param.templateStringKind] === value - ) { - // Shortcut when it's a single quasi and doesn't need to be replaced - return; - } - replaces.push({ - range, - value - }); - } else { - // Expression + param.parts.forEach((part, i) => { + if (i % 2 === 0) { + // Quasis or merged quasi + let range = part.range; + let value = part.string; + if (param.templateStringKind === "cooked") { + value = JSON.stringify(value); + value = value.slice(1, value.length - 1); + } + if (i === 0) { + // prefix + value = prefix; + range = [param.range[0], part.range[1]]; + value = + (param.templateStringKind === "cooked" ? "`" : "String.raw`") + + value; + } else if (i === param.parts.length - 1) { + // postfix + value = postfix; + range = [part.range[0], param.range[1]]; + value = value + "`"; + } else if ( + param.expression && + param.expression.type === "TemplateElement" && + param.expression.value[param.templateStringKind] === value + ) { + // Shortcut when it's a single quasi and doesn't need to be replaced + return; + } + replaces.push({ + range, + value + }); + } else { + // Expression + if (parser) { parser.walkExpression(part.expression); } - }); - } + } + }); dep.replaces = replaces; dep.critical = From 22aee1ec6923869bc2a3f6a57c9061b5a39dfd6e Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Sat, 3 Nov 2018 21:59:27 +0100 Subject: [PATCH 13/13] fix shortcut condition --- lib/dependencies/ContextDependencyHelpers.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index 16f5de55200..36e2dedee40 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -120,9 +120,9 @@ ContextDependencyHelpers.create = ( range = [part.range[0], param.range[1]]; value = value + "`"; } else if ( - param.expression && - param.expression.type === "TemplateElement" && - param.expression.value[param.templateStringKind] === value + part.expression && + part.expression.type === "TemplateElement" && + part.expression.value.raw === value ) { // Shortcut when it's a single quasi and doesn't need to be replaced return;