Visitar URL original
fix(parser): keep require() on the exports object inside a concatenation · webpack/webpack@3d161cb · GitHub
Skip to content

Commit 3d161cb

Browse files
alexander-akaitalexander-akait
committed
fix(parser): keep require() on the exports object inside a concatenation
Concatenation wrapped the shared external binding, so a CommonJS `require` in the same group was handed the spec namespace and lost `__esModule`, which a default-interop helper reads. The same sources built without concatenation gave it the exports object, so the option's behaviour depended on whether concatenation ran. Wrap at the reference instead, where `moduleExportsAccess` already marks a `require` apart from an import. Costs 3-5 gzip bytes on the cases that concatenate.
1 parent 6e490c2 commit 3d161cb

9 files changed

Lines changed: 84 additions & 64 deletions

File tree

‎lib/optimize/ConcatenatedModule.js‎

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ const findIdentifiers = (source) => {
248248
* @property {boolean} deferred the module is deferred at least once
249249
* @property {boolean} deferredNamespaceObjectUsed deferred namespace object that being used in a not-analyzable way so it must be materialized
250250
* @property {string | undefined} deferredNamespaceObjectName deferred namespace object that being used in a not-analyzable way so it must be materialized
251+
* @property {boolean} specNamespaceObjectUsed an import reads the spec namespace object the module hands out
251252
* @property {boolean} interopNamespaceObjectUsed "default-with-named" namespace
252253
* @property {string | undefined} interopNamespaceObjectName "default-with-named" namespace
253254
* @property {boolean} interopNamespaceObject2Used "default-only" namespace
@@ -669,7 +670,7 @@ const getFinalBinding = (
669670
exportName,
670671
accessorCall: info.wrapped
671672
};
672-
case "external":
673+
case "external": {
673674
if (deferred) {
674675
info.deferredNamespaceObjectUsed = true;
675676
return {
@@ -679,13 +680,30 @@ const getFinalBinding = (
679680
exportName
680681
};
681682
}
683+
684+
// `require()` reads the exports object rather than a namespace, so
685+
// only an import gets the spec object and interop keeps working.
686+
const spec =
687+
!moduleExportsAccess &&
688+
exportsType === "namespace" &&
689+
Boolean(
690+
/** @type {BuildInfo} */
691+
(info.module.buildInfo).specNamespaceObject
692+
);
693+
const rawName = getExternalModuleExpr(info);
694+
695+
if (spec) info.specNamespaceObjectUsed = true;
696+
682697
return {
683698
info,
684-
rawName: getExternalModuleExpr(info),
699+
rawName: spec
700+
? `${RuntimeGlobals.specNamespaceObject}(${rawName})`
701+
: rawName,
685702
ids: exportName,
686703
exportName,
687-
accessorCall: info.wrapped
704+
accessorCall: spec || info.wrapped
688705
};
706+
}
689707
}
690708
}
691709
const exportsInfo = moduleGraph.getExportsInfo(info.module);
@@ -2730,22 +2748,12 @@ ${defineGetters}`
27302748
runtimeRequirements
27312749
});
27322750
const moduleId = JSON.stringify(chunkGraph.getModuleId(info.module));
2733-
// Wrapping the accessor covers every reference at once, and the
2734-
// runtime caches per exports object so identity still holds.
2735-
let required = `${RuntimeGlobals.require}(${moduleId})`;
2751+
const required = `${RuntimeGlobals.require}(${moduleId})`;
27362752

2737-
if (
2738-
/** @type {BuildInfo} */
2739-
(info.module.buildInfo).specNamespaceObject &&
2740-
getExportsType(
2741-
moduleGraph,
2742-
info,
2743-
/** @type {BuildMeta} */
2744-
(this.rootModule.buildMeta).strictHarmonyModule
2745-
) === "namespace"
2746-
) {
2753+
// The wrapping happens per reference, so only the helper has to be
2754+
// declared here.
2755+
if (info.specNamespaceObjectUsed) {
27472756
runtimeRequirements.add(RuntimeGlobals.specNamespaceObject);
2748-
required = `${RuntimeGlobals.specNamespaceObject}(${required})`;
27492757
}
27502758

27512759
const body =
@@ -2917,22 +2925,12 @@ ${defineGetters}`
29172925
// External module bindings may be wrapped in
29182926
// runtime-condition `if` blocks but referenced outside,
29192927
// so they must remain function-scoped (`var`).
2920-
let required = `__webpack_require__(${moduleId})`;
2928+
const required = `__webpack_require__(${moduleId})`;
29212929

