Skip to content

Commit 6ee759e

Browse files
alexander-akaitalexander-akait
committed
fix(defer): await an async dependency that is evaluating-async
`GatherAsynchronousTransitiveDependencies` skips a module whose status is `evaluating` or `evaluated`; `evaluating-async` is a distinct status and must still be awaited. The filter also skipped the latter, so a deferred import whose async dependency was suspended contributed nothing to wait on. Un-skips test262's async-cycle-dependency-of-deferred-module. `harmony-import-mixed` registered its tests from an async module, in a window the runner cannot await; the imports move to `entry.js` so registration is synchronous and the case stops depending on microtask timing.
1 parent f6b470a commit 6ee759e

5 files changed

Lines changed: 34 additions & 20 deletions

File tree

‎.changeset/014-defer-async-cycle-gather.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Await a deferred import's async dependency that is evaluating-async.

‎lib/runtime/MakeDeferredNamespaceObjectRuntime.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,13 +290,15 @@ class MakeOptimizedDeferredNamespaceObjectRuntimeModule extends HelperRuntimeMod
290290
]),
291291
"};",
292292
hasAsync
293-
? // `GatherAsynchronousTransitiveDependencies` skips a module that is
294-
// already evaluating, and evaluates the rest right here.
293+
? // `GatherAsynchronousTransitiveDependencies` skips a module whose
294+
// status is `evaluating` — on the stack, so awaiting it would
295+
// deadlock — but not one that is `evaluating-async`, which is
296+
// suspended and must still be awaited. It evaluates the rest here.
295297
`if(isAsync) obj[${RuntimeGlobals.deferredModuleAsyncTransitiveDependenciesSymbol}] = asyncDeps.filter(${runtimeTemplate.basicFunction(
296298
"id",
297299
[
298300
`${cst} dep = __webpack_module_cache__[id];`,
299-
"if (dep && (dep.evaluating || dep.evaluatingAsync)) return false;",
301+
"if (dep && dep.evaluating) return false;",
300302
"r(id);",
301303
"return true;"
302304
]

‎test/configCases/defer-import/harmony-import-mixed/entry.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { value as directValue } from "./shared.js"; // non-defer import
2+
import * as deferredShared from /* webpackDefer: true */ "./shared.js"; // defer import
3+
4+
import { value as directValueAsync } from "./shared-async.js"; // non-defer import
5+
import * as deferredSharedAsync from /* webpackDefer: true */ "./shared-async.js"; // defer import
6+
7+
export {
8+
directValue,
9+
deferredShared,
10+
directValueAsync,
11+
deferredSharedAsync
12+
};

‎test/configCases/defer-import/harmony-import-mixed/index.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,27 @@
1-
import { value as directValue } from "./shared.js"; // non-defer import
2-
import * as deferredShared from /* webpackDefer: true */ "./shared.js"; // defer import
1+
// The mixed defer/non-defer imports live in `entry.js` so that this module stays
2+
// synchronous: an async index registers its tests in a window the runner cannot
3+
// await, which makes the case race rather than test anything.
4+
it("should handle mixed defer/non-defer targets correctly", async () => {
5+
const { directValue, deferredShared } = await require("./entry.js");
36

4-
import { value as directValueAsync } from "./shared-async.js"; // non-defer import
5-
import * as deferredSharedAsync from /* webpackDefer: true */ "./shared-async.js"; // defer import
6-
7-
it("should handle mixed defer/non-defer targets correctly", () => {
8-
// Test direct non-defer access
97
expect(typeof directValue).toBe("string");
108
expect(directValue).toBe("shared-value");
11-
12-
// Test deferred access
9+
1310
expect(typeof deferredShared).toBe("object");
1411
expect(deferredShared).not.toBe(null);
1512
expect(deferredShared.value).toBe("shared-value");
16-
13+
1714
// Both should access the same underlying value
1815
expect(directValue).toBe(deferredShared.value);
1916
});
2017

21-
it("should handle mixed defer/non-defer targets with async correctly", () => {
22-
// Test direct non-defer access
18+
it("should handle mixed defer/non-defer targets with async correctly", async () => {
19+
const { directValueAsync, deferredSharedAsync } =
20+
await require("./entry.js");
21+
2322
expect(typeof directValueAsync).toBe("string");
2423
expect(directValueAsync).toBe("shared-value-async");
2524

26-
// Test deferred access
2725
expect(deferredSharedAsync).not.toBeInstanceOf(Promise);
2826
((m) => {
2927
expect(m.value).toBe("shared-value-async");

‎test/test262.spectest.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -894,10 +894,7 @@ const knownBugs = [
894894
// `#mark in obj` needs `isExtensible() === false` without evaluating, so the
895895
// target must be sealed with every export name on it, and a mismatch loses one.
896896
"import/import-defer/evaluation-triggers/ignore-private-name-access.js",
897-
// A deferred module must wait for the whole strongly-connected component when a
898-
// dependency's cycle root is still evaluating-async (`IsModuleSCCEvaluated`).
899-
"import/import-defer/evaluation-top-level-await/async-cycle-dependency-of-deferred-module/main.js",
900-
// It also wants `ownKeys` to be the sorted exports plus `@@toStringTag`, so
897+
// It wants `ownKeys` to be the sorted exports plus `@@toStringTag`, so
901898
// `__esModule` would have to go — the namespace trade-off, not a defer fix.
902899
"import/import-defer/deferred-namespace-object/exotic-object-behavior.js",
903900
// Improvement- bug with `delete` and `ns[0] = something` when using `import * as ns from "...";`

0 commit comments

Comments
 (0)