From b3ec7754530fa6146dfe0211e014bcbbdacce065 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 17 Jul 2019 13:08:51 +0200 Subject: [PATCH] improve merging of resolve and parsing options by rules and via loader API Arrays overwrite by default, but can reference old value with "..." --- lib/NormalModuleFactory.js | 4 +- lib/ResolverFactory.js | 3 +- lib/WebpackOptionsApply.js | 11 ++- lib/util/cleverMerge.js | 77 +++++++++++++++++++ test/cases/loaders/resolve/index.js | 7 +- test/cases/loaders/resolve/loader.js | 19 ++++- .../configCases/rule-set/resolve-options/a.js | 2 +- .../configCases/rule-set/resolve-options/b.js | 2 +- .../configCases/rule-set/resolve-options/c.js | 1 + .../rule-set/resolve-options/index.js | 6 +- .../rule-set/resolve-options/normal.js | 1 + .../rule-set/resolve-options/ok.ok.js | 1 + .../rule-set/resolve-options/ok2.js | 1 + .../rule-set/resolve-options/ok2.yes.js | 1 + .../resolve-options/webpack.config.js | 23 +++++- .../rule-set/resolve-options/wrong2.js | 1 + .../rule-set/resolve-options/wrong2.yes.js | 1 + 17 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 lib/util/cleverMerge.js create mode 100644 test/configCases/rule-set/resolve-options/c.js create mode 100644 test/configCases/rule-set/resolve-options/normal.js create mode 100644 test/configCases/rule-set/resolve-options/ok.ok.js create mode 100644 test/configCases/rule-set/resolve-options/ok2.js create mode 100644 test/configCases/rule-set/resolve-options/ok2.yes.js create mode 100644 test/configCases/rule-set/resolve-options/wrong2.js create mode 100644 test/configCases/rule-set/resolve-options/wrong2.yes.js diff --git a/lib/NormalModuleFactory.js b/lib/NormalModuleFactory.js index 2c2d34b27..e6a41094f 100644 --- a/lib/NormalModuleFactory.js +++ b/lib/NormalModuleFactory.js @@ -17,7 +17,7 @@ const { const NormalModule = require("./NormalModule"); const RawModule = require("./RawModule"); const RuleSet = require("./RuleSet"); -const cachedMerge = require("./util/cachedMerge"); +const { cachedCleverMerge } = require("./util/cleverMerge"); const EMPTY_RESOLVE_OPTIONS = {}; @@ -304,7 +304,7 @@ class NormalModuleFactory extends Tapable { typeof settings[r.type] === "object" && settings[r.type] !== null ) { - settings[r.type] = cachedMerge(settings[r.type], r.value); + settings[r.type] = cachedCleverMerge(settings[r.type], r.value); } else { settings[r.type] = r.value; } diff --git a/lib/ResolverFactory.js b/lib/ResolverFactory.js index 80c5e8b68..fc0ad4f8e 100644 --- a/lib/ResolverFactory.js +++ b/lib/ResolverFactory.js @@ -6,6 +6,7 @@ const { Tapable, HookMap, SyncHook, SyncWaterfallHook } = require("tapable"); const Factory = require("enhanced-resolve").ResolverFactory; +const { cachedCleverMerge } = require("./util/cleverMerge"); /** @typedef {import("enhanced-resolve").Resolver} Resolver */ @@ -66,7 +67,7 @@ module.exports = class ResolverFactory extends Tapable { resolver.withOptions = options => { const cacheEntry = childCache.get(options); if (cacheEntry !== undefined) return cacheEntry; - const mergedOptions = Object.assign({}, originalResolveOptions, options); + const mergedOptions = cachedCleverMerge(originalResolveOptions, options); const resolver = this.get(type, mergedOptions); childCache.set(options, resolver); return resolver; diff --git a/lib/WebpackOptionsApply.js b/lib/WebpackOptionsApply.js index 32a813e42..a80113ef9 100644 --- a/lib/WebpackOptionsApply.js +++ b/lib/WebpackOptionsApply.js @@ -37,6 +37,8 @@ const RequireContextPlugin = require("./dependencies/RequireContextPlugin"); const RequireEnsurePlugin = require("./dependencies/RequireEnsurePlugin"); const RequireIncludePlugin = require("./dependencies/RequireIncludePlugin"); +const { cachedCleverMerge } = require("./util/cleverMerge"); + /** @typedef {import("../declarations/WebpackOptions").WebpackOptions} WebpackOptions */ /** @typedef {import("./Compiler")} Compiler */ @@ -512,8 +514,7 @@ class WebpackOptionsApply extends OptionsApply { { fileSystem: compiler.inputFileSystem }, - options.resolve, - resolveOptions + cachedCleverMerge(options.resolve, resolveOptions) ); }); compiler.resolverFactory.hooks.resolveOptions @@ -524,8 +525,7 @@ class WebpackOptionsApply extends OptionsApply { fileSystem: compiler.inputFileSystem, resolveToContext: true }, - options.resolve, - resolveOptions + cachedCleverMerge(options.resolve, resolveOptions) ); }); compiler.resolverFactory.hooks.resolveOptions @@ -535,8 +535,7 @@ class WebpackOptionsApply extends OptionsApply { { fileSystem: compiler.inputFileSystem }, - options.resolveLoader, - resolveOptions + cachedCleverMerge(options.resolveLoader, resolveOptions) ); }); compiler.hooks.afterResolvers.call(compiler); diff --git a/lib/util/cleverMerge.js b/lib/util/cleverMerge.js new file mode 100644 index 000000000..23060cedb --- /dev/null +++ b/lib/util/cleverMerge.js @@ -0,0 +1,77 @@ +/* + MIT License http://www.opensource.org/licenses/mit-license.php + Author Tobias Koppers @sokra +*/ + +"use strict"; + +const mergeCache = new WeakMap(); + +/** + * Merges two given objects and caches the result to avoid computation if same objects passed as arguments again. + * @example + * // performs cleverMerge(first, second), stores the result in WeakMap and returns result + * cachedCleverMerge({a: 1}, {a: 2}) + * {a: 2} + * // when same arguments passed, gets the result from WeakMap and returns it. + * cachedCleverMerge({a: 1}, {a: 2}) + * {a: 2} + * @param {object} first first object + * @param {object} second second object + * @returns {object} merged object of first and second object + */ +const cachedCleverMerge = (first, second) => { + let innerCache = mergeCache.get(first); + if (innerCache === undefined) { + innerCache = new WeakMap(); + mergeCache.set(first, innerCache); + } + const prevMerge = innerCache.get(second); + if (prevMerge !== undefined) return prevMerge; + const newMerge = cleverMerge(first, second); + innerCache.set(second, newMerge); + return newMerge; +}; + +/** + * Merges two objects. Objects are not deeply merged. + * TODO webpack 5: merge objects deeply clever. + * Arrays might reference the old value with "..." + * @param {object} first first object + * @param {object} second second object + * @returns {object} merged object of first and second object + */ +const cleverMerge = (first, second) => { + const newObject = Object.assign({}, first); + for (const key of Object.keys(second)) { + if (!(key in newObject)) { + newObject[key] = second[key]; + continue; + } + const secondValue = second[key]; + if (!Array.isArray(secondValue)) { + newObject[key] = secondValue; + continue; + } + const firstValue = newObject[key]; + if (Array.isArray(firstValue)) { + const newArray = []; + for (const item of secondValue) { + if (item === "...") { + for (const item of firstValue) { + newArray.push(item); + } + } else { + newArray.push(item); + } + } + newObject[key] = newArray; + } else { + newObject[key] = secondValue; + } + } + return newObject; +}; + +exports.cachedCleverMerge = cachedCleverMerge; +exports.cleverMerge = cleverMerge; diff --git a/test/cases/loaders/resolve/index.js b/test/cases/loaders/resolve/index.js index 07e030a14..23f20fec5 100644 --- a/test/cases/loaders/resolve/index.js +++ b/test/cases/loaders/resolve/index.js @@ -2,6 +2,9 @@ it("should be possible to create resolver with different options", () => { const result = require("./loader!"); expect(result).toEqual({ one: "index.js", - two: "index.xyz" + two: "index.xyz", + three: "index.js", + four: "index.xyz", + five: "index.js" }); -}) +}); diff --git a/test/cases/loaders/resolve/loader.js b/test/cases/loaders/resolve/loader.js index 3e5581127..3bb4b23d8 100644 --- a/test/cases/loaders/resolve/loader.js +++ b/test/cases/loaders/resolve/loader.js @@ -4,13 +4,28 @@ module.exports = function() { const resolve2 = this.getResolve({ extensions: [".xyz", ".js"] }); + const resolve3 = this.getResolve({ + extensions: [".hee", "..."] + }); + const resolve4 = this.getResolve({ + extensions: [".xyz", "..."] + }); + const resolve5 = this.getResolve({ + extensions: ["...", ".xyz"] + }); return Promise.all([ resolve1(__dirname, "./index"), - resolve2(__dirname, "./index") - ]).then(([one, two]) => { + resolve2(__dirname, "./index"), + resolve3(__dirname, "./index"), + resolve4(__dirname, "./index"), + resolve5(__dirname, "./index") + ]).then(([one, two, three, four, five]) => { return `module.exports = ${JSON.stringify({ one: path.basename(one), two: path.basename(two), + three: path.basename(three), + four: path.basename(four), + five: path.basename(five) })}`; }); }; diff --git a/test/configCases/rule-set/resolve-options/a.js b/test/configCases/rule-set/resolve-options/a.js index f7293f306..79895f971 100644 --- a/test/configCases/rule-set/resolve-options/a.js +++ b/test/configCases/rule-set/resolve-options/a.js @@ -1 +1 @@ -module.exports = require("./wrong"); +module.exports = require("./wrong") + require("./normal") + require("./wrong2"); diff --git a/test/configCases/rule-set/resolve-options/b.js b/test/configCases/rule-set/resolve-options/b.js index f7293f306..79895f971 100644 --- a/test/configCases/rule-set/resolve-options/b.js +++ b/test/configCases/rule-set/resolve-options/b.js @@ -1 +1 @@ -module.exports = require("./wrong"); +module.exports = require("./wrong") + require("./normal") + require("./wrong2"); diff --git a/test/configCases/rule-set/resolve-options/c.js b/test/configCases/rule-set/resolve-options/c.js new file mode 100644 index 000000000..79895f971 --- /dev/null +++ b/test/configCases/rule-set/resolve-options/c.js @@ -0,0 +1 @@ +module.exports = require("./wrong") + require("./normal") + require("./wrong2"); diff --git a/test/configCases/rule-set/resolve-options/index.js b/test/configCases/rule-set/resolve-options/index.js index 5baf4c239..f64073277 100644 --- a/test/configCases/rule-set/resolve-options/index.js +++ b/test/configCases/rule-set/resolve-options/index.js @@ -1,6 +1,8 @@ it("should allow to set custom resolving rules", function() { var a = require("./a"); - expect(a).toBe("ok"); + expect(a).toBe("ok-normal-wrong2"); var b = require("./b"); - expect(b).toBe("wrong"); + expect(b).toBe("ok-normal-wrong2-yes"); + var c = require("./c"); + expect(c).toBe("wrong-normal-ok2"); }); diff --git a/test/configCases/rule-set/resolve-options/normal.js b/test/configCases/rule-set/resolve-options/normal.js new file mode 100644 index 000000000..f5f0f4cc5 --- /dev/null +++ b/test/configCases/rule-set/resolve-options/normal.js @@ -0,0 +1 @@ +module.exports = "-normal-"; diff --git a/test/configCases/rule-set/resolve-options/ok.ok.js b/test/configCases/rule-set/resolve-options/ok.ok.js new file mode 100644 index 000000000..11132c143 --- /dev/null +++ b/test/configCases/rule-set/resolve-options/ok.ok.js @@ -0,0 +1 @@ +module.exports = "ok-ok"; diff --git a/test/configCases/rule-set/resolve-options/ok2.js b/test/configCases/rule-set/resolve-options/ok2.js new file mode 100644 index 000000000..c0c585d76 --- /dev/null +++ b/test/configCases/rule-set/resolve-options/ok2.js @@ -0,0 +1 @@ +module.exports = "ok2"; diff --git a/test/configCases/rule-set/resolve-options/ok2.yes.js b/test/configCases/rule-set/resolve-options/ok2.yes.js new file mode 100644 index 000000000..46778e20f --- /dev/null +++ b/test/configCases/rule-set/resolve-options/ok2.yes.js @@ -0,0 +1 @@ +module.exports = "ok2-yes"; diff --git a/test/configCases/rule-set/resolve-options/webpack.config.js b/test/configCases/rule-set/resolve-options/webpack.config.js index 9ce4b7957..7808abf02 100644 --- a/test/configCases/rule-set/resolve-options/webpack.config.js +++ b/test/configCases/rule-set/resolve-options/webpack.config.js @@ -1,4 +1,9 @@ module.exports = { + resolve: { + alias: { + "./wrong2": "./ok2" + } + }, module: { rules: [ { @@ -6,7 +11,23 @@ module.exports = { resolve: { alias: { "./wrong": "./ok" - } + }, + extensions: [".js", ".ok.js"] + } + }, + { + test: require.resolve("./b"), + resolve: { + alias: { + "./wrong": "./ok" + }, + extensions: ["...", ".ok.js"] + } + }, + { + test: require.resolve("./b"), + resolve: { + extensions: [".yes.js", "..."] } } ] diff --git a/test/configCases/rule-set/resolve-options/wrong2.js b/test/configCases/rule-set/resolve-options/wrong2.js new file mode 100644 index 000000000..62f3d2d8d --- /dev/null +++ b/test/configCases/rule-set/resolve-options/wrong2.js @@ -0,0 +1 @@ +module.exports = "wrong2"; diff --git a/test/configCases/rule-set/resolve-options/wrong2.yes.js b/test/configCases/rule-set/resolve-options/wrong2.yes.js new file mode 100644 index 000000000..cbe1bee99 --- /dev/null +++ b/test/configCases/rule-set/resolve-options/wrong2.yes.js @@ -0,0 +1 @@ +module.exports = "wrong2-yes";