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

Fix resolving with yield #15423

Merged
merged 6 commits into from Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 16 additions & 6 deletions lib/ContextModule.js
Expand Up @@ -61,7 +61,7 @@ const makeSerializable = require("./util/makeSerializable");

/**
* @typedef {Object} ContextModuleOptionsExtras
* @property {string|string[]} resource
* @property {false|string|string[]} resource
* @property {string=} resourceQuery
* @property {string=} resourceFragment
* @property {TODO} resolveOptions
Expand Down Expand Up @@ -170,8 +170,9 @@ class ContextModule extends Module {
_createIdentifier() {
let identifier =
this.context ||
(typeof this.options.resource === "string"
? this.options.resource
(typeof this.options.resource === "string" ||
this.options.resource === false
? `${this.options.resource}`
: this.options.resource.join("|"));
if (this.options.resourceQuery) {
identifier += `|${this.options.resourceQuery}`;
Expand Down Expand Up @@ -240,8 +241,11 @@ class ContextModule extends Module {
let identifier;
if (this.context) {
identifier = requestShortener.shorten(this.context) + "/";
} else if (typeof this.options.resource === "string") {
identifier = requestShortener.shorten(this.options.resource) + "/";
} else if (
typeof this.options.resource === "string" ||
this.options.resource === false
) {
identifier = requestShortener.shorten(`${this.options.resource}`) + "/";
} else {
identifier = this.options.resource
.map(r => requestShortener.shorten(r) + "/")
Expand Down Expand Up @@ -310,6 +314,8 @@ class ContextModule extends Module {
this.options.resource,
options.associatedObjectForCache
);
} else if (this.options.resource === false) {
identifier = "false";
} else {
const arr = [];
for (const res of this.options.resource) {
Expand Down Expand Up @@ -484,14 +490,16 @@ class ContextModule extends Module {
);
return;
}
if (!this.context || !this.options.resource) return callback();

compilation.fileSystemInfo.createSnapshot(
startTime,
null,
this.context
? [this.context]
: typeof this.options.resource === "string"
? [this.options.resource]
: this.options.resource,
: /** @type {string[]} */ (this.options.resource),
null,
SNAPSHOT_OPTIONS,
(err, snapshot) => {
Expand Down Expand Up @@ -519,6 +527,8 @@ class ContextModule extends Module {
contextDependencies.add(this.context);
} else if (typeof this.options.resource === "string") {
contextDependencies.add(this.options.resource);
} else if (this.options.resource === false) {
return;
} else {
for (const res of this.options.resource) contextDependencies.add(res);
}
Expand Down
7 changes: 6 additions & 1 deletion lib/ContextModuleFactory.js
Expand Up @@ -217,7 +217,12 @@ module.exports = class ContextModuleFactory extends ModuleFactory {
contextDependencies
});
}
const [contextResult, loaderResult] = result;
let [contextResult, loaderResult] = result;
if (contextResult.length > 1) {
const first = contextResult[0];
contextResult = contextResult.filter(r => r.path);
if (contextResult.length === 0) contextResult.push(first);
}
this.hooks.afterResolve.callAsync(
{
addon:
Expand Down
21 changes: 15 additions & 6 deletions lib/cache/ResolverCachePlugin.js
Expand Up @@ -129,8 +129,10 @@ class ResolverCachePlugin {
contextDependencies: new LazySet()
};
let yieldResult;
let withYield = false;
if (typeof newResolveContext.yield === "function") {
yieldResult = [];
withYield = true;
newResolveContext.yield = obj => yieldResult.push(obj);
}
const propagate = key => {
Expand Down Expand Up @@ -160,7 +162,10 @@ class ResolverCachePlugin {
snapshotOptions,
(err, snapshot) => {
if (err) return callback(err);
const resolveResult = result || yieldResult;
const resolveResult = withYield ? yieldResult : result;
// since we intercept resolve hook
// we still can get result in callback
if (withYield && result) yieldResult.push(result);
if (!snapshot) {
if (resolveResult) return callback(null, resolveResult);
return callback();
Expand Down Expand Up @@ -242,11 +247,15 @@ class ResolverCachePlugin {
yields = undefined;
callbacks = false;
} else {
for (let i = 0; i < callbacks.length; i++) {
const cb = callbacks[i];
const yield_ = yields[i];
if (result) for (const r of result) yield_(r);
cb(null, null);
if (err) {
for (const cb of callbacks) cb(err);
} else {
for (let i = 0; i < callbacks.length; i++) {
const cb = callbacks[i];
const yield_ = yields[i];
if (result) for (const r of result) yield_(r);
cb(null, null);
}
}
activeRequestsWithYield.delete(identifier);
yields = undefined;
Expand Down
7 changes: 7 additions & 0 deletions test/configCases/resolve/empty-context-module/index.js
@@ -0,0 +1,7 @@
const id = () => Math.random();

it("should compile", () => {
expect(/* webpackMode: "lazy" */ import(`foo/${id()}`)).rejects.toBeTruthy();
expect(/* webpackMode: "lazy" */ import(`foo/${id()}`)).rejects.toBeTruthy();
expect(/* webpackMode: "lazy" */ import(`foo/${id()}`)).rejects.toBeTruthy();
vankop marked this conversation as resolved.
Show resolved Hide resolved
});
20 changes: 20 additions & 0 deletions test/configCases/resolve/empty-context-module/webpack.config.js
@@ -0,0 +1,20 @@
/** @type {import("../../../../").Configuration[]} */
module.exports = [
{
cache: true,
resolve: {
alias: {
foo: false
},
unsafeCache: true
}
},
{
resolve: {
alias: {
foo: false
},
unsafeCache: true
}
}
];