Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve merging of resolve and parsing options #9422

Merged
merged 1 commit into from Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/NormalModuleFactory.js
Expand Up @@ -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 = {};

Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion lib/ResolverFactory.js
Expand Up @@ -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 */

Expand Down Expand Up @@ -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;
Expand Down
11 changes: 5 additions & 6 deletions lib/WebpackOptionsApply.js
Expand Up @@ -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 */

Expand Down Expand Up @@ -512,8 +514,7 @@ class WebpackOptionsApply extends OptionsApply {
{
fileSystem: compiler.inputFileSystem
},
options.resolve,
resolveOptions
cachedCleverMerge(options.resolve, resolveOptions)
);
});
compiler.resolverFactory.hooks.resolveOptions
Expand All @@ -524,8 +525,7 @@ class WebpackOptionsApply extends OptionsApply {
fileSystem: compiler.inputFileSystem,
resolveToContext: true
},
options.resolve,
resolveOptions
cachedCleverMerge(options.resolve, resolveOptions)
);
});
compiler.resolverFactory.hooks.resolveOptions
Expand All @@ -535,8 +535,7 @@ class WebpackOptionsApply extends OptionsApply {
{
fileSystem: compiler.inputFileSystem
},
options.resolveLoader,
resolveOptions
cachedCleverMerge(options.resolveLoader, resolveOptions)
);
});
compiler.hooks.afterResolvers.call(compiler);
Expand Down
77 changes: 77 additions & 0 deletions 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;
7 changes: 5 additions & 2 deletions test/cases/loaders/resolve/index.js
Expand Up @@ -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"
});
})
});
19 changes: 17 additions & 2 deletions test/cases/loaders/resolve/loader.js
Expand Up @@ -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)
})}`;
});
};
2 changes: 1 addition & 1 deletion test/configCases/rule-set/resolve-options/a.js
@@ -1 +1 @@
module.exports = require("./wrong");
module.exports = require("./wrong") + require("./normal") + require("./wrong2");
2 changes: 1 addition & 1 deletion test/configCases/rule-set/resolve-options/b.js
@@ -1 +1 @@
module.exports = require("./wrong");
module.exports = require("./wrong") + require("./normal") + require("./wrong2");
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/c.js
@@ -0,0 +1 @@
module.exports = require("./wrong") + require("./normal") + require("./wrong2");
6 changes: 4 additions & 2 deletions 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");
});
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/normal.js
@@ -0,0 +1 @@
module.exports = "-normal-";
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/ok.ok.js
@@ -0,0 +1 @@
module.exports = "ok-ok";
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/ok2.js
@@ -0,0 +1 @@
module.exports = "ok2";
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/ok2.yes.js
@@ -0,0 +1 @@
module.exports = "ok2-yes";
23 changes: 22 additions & 1 deletion test/configCases/rule-set/resolve-options/webpack.config.js
@@ -1,12 +1,33 @@
module.exports = {
resolve: {
alias: {
"./wrong2": "./ok2"
}
},
module: {
rules: [
{
test: require.resolve("./a"),
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", "..."]
}
}
]
Expand Down
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/wrong2.js
@@ -0,0 +1 @@
module.exports = "wrong2";
1 change: 1 addition & 0 deletions test/configCases/rule-set/resolve-options/wrong2.yes.js
@@ -0,0 +1 @@
module.exports = "wrong2-yes";