Skip to content

Commit f46a03c

Browse files
alexander-akaitalexander-akait
authored
revert: do not use heuristic fallback for "module-import"
2 parents 8bf907b + 60f1898 commit f46a03c

File tree

11 files changed

+158
-151
lines changed

11 files changed

+158
-151
lines changed

‎lib/ExternalModule.js‎

Lines changed: 92 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const propertyAccess = require("./util/propertyAccess");
2222
const { register } = require("./util/serialization");
2323

2424
/** @typedef {import("webpack-sources").Source} Source */
25-
/** @typedef {import("../declarations/WebpackOptions").ExternalsPresets} ExternalsPresets */
2625
/** @typedef {import("../declarations/WebpackOptions").WebpackOptionsNormalized} WebpackOptions */
2726
/** @typedef {import("./Chunk")} Chunk */
2827
/** @typedef {import("./ChunkGraph")} ChunkGraph */
@@ -54,7 +53,7 @@ const { register } = require("./util/serialization");
5453
/** @typedef {import("./util/fs").InputFileSystem} InputFileSystem */
5554
/** @typedef {import("./util/runtime").RuntimeSpec} RuntimeSpec */
5655

57-
/** @typedef {{ attributes?: ImportAttributes, externalType: "import" | "module" | undefined, externalsPresets: ExternalsPresets | undefined }} ImportDependencyMeta */
56+
/** @typedef {{ attributes?: ImportAttributes, externalType: "import" | "module" | undefined }} ImportDependencyMeta */
5857
/** @typedef {{ layer?: string, supports?: string, media?: string }} CssImportDependencyMeta */
5958

6059
/** @typedef {ImportDependencyMeta | CssImportDependencyMeta} DependencyMeta */
@@ -547,6 +546,25 @@ class ExternalModule extends Module {
547546
return callback(null, !this.buildMeta);
548547
}
549548

