Skip to content

Commit

Permalink
fix: avoid import external module re-assigin in concatenation (#7839)
Browse files Browse the repository at this point in the history
* fix: avoid import external module re-assigin in concatenation

* test: add test
  • Loading branch information
fi3ework authored Sep 18, 2024
1 parent f108cfa commit 0a89e43
Show file tree
Hide file tree
Showing 14 changed files with 143 additions and 13 deletions.
15 changes: 15 additions & 0 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,21 @@ impl Module for ConcatenatedModule {
}
}

// Handle the name passed through by namespace_export_symbol
if let Some(ref namespace_export_symbol) = info.namespace_export_symbol {
if namespace_export_symbol.starts_with(NAMESPACE_OBJECT_EXPORT)
&& namespace_export_symbol.len() > NAMESPACE_OBJECT_EXPORT.len()
{
let name =
Atom::from(namespace_export_symbol[NAMESPACE_OBJECT_EXPORT.len()..].to_string());
all_used_names.insert(name.clone());
info
.internal_names
.insert(namespace_export_symbol.clone(), name.clone());
top_level_declarations.insert(name.clone());
}
}

// Handle namespaceObjectName for concatenated type
let namespace_object_name =
if let Some(ref namespace_export_symbol) = info.namespace_export_symbol {
Expand Down
23 changes: 16 additions & 7 deletions crates/rspack_core/src/external_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,22 @@ impl ExternalModule {
)
.boxed(),
);
format!(
r#"
{} = __WEBPACK_EXTERNAL_MODULE_{}__;
"#,
get_namespace_object_export(concatenation_scope, supports_const),
id.clone()
)

