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: library module without export statement #18176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
36 changes: 35 additions & 1 deletion lib/FlagDependencyExportsPlugin.js
Expand Up @@ -15,11 +15,38 @@ const Queue = require("./util/Queue");
/** @typedef {import("./Dependency").ExportsSpec} ExportsSpec */
/** @typedef {import("./ExportsInfo")} ExportsInfo */
/** @typedef {import("./Module")} Module */
/** @typedef {import("./ModuleGraph")} ModuleGraph */
/** @typedef {import("../declarations/WebpackOptions").WebpackOptionsNormalized} WebpackOptions */

const PLUGIN_NAME = "FlagDependencyExportsPlugin";
const PLUGIN_LOGGER_NAME = `webpack.${PLUGIN_NAME}`;

class FlagDependencyExportsPlugin {
/**
* @param {WebpackOptions} webpackOptions webpack options
*/
constructor(webpackOptions) {
this._webpackOptions = webpackOptions;
}

/**
*
* @param {Module} module module
* @param {ModuleGraph} moduleGraph module graph
* @returns {boolean} need flag exportsInfo
*/
needFlagModuleExports(module, moduleGraph) {
const { optimization, output } = this._webpackOptions;
const { providedExports } = optimization;
const { library } = output;
if (providedExports === false && library && library.type === "module") {
// only flag the entry module
let connection = Array.from(moduleGraph.getIncomingConnections(module));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any other better way to determine whether it is an entry module during finishModules stage?

return connection.length === 1 && connection[0].originModule === null;
}
return providedExports;
}

/**
* Apply the plugin
* @param {Compiler} compiler the compiler instance
Expand All @@ -32,6 +59,9 @@ class FlagDependencyExportsPlugin {
compilation.hooks.finishModules.tapAsync(
PLUGIN_NAME,
(modules, callback) => {
const _modules = Array.from(modules).filter(module =>
this.needFlagModuleExports(module, moduleGraph)
);
const logger = compilation.getLogger(PLUGIN_LOGGER_NAME);
let statRestoredFromMemCache = 0;
let statRestoredFromCache = 0;
Expand All @@ -48,7 +78,7 @@ class FlagDependencyExportsPlugin {
// Step 1: Try to restore cached provided export info from cache
logger.time("restore cached provided exports");
asyncLib.each(
modules,
_modules,
(module, callback) => {
const exportsInfo = moduleGraph.getExportsInfo(module);
// If the module doesn't have an exportsType, it's a module
Expand Down Expand Up @@ -388,12 +418,16 @@ class FlagDependencyExportsPlugin {
/** @type {WeakMap<Module, any>} */
const providedExportsCache = new WeakMap();
compilation.hooks.rebuildModule.tap(PLUGIN_NAME, module => {
if (!this.needFlagModuleExports(module, compilation.moduleGraph))
return;
providedExportsCache.set(
module,
moduleGraph.getExportsInfo(module).getRestoreProvidedData()
);
});
compilation.hooks.finishRebuildingModule.tap(PLUGIN_NAME, module => {
if (!this.needFlagModuleExports(module, compilation.moduleGraph))
return;
moduleGraph
.getExportsInfo(module)
.restoreProvided(providedExportsCache.get(module));
Expand Down
9 changes: 7 additions & 2 deletions lib/WebpackOptionsApply.js
Expand Up @@ -425,9 +425,14 @@ class WebpackOptionsApply extends OptionsApply {
options.optimization.sideEffects === true
).apply(compiler);
}
if (options.optimization.providedExports) {
if (
options.optimization.providedExports ||
(options.optimization.providedExports === false &&
options.output.library &&
options.output.library.type === "module")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont' think we should rely on such things, we need to improve logic inside the plugin itself

Copy link
Contributor Author

@hai-x hai-x Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i think this would also destroy the meaning of the config, but 'libraryType: module' does rely on the module's exportsInfo, maybe we need to simply record export by sth like 'exportsInfo' for 'output: module'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any other suggestions? I'd be glad to read the related code and fix this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i think this would also destroy the meaning of the config, but 'output: module' does rely on the module's exportsInfo, maybe we need to simply record export by sth like 'exportsInfo' for 'output: module'.

And it seems that users rarely set 'providedExport' to false because of the performance issues of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look deeply later

) {
const FlagDependencyExportsPlugin = require("./FlagDependencyExportsPlugin");
new FlagDependencyExportsPlugin().apply(compiler);
new FlagDependencyExportsPlugin(options).apply(compiler);
}
if (options.optimization.usedExports) {
const FlagDependencyUsagePlugin = require("./FlagDependencyUsagePlugin");
Expand Down
9 changes: 9 additions & 0 deletions test/configCases/output-module/issue-18056/index.js
@@ -0,0 +1,9 @@
import React from 'react';
const m = true

export default 'issue-18056'
export { React, m }

it("should compile and run", () => {
});

6 changes: 6 additions & 0 deletions test/configCases/output-module/issue-18056/run.js
@@ -0,0 +1,6 @@
it("should compile and run", () => {
expect(issue18056.default).toBe('issue-18056');
expect(issue18056.m).toBe(true);
});


42 changes: 42 additions & 0 deletions test/configCases/output-module/issue-18056/webpack.config.js
@@ -0,0 +1,42 @@
var webpack = require("../../../../");

/** @type {import("../../../../").Configuration} */
module.exports = [
{
target: "node",
entry: {
import: "./index.js"
},
optimization: {
providedExports: false,
usedExports: true
},
output: {
libraryTarget: "module",
module: true,
chunkFormat: "module"
},
experiments: {
outputModule: true
},
externals: ["react"],
externalsType: "module"
},
{
entry: "./run.js",
plugins: [
new webpack.BannerPlugin({
raw: true,
banner:
"import mod, { m } from './bundle0.mjs';\nlet issue18056 = { default: mod, m };"
})
],
output: {
module: true,
chunkFormat: "module"
},
experiments: {
outputModule: true
}
}
];