2922-
// The binding covers every reference at once, and the runtime
2923-
// caches per exports object so identity still holds.
2924-
if (
2925-
/** @type {BuildInfo} */
2926-
(info.module.buildInfo).specNamespaceObject &&
2927-
getExportsType(
2928-
moduleGraph,
2929-
info,
2930-
/** @type {BuildMeta} */
2931-
(this.rootModule.buildMeta).strictHarmonyModule
2932-
) === "namespace"
2933-
) {
2930+
// The wrapping happens per reference, so only the helper has
2931+
// to be declared here.
2932+
if (info.specNamespaceObjectUsed) {
29342933
runtimeRequirements.add(RuntimeGlobals.specNamespaceObject);
2935-
required = `${RuntimeGlobals.specNamespaceObject}(${required})`;
29362934
}
29372935

29382936
result.add(`var ${info.name} = ${required};`);
@@ -3243,6 +3241,7 @@ ${defineGetters}`
32433241
namespaceFnName: undefined,
32443242
escapeNamespaceObjectName: undefined,
32453243
deferredName: undefined,
3244+
specNamespaceObjectUsed: false,
32463245
interopNamespaceObjectUsed: false,
32473246
interopNamespaceObjectName: undefined,
32483247
interopNamespaceObject2Used: false,

‎test/configCases/spec-namespace-objects/concatenated-wrapped/index.js‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎test/configCases/spec-namespace-objects/concatenated-wrapped/inner.js‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎test/configCases/spec-namespace-objects/concatenated-wrapped/webpack.config.js‎

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎test/configCases/spec-namespace-objects/require-interop/index.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import * as ns from "./m.js";
2+
import { required } from "./inner.js";
3+
4+
it("should hand out a spec namespace object to an import", () => {
5+
expect(Object.getPrototypeOf(ns)).toBe(null);
6+
expect(Object.getOwnPropertyNames(ns)).toEqual(["a", "b", "default"]);
7+
expect(ns[Symbol.toStringTag]).toBe("Module");
8+
expect(Object.isExtensible(ns)).toBe(false);
9+
expect(ns.a).toBe(1);
10+
expect(ns.default).toBe(3);
11+
});
12+
13+
it("should hand a require() the exports object, not the namespace", () => {
14+
// `require` is not an import, so it keeps webpack's interop contract:
15+
// `__esModule` has to stay readable for a default-interop helper.
16+
const value = required();
17+
18+
expect(value).not.toBe(ns);
19+
expect(value.__esModule).toBe(true);
20+
expect(Object.getPrototypeOf(value)).toBe(Object.prototype);
21+
expect(value.default).toBe(3);
22+
});

‎test/configCases/spec-namespace-objects/require-interop/inner.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
"use strict";
2+
3+
// A strict CommonJS `require` is both what this case is about and what makes
4+
// the concatenation reach `m.js` through a lazy accessor.
5+
exports.required = function required() {
6+
return require("./m.js");
7+
};

test/configCases/spec-namespace-objects/concatenated-wrapped/m.js renamed to test/configCases/spec-namespace-objects/require-interop/m.js

File renamed without changes.

‎test/configCases/spec-namespace-objects/require-interop/webpack.config.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
"use strict";
2+
3+
/**
4+
* @param {string} name config name
5+
* @param {boolean} concatenateModules whether to enable module concatenation
6+
* @returns {import("../../../../").Configuration} config
7+
*/
8+
const config = (name, concatenateModules) => ({
9+
name,
10+
optimization: { concatenateModules },
11+
module: {
12+
rules: [{ test: /m\.js$/, parser: { specNamespaceObject: true } }]
13+
}
14+
});
15+
16+
/** @type {import("../../../../").Configuration[]} */
17+
module.exports = [
18+
config("require-interop without module concatenation", false),
19+
config("require-interop with module concatenation", true)
20+
];

‎types.d.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9043,6 +9043,11 @@ declare interface ExternalModuleInfo {
90439043
*/
90449044
deferredNamespaceObjectName?: string;
90459045

9046+
/**
9047+
* an import reads the spec namespace object the module hands out
9048+
*/
9049+
specNamespaceObjectUsed: boolean;
9050+
90469051
/**
90479052
* "default-with-named" namespace
90489053
*/

0 commit comments

Comments
 (0)