549+
/**
550+
* @param {string} externalType raw external type
551+
* @returns {string} resolved external type
552+
*/
553+
getModuleImportType(externalType) {
554+
if (externalType === "module-import") {
555+
if (
556+
this.dependencyMeta &&
557+
/** @type {ImportDependencyMeta} */ (this.dependencyMeta).externalType
558+
) {
559+
return /** @type {ImportDependencyMeta} */ (this.dependencyMeta)
560+
.externalType;
561+
}
562+
return "module";
563+
}
564+
565+
return externalType;
566+
}
567+
550568
/**
551569
* @param {WebpackOptions} options webpack options
552570
* @param {Compilation} compilation the compilation
@@ -579,25 +597,6 @@ class ExternalModule extends Module {
579597
canMangle = true;
580598
}
581599
break;
582-
case "module":
583-
if (this.buildInfo.module) {
584-
if (!Array.isArray(request) || request.length === 1) {
585-
this.buildMeta.exportsType = "namespace";
586-
canMangle = true;
587-
}
588-
} else {
589-
this.buildMeta.async = true;
590-
EnvironmentNotSupportAsyncWarning.check(
591-
this,
592-
compilation.runtimeTemplate,
593-
"external module"
594-
);
595-
if (!Array.isArray(request) || request.length === 1) {
596-
this.buildMeta.exportsType = "namespace";
597-
canMangle = false;
598-
}
599-
}
600-
break;
601600
case "script":
602601
this.buildMeta.async = true;
603602
EnvironmentNotSupportAsyncWarning.check(
@@ -614,18 +613,45 @@ class ExternalModule extends Module {
614613
"external promise"
615614
);
616615
break;
616+
case "module":
617617
case "import":
618-
this.buildMeta.async = true;
619-
EnvironmentNotSupportAsyncWarning.check(
620-
this,
621-
compilation.runtimeTemplate,
622-
"external import"
623-
);
624-
if (!Array.isArray(request) || request.length === 1) {
625-
this.buildMeta.exportsType = "namespace";
626-
canMangle = false;
618+
case "module-import": {
619+
const type = this.getModuleImportType(externalType);
620+
if (type === "module") {
621+
if (this.buildInfo.module) {
622+
if (!Array.isArray(request) || request.length === 1) {
623+
this.buildMeta.exportsType = "namespace";
624+
canMangle = true;
625+
}
626+
} else {
627+
this.buildMeta.async = true;
628+
EnvironmentNotSupportAsyncWarning.check(
629+
this,
630+
compilation.runtimeTemplate,
631+
"external module"
632+
);
633+
if (!Array.isArray(request) || request.length === 1) {
634+
this.buildMeta.exportsType = "namespace";
635+
canMangle = false;
636+
}
637+
}
627638
}
639+
640+
if (type === "import") {
641+
this.buildMeta.async = true;
642+
EnvironmentNotSupportAsyncWarning.check(
643+
this,
644+
compilation.runtimeTemplate,
645+
"external import"
646+
);
647+
if (!Array.isArray(request) || request.length === 1) {
648+
this.buildMeta.exportsType = "namespace";
649+
canMangle = false;
650+
}
651+
}
652+
628653
break;
654+
}
629655
}
630656
this.addDependency(new StaticExportsDependency(true, canMangle));
631657
callback();
@@ -659,36 +685,6 @@ class ExternalModule extends Module {
659685

660686
_getRequestAndExternalType() {
661687
let { request, externalType } = this;
662-
663-
if (externalType === "module-import") {
664-
const dependencyMeta = /** @type {ImportDependencyMeta} */ (
665-
this.dependencyMeta
666-
);
667-
668-
if (dependencyMeta && dependencyMeta.externalType) {
669-
externalType = dependencyMeta.externalType;
670-
} else if (dependencyMeta && dependencyMeta.externalsPresets) {
671-
const presets = dependencyMeta.externalsPresets;
672-
// TODO: what if user set multiple presets?
673-
if (presets.web) {
674-
externalType = "module";
675-
} else if (presets.webAsync) {
676-
externalType = "import";
677-
} else if (
678-
presets.electron ||
679-
presets.electronMain ||
680-
presets.electronPreload ||
681-
presets.electronRenderer ||
682-
presets.node ||
683-
presets.nwjs
684-
) {
685-
externalType = "node-commonjs";
686-
}
687-
} else {
688-
externalType = "commonjs";
689-
}
690-
}
691-
692688
if (typeof request === "object" && !Array.isArray(request))
693689
request = request[externalType];
694690
return { request, externalType };
@@ -753,43 +749,52 @@ class ExternalModule extends Module {
753749
runtimeTemplate
754750
);
755751
}
756-
case "import":
757-
return getSourceForImportExternal(
758-
request,
759-
runtimeTemplate,
760-
/** @type {ImportDependencyMeta} */ (dependencyMeta)
761-
);
762752
case "script":
763753
return getSourceForScriptExternal(request, runtimeTemplate);
764-
case "module": {
765-
if (!(/** @type {BuildInfo} */ (this.buildInfo).module)) {
766-
if (!runtimeTemplate.supportsDynamicImport()) {
767-
throw new Error(
768-
`The target environment doesn't support dynamic import() syntax so it's not possible to use external type 'module' within a script${
769-
runtimeTemplate.supportsEcmaScriptModuleSyntax()
770-
? "\nDid you mean to build a EcmaScript Module ('output.module: true')?"
771-
: ""
772-
}`
773-
);
774-
}
754+
case "module":
755+
case "import":
756+
case "module-import": {
757+
const type = this.getModuleImportType(externalType);
758+
if (type === "import") {
775759
return getSourceForImportExternal(
776760
request,
777761
runtimeTemplate,
778762
/** @type {ImportDependencyMeta} */ (dependencyMeta)
779763
);
780764
}
781-
if (!runtimeTemplate.supportsEcmaScriptModuleSyntax()) {
782-
throw new Error(
783-
"The target environment doesn't support EcmaScriptModule syntax so it's not possible to use external type 'module'"
765+
766+
if (type === "module") {
767+
if (!(/** @type {BuildInfo} */ (this.buildInfo).module)) {
768+
if (!runtimeTemplate.supportsDynamicImport()) {
769+
throw new Error(
770+
`The target environment doesn't support dynamic import() syntax so it's not possible to use external type 'module' within a script${
771+
runtimeTemplate.supportsEcmaScriptModuleSyntax()
772+
? "\nDid you mean to build a EcmaScript Module ('output.module: true')?"
773+
: ""
774+
}`
775+
);
776+
}
777+
return getSourceForImportExternal(
778+
request,
779+
runtimeTemplate,
780+
/** @type {ImportDependencyMeta} */ (dependencyMeta)
781+
);
782+
}
783+
if (!runtimeTemplate.supportsEcmaScriptModuleSyntax()) {
784+
throw new Error(
785+
"The target environment doesn't support EcmaScriptModule syntax so it's not possible to use external type 'module'"
786+
);
787+
}
788+
return getSourceForModuleExternal(
789+
request,
790+
moduleGraph.getExportsInfo(this),
791+
runtime,
792+
runtimeTemplate,
793+
/** @type {ImportDependencyMeta} */ (dependencyMeta)
784794
);
785795
}
786-
return getSourceForModuleExternal(
787-
request,
788-
moduleGraph.getExportsInfo(this),
789-
runtime,
790-
runtimeTemplate,
791-
/** @type {ImportDependencyMeta} */ (dependencyMeta)
792-
);
796+
797+
break;
793798
}
794799
case "var":
795800
case "promise":

‎lib/ExternalModuleFactoryPlugin.js‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ const ImportDependency = require("./dependencies/ImportDependency");
1414
const { resolveByProperty, cachedSetProperty } = require("./util/cleverMerge");
1515

1616
/** @typedef {import("../declarations/WebpackOptions").Externals} Externals */
17-
/** @typedef {import("../declarations/WebpackOptions").ExternalsPresets} ExternalsPresets */
1817
/** @typedef {import("./Compilation").DepConstructor} DepConstructor */
1918
/** @typedef {import("./ExternalModule").DependencyMeta} DependencyMeta */
2019
/** @typedef {import("./Module")} Module */
@@ -61,12 +60,10 @@ class ExternalModuleFactoryPlugin {
6160
/**
6261
* @param {string | undefined} type default external type
6362
* @param {Externals} externals externals config
64-
* @param {ExternalsPresets} [externalsPresets] externals preset config
6563
*/
66-
constructor(type, externals, externalsPresets) {
64+
constructor(type, externals) {
6765
this.type = type;
6866
this.externals = externals;
69-
this.externalsPresets = externalsPresets;
7067
}
7168

7269
/**
@@ -75,7 +72,6 @@ class ExternalModuleFactoryPlugin {
7572
*/
7673
apply(normalModuleFactory) {
7774
const globalType = this.type;
78-
const externalsPresets = this.externalsPresets;
7975
normalModuleFactory.hooks.factorize.tapAsync(
8076
"ExternalModuleFactoryPlugin",
8177
(data, callback) => {
@@ -139,21 +135,14 @@ class ExternalModuleFactoryPlugin {
139135

140136
dependencyMeta = {
141137
attributes: dependency.assertions,
142-
externalType,
143-
externalsPresets
138+
externalType
144139
};
145140
} else if (dependency instanceof CssImportDependency) {
146141
dependencyMeta = {
147142
layer: dependency.layer,
148143
supports: dependency.supports,
149144
media: dependency.media
150145
};
151-
} else {
152-
dependencyMeta = {
153-
attributes: undefined,
154-
externalType: undefined,
155-
externalsPresets
156-
};
157146
}
158147

159148
callback(

‎lib/ExternalsPlugin.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,9 @@ class ExternalsPlugin {
2727
*/
2828
apply(compiler) {
2929
compiler.hooks.compile.tap("ExternalsPlugin", ({ normalModuleFactory }) => {
30-
new ExternalModuleFactoryPlugin(
31-
this.type,
32-
this.externals,
33-
compiler.options.externalsPresets
34-
).apply(normalModuleFactory);
30+
new ExternalModuleFactoryPlugin(this.type, this.externals).apply(
31+
normalModuleFactory
32+
);
3533
});
3634
}
3735
}

‎test/configCases/externals/module-import-externals-presets/index.js‎

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

‎test/configCases/externals/module-import-externals-presets/test.filter.js‎

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

‎test/configCases/externals/module-import-externals-presets/webpack.config.js‎

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

‎test/configCases/externals/module-import/a.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import external0 from "external0"; // module
2+
const external1 = require("external1"); // module
3+
const external2 = require("external2"); // node-commonjs
4+
const external3 = import("external3"); // import
5+
6+
console.log(external0, external1, external2, external3);

‎test/configCases/externals/module-import/index.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
const fs = require("fs");
2+
const path = require("path");
3+
4+
it("module-import should correctly get fallback type", function() {
5+
const content = fs.readFileSync(path.resolve(__dirname, "a.js"), "utf-8");
6+
expect(content).toContain(`import * as __WEBPACK_EXTERNAL_MODULE_external0__ from "external0";`); // module
7+
expect(content).toContain(`import * as __WEBPACK_EXTERNAL_MODULE_external1__ from "external1";`); // module
8+
expect(content).toContain(`module.exports = __WEBPACK_EXTERNAL_createRequire(import.meta.url)("external2");`); // node-commonjs
9+
expect(content).toContain(`module.exports = import("external3");`); // import
10+
});

‎test/configCases/externals/module-import/test.config.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
findBundle: (i, options) => ["main.js"]
3+
};

0 commit comments

Comments
 (0)