if let Some(concatenation_scope) = concatenation_scope {
let external_module_id = format!("__WEBPACK_EXTERNAL_MODULE_{}__", id);
let namespace_export_with_name =
format!("{}{}", NAMESPACE_OBJECT_EXPORT, &external_module_id);
concatenation_scope.register_namespace_export(&namespace_export_with_name);
String::new()
} else {
format!(
r#"
{} = __WEBPACK_EXTERNAL_MODULE_{}__;
"#,
get_namespace_object_export(concatenation_scope, supports_const),
id.clone()
)
}
} else {
format!(
"{} = {};",
Expand Down
2 changes: 1 addition & 1 deletion packages/rspack-test-tools/etc/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ export function parseModules(content: string, options?: {
};

// @public (undocumented)
export function readConfigFile<T extends ECompilerType>(files: string[]): TCompilerOptions<T>[];
export function readConfigFile<T extends ECompilerType>(files: string[], functionApply?: (config: (TCompilerOptions<T> | ((...args: unknown[]) => TCompilerOptions<T>))[]) => TCompilerOptions<T>[]): TCompilerOptions<T>[];

// @public (undocumented)
export function replaceModuleArgument(raw: string): string;
Expand Down
11 changes: 9 additions & 2 deletions packages/rspack-test-tools/src/helper/read-config-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@ import fs from "fs-extra";
import type { ECompilerType, TCompilerOptions } from "../type";

export function readConfigFile<T extends ECompilerType>(
files: string[]
files: string[],
functionApply?: (
config: (
| TCompilerOptions<T>
| ((...args: unknown[]) => TCompilerOptions<T>)
)[]
) => TCompilerOptions<T>[]
): TCompilerOptions<T>[] {
const existsFile = files.find(i => fs.existsSync(i));
const fileConfig = existsFile ? require(existsFile) : {};
return Array.isArray(fileConfig) ? fileConfig : [fileConfig];
const configArr = Array.isArray(fileConfig) ? fileConfig : [fileConfig];
return functionApply ? functionApply(configArr) : configArr;
}
16 changes: 15 additions & 1 deletion packages/rspack-test-tools/src/processor/multi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,21 @@ export class MultiTaskProcessor<T extends ECompilerType>
this._multiOptions.configFiles
)
? readConfigFile(
this._multiOptions.configFiles!.map(i => context.getSource(i))
this._multiOptions.configFiles!.map(i => context.getSource(i)),
configs => {
return configs.flatMap(c => {
if (typeof c === "function") {
const options = {
testPath: context.getDist(),
env: undefined
};

return c(options.env, options) as TCompilerOptions<T>;
}

return c as TCompilerOptions<T>;
});
}
)
: [{}];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const m1 = 11111;
export const m2 = 22222;

export const m1Add = () => m1++;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { m1, m2, m1Add } from "./externals/m.mjs";

export { m1, m2, m1Add }
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const rspack = require("@rspack/core");

/** @type {import("@rspack/core").Configuration} */
module.exports = {
externals: [/\.\/externals\/.*/],
externalsType: "module",
output: {
module: true,
chunkFormat: "module",
filename: "[name].mjs",
library: {
type: "modern-module",
}
},
experiments: {
outputModule: true
},
plugins: [
new rspack.CopyRspackPlugin({
patterns: ["./externals/**/*"],
}),
]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.noTests = true;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { m1Add } from 'library'

m1Add()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import fs from "fs";
import path from "path";

it("static import from external module should be tree shaking friendly", () => {
const content = fs.readFileSync(path.resolve(__dirname, "consume.mjs"), "utf-8");
expect(content).not.toContain("22222");
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const { rspack } = require("@rspack/core");
const path = require("path");

/** @type {function(any, any): import("@rspack/core").Configuration[]} */
module.exports = (env, { testPath }) => {
return {
entry: {
consume: "./consume.js",
main: "./index.js"
},
resolve: {
alias: {
library: path.resolve(testPath, "../0-concatenation-tree-shaking/main.mjs")
}
},
output: {
module: true,
chunkFormat: "module",
filename: "[name].mjs"
},
experiments: {
outputModule: true
},
optimization: {
minimize: true,
concatenateModules: true,
},
plugins: [
new rspack.SwcJsMinimizerRspackPlugin({
minimizerOptions: {
minify: false,
mangle: false
}
})
]
};
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/** @type {import("../../../../dist").TConfigCaseConfig} */
module.exports = {
findBundle: (i, options) => {
return ["main.mjs"];
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ module.exports = {
compilation.hooks.afterProcessAssets.tap("testcase", assets => {
const bundle = Object.values(assets)[0]._value
expect(bundle).toContain(`var __webpack_exports__cjsInterop = (foo_default());
var __webpack_exports__defaultImport = external_external_module_namespaceObject["default"];
var __webpack_exports__namedImport = external_external_module_namespaceObject.namedImport;`)
var __webpack_exports__defaultImport = __WEBPACK_EXTERNAL_MODULE_external_module__["default"];
var __webpack_exports__namedImport = __WEBPACK_EXTERNAL_MODULE_external_module__.namedImport;`)
});
};
this.hooks.compilation.tap("testcase", handler);
Expand Down

2 comments on commit 0a89e43

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-09-18 6f17ffa) Current Change
10000_development-mode + exec 2.21 s ± 26 ms 2.22 s ± 21 ms +0.27 %
10000_development-mode_hmr + exec 688 ms ± 16 ms 685 ms ± 12 ms -0.32 %
10000_production-mode + exec 2.83 s ± 40 ms 2.84 s ± 24 ms +0.16 %
arco-pro_development-mode + exec 1.82 s ± 69 ms 1.86 s ± 64 ms +2.35 %
arco-pro_development-mode_hmr + exec 433 ms ± 0.93 ms 434 ms ± 3.4 ms +0.36 %
arco-pro_production-mode + exec 3.25 s ± 66 ms 3.25 s ± 108 ms +0.19 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.31 s ± 93 ms 3.27 s ± 54 ms -1.16 %
threejs_development-mode_10x + exec 1.67 s ± 24 ms 1.67 s ± 18 ms -0.11 %
threejs_development-mode_10x_hmr + exec 781 ms ± 8.3 ms 790 ms ± 9.8 ms +1.15 %
threejs_production-mode_10x + exec 5.15 s ± 20 ms 5.15 s ± 25 ms +0.07 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rslib ❌ failure
rsbuild ❌ failure
examples ✅ success

Please sign in to comment.