Skip to content

Commit 6205eea

Browse files
alexander-akaitalexander-akait
committed
fix(concatenate-modules): guard a module reference against ASI
A wrapped module's reference is emitted parenthesized, and the leading semicolon that keeps it off the previous statement is only written when the caller reports the position as unsafe. Both CommonJS call sites claimed the position was always safe, so in semicolon-free source the reference merged into the statement above it and threw at runtime. Compute `asiSafe` from where the original call sits instead, and apply it to the unused-export expression, which is parenthesized as well. Closes #21955 Closes #21956
1 parent a03c24a commit 6205eea

23 files changed

Lines changed: 1515 additions & 9 deletions

‎.changeset/017-concatenate-asi-position.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+
Keep a concatenated CommonJS reference from joining the previous statement.

‎lib/dependencies/CommonJsExportRequireDependency.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,9 @@ CommonJsExportRequireDependency.Template = class CommonJsExportRequireDependency
528528
: ids;
529529
requireExpr = concatenationScope.createModuleReference(importedModule, {
530530
ids: requestedIds.length > 0 ? requestedIds : undefined,
531-
asiSafe: true,
531+
// a used reexport keeps its assignment, so only an unused one is
532+
// left standing where the original statement began
533+
asiSafe: used ? true : dep.asiSafe,
532534
moduleExportsAccess: true
533535
});
534536
} else {

‎lib/dependencies/CommonJsExportsParserPlugin.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,9 @@ class CommonJsExportsParserPlugin {
479479
requireCall.ids,
480480
!parser.isStatementLevelExpression(expr)
481481
);
482+
dep.asiSafe = !parser.isAsiPosition(
483+
/** @type {Range} */ (expr.range)[0]
484+
);
482485
dep.loc = parser.getLocation(expr);
483486
dep.optional = Boolean(parser.scope.inTry);
484487
parser.state.module.addDependency(dep);
@@ -670,6 +673,9 @@ class CommonJsExportsParserPlugin {
670673
false
671674
);
672675
dep.getter = reexport.getter;
676+
dep.asiSafe = !parser.isAsiPosition(
677+
/** @type {Range} */ (expr.range)[0]
678+
);
673679
dep.loc = parser.getLocation(expr);
674680
dep.optional = Boolean(parser.scope.inTry);
675681
parser.state.module.addDependency(dep);

‎lib/dependencies/CommonJsImportsParserPlugin.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ const createRequireCallHandler = (
212212
/** @type {Range} */ (expr.range)
213213
);
214214
if (binding) binding.dep = dep;
215+
dep.asiSafe = !parser.isAsiPosition(/** @type {Range} */ (expr.range)[0]);
215216
dep.loc = parser.getLocation(expr);
216217
dep.optional = Boolean(parser.scope.inTry);
217218
parser.state.current.addDependency(dep);

‎lib/dependencies/CommonJsRequireDependency.js‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ const ModuleDependency = require("./ModuleDependency");
3636
/** @import { UsedByExports } from "../optimize/InnerGraph" */
3737
/** @import { RuntimeSpec } from "../util/runtime" */
3838
/** @import { DependencyGuard } from "./HarmonyImportGuard" */
39-
/** @typedef {import("../serialization/ObjectMiddleware").ObjectDeserializerContext<[RawReferencedExports | null, Range | undefined, DependencyGuard[] | undefined, boolean, boolean, UsedByExports | undefined]>} ObjectDeserializerContext */
40-
/** @typedef {import("../serialization/ObjectMiddleware").ObjectSerializerContext<[RawReferencedExports | null, Range | undefined, DependencyGuard[] | undefined, boolean, boolean, UsedByExports | undefined]>} ObjectSerializerContext */
39+
/** @typedef {import("../serialization/ObjectMiddleware").ObjectDeserializerContext<[RawReferencedExports | null, Range | undefined, DependencyGuard[] | undefined, boolean, boolean, UsedByExports | undefined, boolean | undefined]>} ObjectDeserializerContext */
40+
/** @typedef {import("../serialization/ObjectMiddleware").ObjectSerializerContext<[RawReferencedExports | null, Range | undefined, DependencyGuard[] | undefined, boolean, boolean, UsedByExports | undefined, boolean | undefined]>} ObjectSerializerContext */
4141

4242
/**
4343
* Trims a member-access path at the first segment that is definitively not a
@@ -93,6 +93,10 @@ class CommonJsRequireDependency extends ModuleDependency {
9393
// `new require(...)`; see the template for the return-value rule
9494
/** @type {boolean} */
9595
this.callNew = false;
96+
// whether the position the call sits at cannot be joined to what precedes
97+
// it by ASI; only meaningful once the call renders as a module reference
98+
/** @type {boolean | undefined} */
99+
this.asiSafe = undefined;
96100
}
97101

98102
get type() {
@@ -226,7 +230,8 @@ class CommonJsRequireDependency extends ModuleDependency {
226230
.write(this.branchGuards)
227231
.write(this._canConcatenate)
228232
.write(this.callNew)
229-
.write(this.usedByExports);
233+
.write(this.usedByExports)
234+
.write(this.asiSafe);
230235
super.serialize(context);
231236
}
232237

@@ -246,7 +251,9 @@ class CommonJsRequireDependency extends ModuleDependency {
246251
this.callNew = c4.read();
247252
const c5 = c4.rest;
248253
this.usedByExports = c5.read();
249-
super.deserialize(c5.rest);
254+
const c6 = c5.rest;
255+
this.asiSafe = c6.read();
256+
super.deserialize(c6.rest);
250257
}
251258
}
252259

@@ -303,7 +310,9 @@ CommonJsRequireDependency.Template = class CommonJsRequireDependencyTemplate ext
303310
ids: isRequireEsmModuleExportsModule(connection.module, moduleGraph)
304311
? [ESM_MODULE_EXPORTS_NAME]
305312
: undefined,
306-
asiSafe: true,
313+
// `new` wraps the reference in a call, so only a plain
314+
// replacement inherits the original call's position
315+
asiSafe: dep.callNew ? true : dep.asiSafe,
307316
moduleExportsAccess: true
308317
}
309318
);

‎lib/optimize/ConcatenatedModule.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,11 +423,12 @@ const getInteropExpr = (info, interopName) =>
423423
* slot, but a wrapped one only runs when its accessor is called, so dropping
424424
* the reference would drop the module's side effects with it.
425425
* @param {ConcatenatedModuleInfo} info module info
426+
* @param {boolean | undefined} asiSafe asiSafe
426427
* @returns {string} expression evaluating to undefined
427428
*/
428-
const getUnusedExportExpr = (info) =>
429+
const getUnusedExportExpr = (info, asiSafe) =>
429430
info.wrapped
430-
? `/* unused export */ (${info.namespaceObjectName}, undefined)`
431+
? `${asiSafe === false ? ";" : ""}/* unused export */ (${info.namespaceObjectName}, undefined)`
431432
: "/* unused export */ undefined";
432433

433434
/**
@@ -484,7 +485,7 @@ const getFinalBinding = (
484485
if (!usedName) {
485486
return {
486487
info,
487-
rawName: getUnusedExportExpr(info),
488+
rawName: getUnusedExportExpr(info, asiSafe),
488489
ids: [],
489490
exportName
490491
};

0 commit comments

Comments
 (0)