From 534660ab92e7023cb5c3f9ce4502a0847eb30683 Mon Sep 17 00:00:00 2001 From: Vincent Bailly Date: Tue, 10 Dec 2024 17:26:20 +0100 Subject: [PATCH 1/5] optimize alias plugin for huge alias objects --- lib/AliasPlugin.js | 275 +++++++++++++++++++++++---------------------- 1 file changed, 139 insertions(+), 136 deletions(-) diff --git a/lib/AliasPlugin.js b/lib/AliasPlugin.js index bbe9724a..bde95d54 100644 --- a/lib/AliasPlugin.js +++ b/lib/AliasPlugin.js @@ -1,11 +1,10 @@ /* - MIT License http://www.opensource.org/licenses/mit-license.php - Author Tobias Koppers @sokra + MIT License http://www.opensource.org/licenses/mit-license.php + Author Tobias Koppers @sokra */ "use strict"; -const forEachBail = require("./forEachBail"); const { PathType, getType } = require("./util/path"); /** @typedef {import("./Resolver")} Resolver */ @@ -15,140 +14,144 @@ const { PathType, getType } = require("./util/path"); /** @typedef {{alias: Alias, name: string, onlyModule?: boolean}} AliasOption */ module.exports = class AliasPlugin { - /** - * @param {string | ResolveStepHook} source source - * @param {AliasOption | Array} options options - * @param {string | ResolveStepHook} target target - */ - constructor(source, options, target) { - this.source = source; - this.options = Array.isArray(options) ? options : [options]; - this.target = target; - } + /** + * @param {string | ResolveStepHook} source source + * @param {AliasOption | Array} options options + * @param {string | ResolveStepHook} target target + */ + constructor(source, options, target) { + /** + * @param {string} maybeAbsolutePath path + * @returns {null|string} absolute path with slash ending + */ + const getAbsolutePathWithSlashEnding = maybeAbsolutePath => { + const type = getType(maybeAbsolutePath); + if (type === PathType.AbsolutePosix || type === PathType.AbsoluteWin) { + return resolver.join(maybeAbsolutePath, "_").slice(0, -1); + } + return null; + }; + + this.source = source; + this.options = Array.isArray(options) ? options : [options]; + for(const item of this.options) { + item.nameWithSlash = item.name + '/'; + item.absolutePath = getAbsolutePathWithSlashEnding(item.name); + } + this.target = target; + } + + /** + * @param {Resolver} resolver the resolver + * @returns {void} + */ + apply(resolver) { + const target = resolver.ensureHook(this.target); + /** + * @param {string} path path + * @param {string} maybeSubPath sub path + * @returns {boolean} true, if path is sub path + */ + const isSubPath = (path, absolutePath) => { + if (!absolutePath) return false; + return path.startsWith(absolutePath); + }; + resolver + .getHook(this.source) + .tapAsync("AliasPlugin", (request, resolveContext, callback) => { + const innerRequest = request.request || request.path; + if (!innerRequest) return callback(); + let i = 0; - /** - * @param {Resolver} resolver the resolver - * @returns {void} - */ - apply(resolver) { - const target = resolver.ensureHook(this.target); - /** - * @param {string} maybeAbsolutePath path - * @returns {null|string} absolute path with slash ending - */ - const getAbsolutePathWithSlashEnding = maybeAbsolutePath => { - const type = getType(maybeAbsolutePath); - if (type === PathType.AbsolutePosix || type === PathType.AbsoluteWin) { - return resolver.join(maybeAbsolutePath, "_").slice(0, -1); - } - return null; - }; - /** - * @param {string} path path - * @param {string} maybeSubPath sub path - * @returns {boolean} true, if path is sub path - */ - const isSubPath = (path, maybeSubPath) => { - const absolutePath = getAbsolutePathWithSlashEnding(maybeSubPath); - if (!absolutePath) return false; - return path.startsWith(absolutePath); - }; - resolver - .getHook(this.source) - .tapAsync("AliasPlugin", (request, resolveContext, callback) => { - const innerRequest = request.request || request.path; - if (!innerRequest) return callback(); - forEachBail( - this.options, - (item, callback) => { - /** @type {boolean} */ - let shouldStop = false; - if ( - innerRequest === item.name || - (!item.onlyModule && - (request.request - ? innerRequest.startsWith(`${item.name}/`) - : isSubPath(innerRequest, item.name))) - ) { - /** @type {string} */ - const remainingRequest = innerRequest.slice(item.name.length); - /** - * @param {Alias} alias alias - * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback - * @returns {void} - */ - const resolveWithAlias = (alias, callback) => { - if (alias === false) { - /** @type {ResolveRequest} */ - const ignoreObj = { - ...request, - path: false - }; - if (typeof resolveContext.yield === "function") { - resolveContext.yield(ignoreObj); - return callback(null, null); - } - return callback(null, ignoreObj); - } - if ( - innerRequest !== alias && - !innerRequest.startsWith(alias + "/") - ) { - shouldStop = true; - const newRequestStr = alias + remainingRequest; - /** @type {ResolveRequest} */ - const obj = { - ...request, - request: newRequestStr, - fullySpecified: false - }; - return resolver.doResolve( - target, - obj, - "aliased with mapping '" + - item.name + - "': '" + - alias + - "' to '" + - newRequestStr + - "'", - resolveContext, - (err, result) => { - if (err) return callback(err); - if (result) return callback(null, result); - return callback(); - } - ); - } - return callback(); - }; - /** - * @param {null|Error} [err] error - * @param {null|ResolveRequest} [result] result - * @returns {void} - */ - const stoppingCallback = (err, result) => { - if (err) return callback(err); + let resolveCallback; - if (result) return callback(null, result); - // Don't allow other aliasing or raw request - if (shouldStop) return callback(null, null); - return callback(); - }; - if (Array.isArray(item.alias)) { - return forEachBail( - item.alias, - resolveWithAlias, - stoppingCallback - ); - } else { - return resolveWithAlias(item.alias, stoppingCallback); - } - } - return callback(); - }, - callback - ); - }); + while(i < this.options.length) { + const item = this.options[i]; + i++; + if ( + innerRequest === item.name || + (!item.onlyModule && + (request.request + ? innerRequest.startsWith(item.nameWithSlash) + : isSubPath(innerRequest, item.absolutePath))) + ) { + /** @type {string} */ + const remainingRequest = innerRequest === item.name ? undefined : innerRequest.slice(item.name.length); + /** + * @param {Alias} alias alias + * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback + * @returns {void} + */ + if (Array.isArray(item.alias)) { + for(const alias of item.alias) { + const done = processAlias(alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback); + if (done) { + return; + } + } + } else { + const done = processAlias(item.alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback); + if (done) { + return; + } + } + } } + // No match + callback(); + }) + } }; + +function processAlias(alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback) { + if (alias === false) { + /** @type {ResolveRequest} */ + const ignoreObj = { + ...request, + path: false + }; + if (typeof resolveContext.yield === "function") { + resolveContext.yield(ignoreObj); + callback(null, null); + return true; + } + callback(null, ignoreObj); + return true; + } + if ( + innerRequest !== alias && + !innerRequest.startsWith(alias + "/") + ) { + const newRequestStr = remainingRequest ? alias + remainingRequest : alias; + /** @type {ResolveRequest} */ + const obj = { + ...request, + request: newRequestStr, + fullySpecified: false + }; + if (!resolveCallback) { + resolveCallback = (err, result) => { + if (err || result) { + if (err) return callback(err); + return callback(null, result); + } + return callback(null, null); + } + } + resolver.doResolve( + target, + obj, + "aliased with mapping '" + + item.name + + "': '" + + alias + + "' to '" + + newRequestStr + + "'", + resolveContext, + resolveCallback + ); + // the resolveCallback will take care of calling the callback + return true; + } +} From e4ace48dfc5fcc14afc8227d1ec8f268eb9c95df Mon Sep 17 00:00:00 2001 From: Vincent Bailly Date: Tue, 10 Dec 2024 18:02:30 +0100 Subject: [PATCH 2/5] linting and formating --- lib/AliasPlugin.js | 308 +++++++++++++++++++++++++-------------------- types.d.ts | 2 + 2 files changed, 175 insertions(+), 135 deletions(-) diff --git a/lib/AliasPlugin.js b/lib/AliasPlugin.js index bde95d54..5b760fb6 100644 --- a/lib/AliasPlugin.js +++ b/lib/AliasPlugin.js @@ -1,6 +1,6 @@ /* - MIT License http://www.opensource.org/licenses/mit-license.php - Author Tobias Koppers @sokra + MIT License http://www.opensource.org/licenses/mit-license.php + Author Tobias Koppers @sokra */ "use strict"; @@ -11,147 +11,185 @@ const { PathType, getType } = require("./util/path"); /** @typedef {import("./Resolver").ResolveRequest} ResolveRequest */ /** @typedef {import("./Resolver").ResolveStepHook} ResolveStepHook */ /** @typedef {string | Array | false} Alias */ -/** @typedef {{alias: Alias, name: string, onlyModule?: boolean}} AliasOption */ +/** @typedef {{alias: Alias, name: string, onlyModule?: boolean, nameWithSlash?: string, absolutePath?: string}} AliasOption */ module.exports = class AliasPlugin { - /** - * @param {string | ResolveStepHook} source source - * @param {AliasOption | Array} options options - * @param {string | ResolveStepHook} target target - */ - constructor(source, options, target) { - /** - * @param {string} maybeAbsolutePath path - * @returns {null|string} absolute path with slash ending - */ - const getAbsolutePathWithSlashEnding = maybeAbsolutePath => { - const type = getType(maybeAbsolutePath); - if (type === PathType.AbsolutePosix || type === PathType.AbsoluteWin) { - return resolver.join(maybeAbsolutePath, "_").slice(0, -1); - } - return null; - }; + /** + * @param {string | ResolveStepHook} source source + * @param {AliasOption | Array} options options + * @param {string | ResolveStepHook} target target + */ + constructor(source, options, target) { + this.source = source; + this.options = Array.isArray(options) ? options : [options]; + for (const item of this.options) { + item.nameWithSlash = item.name + "/"; + } + this.target = target; + } - this.source = source; - this.options = Array.isArray(options) ? options : [options]; - for(const item of this.options) { - item.nameWithSlash = item.name + '/'; - item.absolutePath = getAbsolutePathWithSlashEnding(item.name); - } - this.target = target; - } + /** + * @param {Resolver} resolver the resolver + * @returns {void} + */ + apply(resolver) { + const target = resolver.ensureHook(this.target); - /** - * @param {Resolver} resolver the resolver - * @returns {void} - */ - apply(resolver) { - const target = resolver.ensureHook(this.target); - /** - * @param {string} path path - * @param {string} maybeSubPath sub path - * @returns {boolean} true, if path is sub path - */ - const isSubPath = (path, absolutePath) => { - if (!absolutePath) return false; - return path.startsWith(absolutePath); - }; - resolver - .getHook(this.source) - .tapAsync("AliasPlugin", (request, resolveContext, callback) => { - const innerRequest = request.request || request.path; - if (!innerRequest) return callback(); - let i = 0; + /** + * @param {string} maybeAbsolutePath path + * @returns {null|string} absolute path with slash ending + */ + const getAbsolutePathWithSlashEnding = maybeAbsolutePath => { + const type = getType(maybeAbsolutePath); + if (type === PathType.AbsolutePosix || type === PathType.AbsoluteWin) { + return resolver.join(maybeAbsolutePath, "_").slice(0, -1); + } + return null; + }; - let resolveCallback; + /** + * @param {string} path path + * @param {AliasOption} item item + * @returns {boolean} true, if path is sub path + */ + const isSubPath = (path, item) => { + if (item.absolutePath === undefined) { + item.absolutePath = getAbsolutePathWithSlashEnding(item.name); + } - while(i < this.options.length) { - const item = this.options[i]; - i++; - if ( - innerRequest === item.name || - (!item.onlyModule && - (request.request - ? innerRequest.startsWith(item.nameWithSlash) - : isSubPath(innerRequest, item.absolutePath))) - ) { - /** @type {string} */ - const remainingRequest = innerRequest === item.name ? undefined : innerRequest.slice(item.name.length); - /** - * @param {Alias} alias alias - * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback - * @returns {void} - */ - if (Array.isArray(item.alias)) { - for(const alias of item.alias) { - const done = processAlias(alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback); - if (done) { - return; - } - } - } else { - const done = processAlias(item.alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback); - if (done) { - return; - } - } - } + if (!item.absolutePath) return false; + return path.startsWith(item.absolutePath); + }; + + resolver + .getHook(this.source) + .tapAsync("AliasPlugin", (request, resolveContext, callback) => { + const innerRequest = request.request || request.path; + if (!innerRequest) return callback(); + let i = 0; + + let resolveCallback; + + while (i < this.options.length) { + const item = this.options[i]; + i++; + if ( + innerRequest === item.name || + (!item.onlyModule && + (request.request + ? innerRequest.startsWith(item.nameWithSlash) + : isSubPath(innerRequest, item))) + ) { + /** @type {string} */ + const remainingRequest = + innerRequest === item.name + ? undefined + : innerRequest.slice(item.name.length); + /** + * @param {Alias} alias alias + * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback + * @returns {void} + */ + if (Array.isArray(item.alias)) { + for (const alias of item.alias) { + const done = processAlias( + alias, + request, + callback, + innerRequest, + remainingRequest, + resolver, + target, + item, + resolveContext, + resolveCallback + ); + if (done) { + return; + } + } + } else { + const done = processAlias( + item.alias, + request, + callback, + innerRequest, + remainingRequest, + resolver, + target, + item, + resolveContext, + resolveCallback + ); + if (done) { + return; + } + } + } + } + // No match + callback(); + }); } - // No match - callback(); - }) - } }; -function processAlias(alias, request, callback, innerRequest, remainingRequest, resolver, target, item, resolveContext, resolveCallback) { - if (alias === false) { - /** @type {ResolveRequest} */ - const ignoreObj = { - ...request, - path: false - }; - if (typeof resolveContext.yield === "function") { - resolveContext.yield(ignoreObj); - callback(null, null); - return true; - } - callback(null, ignoreObj); - return true; - } - if ( - innerRequest !== alias && - !innerRequest.startsWith(alias + "/") - ) { - const newRequestStr = remainingRequest ? alias + remainingRequest : alias; - /** @type {ResolveRequest} */ - const obj = { - ...request, - request: newRequestStr, - fullySpecified: false - }; - if (!resolveCallback) { - resolveCallback = (err, result) => { - if (err || result) { - if (err) return callback(err); - return callback(null, result); - } - return callback(null, null); +function processAlias( + alias, + request, + callback, + innerRequest, + remainingRequest, + resolver, + target, + item, + resolveContext, + resolveCallback +) { + if (alias === false) { + /** @type {ResolveRequest} */ + const ignoreObj = { + ...request, + path: false + }; + if (typeof resolveContext.yield === "function") { + resolveContext.yield(ignoreObj); + callback(null, null); + return true; + } + callback(null, ignoreObj); + return true; + } + if (innerRequest !== alias && !innerRequest.startsWith(alias + "/")) { + const newRequestStr = remainingRequest ? alias + remainingRequest : alias; + /** @type {ResolveRequest} */ + const obj = { + ...request, + request: newRequestStr, + fullySpecified: false + }; + if (!resolveCallback) { + resolveCallback = (err, result) => { + if (err || result) { + if (err) return callback(err); + return callback(null, result); + } + return callback(null, null); + }; + } + resolver.doResolve( + target, + obj, + "aliased with mapping '" + + item.name + + "': '" + + alias + + "' to '" + + newRequestStr + + "'", + resolveContext, + resolveCallback + ); + // the resolveCallback will take care of calling the callback + return true; } - } - resolver.doResolve( - target, - obj, - "aliased with mapping '" + - item.name + - "': '" + - alias + - "' to '" + - newRequestStr + - "'", - resolveContext, - resolveCallback - ); - // the resolveCallback will take care of calling the callback - return true; - } } diff --git a/types.d.ts b/types.d.ts index fc09289d..26a27115 100644 --- a/types.d.ts +++ b/types.d.ts @@ -19,6 +19,8 @@ declare interface AliasOption { alias: Alias; name: string; onlyModule?: boolean; + nameWithSlash?: string; + absolutePath?: string; } type AliasOptionNewRequest = string | false | string[]; declare interface AliasOptions { From 2c5fcf3c6fbc35f2ebdfba58987915dcefbde1ab Mon Sep 17 00:00:00 2001 From: Vincent Bailly Date: Tue, 10 Dec 2024 18:19:31 +0100 Subject: [PATCH 3/5] fix failing unit test --- lib/AliasPlugin.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/AliasPlugin.js b/lib/AliasPlugin.js index 5b760fb6..94bfb252 100644 --- a/lib/AliasPlugin.js +++ b/lib/AliasPlugin.js @@ -102,7 +102,8 @@ module.exports = class AliasPlugin { target, item, resolveContext, - resolveCallback + resolveCallback, + false ); if (done) { return; @@ -143,7 +144,8 @@ function processAlias( target, item, resolveContext, - resolveCallback + resolveCallback, + shouldBail = true ) { if (alias === false) { /** @type {ResolveRequest} */ @@ -167,13 +169,18 @@ function processAlias( request: newRequestStr, fullySpecified: false }; + let done = false; if (!resolveCallback) { resolveCallback = (err, result) => { if (err || result) { + done = true; if (err) return callback(err); return callback(null, result); } - return callback(null, null); + // TODO: support also logic when this callback is called async + if (shouldBail) { + return callback(null, null); + } }; } resolver.doResolve( @@ -190,6 +197,6 @@ function processAlias( resolveCallback ); // the resolveCallback will take care of calling the callback - return true; + return shouldBail || done; } } From 24b3c604bfe87cb150da92ab08b9dee2ebdb8c96 Mon Sep 17 00:00:00 2001 From: Vincent Bailly Date: Tue, 10 Dec 2024 18:21:02 +0100 Subject: [PATCH 4/5] add TODO comment --- lib/AliasPlugin.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/AliasPlugin.js b/lib/AliasPlugin.js index 94bfb252..095f887d 100644 --- a/lib/AliasPlugin.js +++ b/lib/AliasPlugin.js @@ -91,7 +91,8 @@ module.exports = class AliasPlugin { * @returns {void} */ if (Array.isArray(item.alias)) { - for (const alias of item.alias) { + for (const alias of item.alias) { + // TODO: maybe shouldBail should be set to true for the last object? const done = processAlias( alias, request, From 44a260398aa930cfab73433df09814f8d9f72202 Mon Sep 17 00:00:00 2001 From: Vincent Bailly Date: Tue, 10 Dec 2024 18:29:00 +0100 Subject: [PATCH 5/5] add unit test to cover new scenario --- test/fallback.test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/fallback.test.js b/test/fallback.test.js index a6f71528..d5782417 100644 --- a/test/fallback.test.js +++ b/test/fallback.test.js @@ -8,7 +8,8 @@ describe("fallback", function () { const fileSystem = Volume.fromJSON( { "/a/index": "", - "/a/dir/index": "", + "/a/dir/index": "", + "/a/second/index": "", "/recursive/index": "", "/recursive/dir/index": "", "/recursive/dir/file": "", @@ -94,6 +95,11 @@ describe("fallback", function () { expect(resolver.resolveSync({}, "/", "multiAlias/anotherDir")).toBe( "/e/anotherDir/index" ); + // The multiAlias works as expected when the matching alias is the last one + expect(resolver.resolveSync({}, "/", "multiAlias/second")).toBe( + "/a/second/index" + ); + }); it("should log the correct info", done => { const log = [];