From 61633aa91c61328a31c132d5afdd2acd8219cb5c Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 3 Aug 2018 09:20:23 +0200 Subject: [PATCH] fix #7778 remove prepend in favor of replaces improve code style in ContextDependencyHelpers --- lib/dependencies/ContextDependencyHelpers.js | 121 +++++++++++------- .../ContextDependencyTemplateAsId.js | 1 + .../ContextDependencyTemplateAsRequireCall.js | 1 + test/cases/parsing/issue-7778/a.js | 2 +- test/cases/parsing/issue-7778/abc.js | 3 + test/cases/parsing/issue-7778/index.js | 60 +++++++-- 6 files changed, 136 insertions(+), 52 deletions(-) create mode 100644 test/cases/parsing/issue-7778/abc.js diff --git a/lib/dependencies/ContextDependencyHelpers.js b/lib/dependencies/ContextDependencyHelpers.js index d4cc72dbb..2dabc3345 100644 --- a/lib/dependencies/ContextDependencyHelpers.js +++ b/lib/dependencies/ContextDependencyHelpers.js @@ -15,6 +15,32 @@ const quotemeta = str => { return str.replace(/[-[\]\\/{}()*+?.^$|]/g, "\\$&"); }; +const splitContextFromPrefix = prefix => { + const idx = prefix.lastIndexOf("/"); + let context = "."; + if (idx >= 0) { + context = prefix.substr(0, idx); + prefix = `.${prefix.substr(idx)}`; + } + return { + context, + prefix + }; +}; + +const splitQueryFromPostfix = postfix => { + const idx = postfix.indexOf("?"); + let query = ""; + if (idx >= 0) { + query = postfix.substr(idx); + postfix = postfix.substr(0, idx); + } + return { + postfix, + query + }; +}; + ContextDependencyHelpers.create = ( Dep, range, @@ -23,38 +49,30 @@ ContextDependencyHelpers.create = ( options, contextOptions ) => { - let dep; - let prefix; - let postfix; - let prefixRange; - let valueRange; - let idx; - let context; - let regExp; if (param.isTemplateString()) { - prefix = param.quasis[0].string; - postfix = + let prefixRaw = param.quasis[0].string; + let postfixRaw = param.quasis.length > 1 ? param.quasis[param.quasis.length - 1].string : ""; - prefixRange = [param.quasis[0].range[0], param.quasis[0].range[1]]; - valueRange = param.range; - idx = prefix.lastIndexOf("/"); - context = "."; - if (idx >= 0) { - context = prefix.substr(0, idx); - prefix = `.${prefix.substr(idx)}`; - } + 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? - regExp = new RegExp( + const regExp = new RegExp( `^${quotemeta(prefix)}${options.wrappedContextRegExp.source}${quotemeta( postfix )}$` ); - dep = new Dep( + const dep = new Dep( Object.assign( { - request: context, + request: context + query, recursive: options.wrappedContextRecursive, regExp, mode: "sync" @@ -65,12 +83,20 @@ ContextDependencyHelpers.create = ( valueRange ); dep.loc = expr.loc; - dep.replaces = [ - { + const replaces = []; + if (prefixRange && prefix !== prefixRaw) { + replaces.push({ range: prefixRange, value: prefix - } - ]; + }); + } + if (postfixRange && postfix !== postfixRaw) { + replaces.push({ + range: postfixRange, + value: postfix + }); + } + dep.replaces = replaces; dep.critical = options.wrappedContextCritical && "a part of the request of a dependency is an expression"; @@ -80,30 +106,26 @@ ContextDependencyHelpers.create = ( ((param.prefix && param.prefix.isString()) || (param.postfix && param.postfix.isString())) ) { - prefix = param.prefix && param.prefix.isString() ? param.prefix.string : ""; - postfix = + let prefixRaw = + param.prefix && param.prefix.isString() ? param.prefix.string : ""; + let postfixRaw = param.postfix && param.postfix.isString() ? param.postfix.string : ""; - prefixRange = + const prefixRange = param.prefix && param.prefix.isString() ? param.prefix.range : null; - valueRange = [ - prefixRange ? prefixRange[1] : param.range[0], - param.range[1] - ]; - idx = prefix.lastIndexOf("/"); - context = "."; - if (idx >= 0) { - context = prefix.substr(0, idx); - prefix = `.${prefix.substr(idx)}`; - } - regExp = new RegExp( + const postfixRange = + param.postfix && param.postfix.isString() ? param.postfix.range : null; + const valueRange = param.range; + const { context, prefix } = splitContextFromPrefix(prefixRaw); + const { postfix, query } = splitQueryFromPostfix(postfixRaw); + const regExp = new RegExp( `^${quotemeta(prefix)}${options.wrappedContextRegExp.source}${quotemeta( postfix )}$` ); - dep = new Dep( + const dep = new Dep( Object.assign( { - request: context, + request: context + query, recursive: options.wrappedContextRecursive, regExp, mode: "sync" @@ -114,13 +136,26 @@ ContextDependencyHelpers.create = ( valueRange ); dep.loc = expr.loc; - dep.prepend = param.prefix && param.prefix.isString() ? prefix : null; + const replaces = []; + if (prefixRange && prefix !== prefixRaw) { + replaces.push({ + range: prefixRange, + value: JSON.stringify(prefix) + }); + } + if (postfixRange && postfix !== postfixRaw) { + replaces.push({ + range: postfixRange, + value: JSON.stringify(postfix) + }); + } + dep.replaces = replaces; dep.critical = options.wrappedContextCritical && "a part of the request of a dependency is an expression"; return dep; } else { - dep = new Dep( + const dep = new Dep( Object.assign( { request: options.exprContextRequest, diff --git a/lib/dependencies/ContextDependencyTemplateAsId.js b/lib/dependencies/ContextDependencyTemplateAsId.js index 14f5aa79e..1ee83ff99 100644 --- a/lib/dependencies/ContextDependencyTemplateAsId.js +++ b/lib/dependencies/ContextDependencyTemplateAsId.js @@ -20,6 +20,7 @@ class ContextDependencyTemplateAsId { } } source.replace(dep.valueRange[1], dep.range[1] - 1, ")"); + // TODO webpack 5 remove `prepend` it's no longer used source.replace( dep.range[0], dep.valueRange[0] - 1, diff --git a/lib/dependencies/ContextDependencyTemplateAsRequireCall.js b/lib/dependencies/ContextDependencyTemplateAsRequireCall.js index 5dac6c782..6d833ac83 100644 --- a/lib/dependencies/ContextDependencyTemplateAsRequireCall.js +++ b/lib/dependencies/ContextDependencyTemplateAsRequireCall.js @@ -20,6 +20,7 @@ class ContextDependencyTemplateAsRequireCall { } } source.replace(dep.valueRange[1], dep.range[1] - 1, ")"); + // TODO webpack 5 remove `prepend` it's no longer used source.replace( dep.range[0], dep.valueRange[0] - 1, diff --git a/test/cases/parsing/issue-7778/a.js b/test/cases/parsing/issue-7778/a.js index 7afed0bb0..aa67bc111 100644 --- a/test/cases/parsing/issue-7778/a.js +++ b/test/cases/parsing/issue-7778/a.js @@ -1,3 +1,3 @@ export default function a() { - return 'a' + return 'a' + __resourceQuery } diff --git a/test/cases/parsing/issue-7778/abc.js b/test/cases/parsing/issue-7778/abc.js new file mode 100644 index 000000000..0b6145ef7 --- /dev/null +++ b/test/cases/parsing/issue-7778/abc.js @@ -0,0 +1,3 @@ +export default function abc() { + return 'abc' + __resourceQuery +} diff --git a/test/cases/parsing/issue-7778/index.js b/test/cases/parsing/issue-7778/index.js index ad1877023..0f8dda218 100644 --- a/test/cases/parsing/issue-7778/index.js +++ b/test/cases/parsing/issue-7778/index.js @@ -1,13 +1,57 @@ it("should detect query strings in dynamic import as a static value 1 ", function() { - import("./a?queryString").then(({ default: a }) => { - expect(a()).toBe("a"); - }); + return Promise.all([ + import("./a").then(({ default: a }) => { + expect(a()).toBe("a"); + }), + import("./abc").then(({ default: a }) => { + expect(a()).toBe("abc"); + }), + import("./a?queryString").then(({ default: a }) => { + expect(a()).toBe("a?queryString"); + }), + import("./abc?query?String").then(({ default: a }) => { + expect(a()).toBe("abc?query?String"); + }), + ]); }); it("should detect query strings in dynamic import as a static value 2", function() { - var testFileName = "a"; - - import(`./${testFileName}?queryString`).then(({ default: a }) => { - expect(a()).toBe("a"); - }); + var testFileName = "a"; + + return Promise.all([ + import(`./${testFileName}`).then(({ default: a }) => { + expect(a()).toBe("a"); + }), + import(`./${testFileName}bc`).then(({ default: a }) => { + expect(a()).toBe("abc"); + }), + import(`./${testFileName}?queryString`).then(({ default: a }) => { + expect(a()).toBe("a?queryString"); + }), + import(`./${testFileName}bc?query?String`).then(({ default: a }) => { + expect(a()).toBe("abc?query?String"); + }) + ]); +}); + +it("should detect query strings in dynamic import as a static value 2", function() { + var testFileName = "a"; + + return Promise.all([ + import("./" + testFileName).then(({ default: a }) => { + expect(a()).toBe("a"); + }), + import("./" + testFileName + "").then(({ default: a }) => { + expect(a()).toBe("a"); + }), + import("./" + testFileName + "bc").then(({ default: a }) => { + expect(a()).toBe("abc"); + }), + import("./" + testFileName + "?queryString").then(({ default: a }) => { + expect(a()).toBe("a?queryString"); + }), + import("./" + testFileName + "bc?query?String").then(({ default: a }) => { + expect(a()).toBe("abc?query?String"); + }) + ]); });