Skip to content

Commit

Permalink
revert: fix: align amd option behavior with webpack (#9103)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Jan 23, 2025
1 parent 9df4956 commit 66c5e0d
Show file tree
Hide file tree
Showing 88 changed files with 218 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,6 @@ impl AMDDefineDependencyParserPlugin {
}
} else {
// define([…], …)
if !first_arg.expr.is_array() {
return None;
}

array = Some(&first_arg.expr);

if is_callable(&second_arg.expr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,26 @@ impl JavascriptParserPlugin for AMDParserPlugin {
None
}

fn identifier(
&self,
parser: &mut JavascriptParser,
ident: &swc_core::ecma::ast::Ident,
for_name: &str,
) -> Option<bool> {
if for_name == DEFINE {
parser
.presentational_dependencies
.push(Box::new(ConstDependency::new(
ident.span().real_lo(),
ident.span().real_hi(),
RuntimeGlobals::AMD_DEFINE.name().into(),
Some(RuntimeGlobals::AMD_DEFINE),
)));
return Some(true);
}
None
}

fn evaluate_identifier(
&self,
_parser: &mut JavascriptParser,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports.value = undefined;
module.exports = (newValue) => {
module.exports.value = newValue;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ it("require([...]) should work well", async function () {

await p;

expect(require('./fn')).toHaveBeenCalledWith(123);
expect(require('./fn').value).toBe(123);
});

it("require([...], function () {}) should work well", function (done) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
amd: {},
};

This file was deleted.

2 changes: 1 addition & 1 deletion packages/rspack/src/config/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const getRawOptions = (
experiments,
node: getRawNode(options.node),
profile: options.profile!,
amd: options.amd !== false ? JSON.stringify(options.amd || {}) : undefined,
amd: options.amd ? JSON.stringify(options.amd || {}) : undefined,
bail: options.bail!,
__references: {}
};
Expand Down
1 change: 1 addition & 0 deletions tests/webpack-test/ConfigTestCases.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const describeCases = config => {
}).not.toThrow();
optionsArr = [].concat(options);
optionsArr.forEach((options, idx) => {
if (options.amd === undefined) options.amd = {};
if (!options.context) options.context = testDirectory;
if (!options.mode) options.mode = "production";
if (!options.optimization) options.optimization = {};
Expand Down
1 change: 1 addition & 0 deletions tests/webpack-test/TestCases.template.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const describeCases = config => {
});
let options = {
...testConfig,
amd: {},
context: casesPath,
entry: "./" + category.name + "/" + testName + "/",
target: config.target || "async-node",
Expand Down
100 changes: 70 additions & 30 deletions tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -574,61 +574,101 @@ Rspack x.x.x compiled successfully in X.23"
exports[`StatsTestCases should print correct stats for limit-chunk-count-plugin 1`] = `
"1 chunks:
asset bundle1.js 3.86 KiB [emitted] (name: main)
chunk (runtime: main) bundle1.js (main) 219 bytes (javascript) 1.84 KiB (runtime) <{909}> >{909}< [entry] [rendered]
asset bundle1.js 4.04 KiB [emitted] (name: main)
chunk (runtime: main) bundle1.js (main) 357 bytes (javascript) 1.84 KiB (runtime) <{909}> >{909}< [entry] [rendered]
./a.js 22 bytes [dependent] [built] [code generated]
./b.js 22 bytes [dependent] [built] [code generated]
./c.js 30 bytes [dependent] [built] [code generated]
./d.js 22 bytes [dependent] [built] [code generated]
./e.js 22 bytes [dependent] [built] [code generated]
./index.js 101 bytes [built] [code generated]
1 chunks (Rspack x.x.x) compiled successfully in X.23
./index.js 101 bytes [built] [code generated] [1 warning]
Xdir/limit-chunk-count-plugin|sync 160 bytes [dependent] [built] [code generated]
WARNING in ./index.js
⚠ Critical dependency: the request of a dependency is an expression
╭─[2:8]
1 │ require.ensure([\\"./a\\"], function() {});
2 │ require([\\"./b\\"]);
· ────────
3 │ import(/* webpackChunkName: \\"c\\" */ \\"./c\\");
╰────
1 chunks (Rspack x.x.x) compiled with 1 warning in X.23
2 chunks:
asset bundle2.js 10.3 KiB [emitted] (name: main)
asset 76.bundle2.js 599 bytes [emitted] (name: c)
chunk (runtime: main) 76.bundle2.js (c) 118 bytes <{76}> <{909}> >{76}< [rendered]
asset bundle2.js 10.6 KiB [emitted] (name: main)
asset 76.bundle2.js 545 bytes [emitted] (name: c)
chunk (runtime: main) 76.bundle2.js (c) 96 bytes <{76}> <{909}> >{76}< [rendered]
dependent modules 44 bytes [dependent]
./d.js 22 bytes [dependent] [built] [code generated]
./e.js 22 bytes [dependent] [built] [code generated]
./a.js 22 bytes [built] [code generated]
./b.js 22 bytes [built] [code generated]
./c.js 30 bytes [built] [code generated]
chunk (runtime: main) bundle2.js (main) 101 bytes (javascript) 8.46 KiB (runtime) >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated]
2 chunks (Rspack x.x.x) compiled successfully in X.23
chunk (runtime: main) bundle2.js (main) 261 bytes (javascript) 8.46 KiB (runtime) >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated] [1 warning]
Xdir/limit-chunk-count-plugin|sync 160 bytes [dependent] [built] [code generated]
WARNING in ./index.js
⚠ Critical dependency: the request of a dependency is an expression
╭─[2:8]
1 │ require.ensure([\\"./a\\"], function() {});
2 │ require([\\"./b\\"]);
· ────────
3 │ import(/* webpackChunkName: \\"c\\" */ \\"./c\\");
╰────
2 chunks (Rspack x.x.x) compiled with 1 warning in X.23
3 chunks:
asset bundle3.js 10.3 KiB [emitted] (name: main)
asset bundle3.js 10.6 KiB [emitted] (name: main)
asset 76.bundle3.js 385 bytes [emitted] (name: c)
asset 656.bundle3.js 290 bytes [emitted]
chunk (runtime: main) 656.bundle3.js 88 bytes <{76}> <{909}> [rendered]
asset 588.bundle3.js 236 bytes [emitted]
chunk (runtime: main) 588.bundle3.js 66 bytes <{76}> <{909}> [rendered]
./a.js 22 bytes [built] [code generated]
./b.js 22 bytes [built] [code generated]
./d.js 22 bytes [built] [code generated]
./e.js 22 bytes [built] [code generated]
chunk (runtime: main) 76.bundle3.js (c) 30 bytes <{909}> >{656}< [rendered]
chunk (runtime: main) 76.bundle3.js (c) 30 bytes <{909}> >{588}< [rendered]
./c.js 30 bytes [built] [code generated]
chunk (runtime: main) bundle3.js (main) 101 bytes (javascript) 8.46 KiB (runtime) >{656}< >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated]
3 chunks (Rspack x.x.x) compiled successfully in X.23
chunk (runtime: main) bundle3.js (main) 261 bytes (javascript) 8.46 KiB (runtime) >{588}< >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated] [1 warning]
Xdir/limit-chunk-count-plugin|sync 160 bytes [dependent] [built] [code generated]
WARNING in ./index.js
⚠ Critical dependency: the request of a dependency is an expression
╭─[2:8]
1 │ require.ensure([\\"./a\\"], function() {});
2 │ require([\\"./b\\"]);
· ────────
3 │ import(/* webpackChunkName: \\"c\\" */ \\"./c\\");
╰────
3 chunks (Rspack x.x.x) compiled with 1 warning in X.23
4 chunks:
asset bundle4.js 10.3 KiB [emitted] (name: main)
asset bundle4.js 10.6 KiB [emitted] (name: main)
asset 76.bundle4.js 385 bytes [emitted] (name: c)
asset 537.bundle4.js 236 bytes [emitted]
asset 697.bundle4.js 128 bytes [emitted]
chunk (runtime: main) 537.bundle4.js 66 bytes <{76}> <{909}> [rendered]
asset 345.bundle4.js 182 bytes [emitted]
asset 272.bundle4.js 128 bytes [emitted]
chunk (runtime: main) 272.bundle4.js 22 bytes <{909}> [rendered]
./a.js 22 bytes [built] [code generated]
./b.js 22 bytes [built] [code generated]
chunk (runtime: main) 345.bundle4.js 44 bytes <{76}> [rendered]
./d.js 22 bytes [built] [code generated]
chunk (runtime: main) 697.bundle4.js 22 bytes <{76}> [rendered]
./e.js 22 bytes [built] [code generated]
chunk (runtime: main) 76.bundle4.js (c) 30 bytes <{909}> >{537}< >{697}< [rendered]
chunk (runtime: main) 76.bundle4.js (c) 30 bytes <{909}> >{345}< [rendered]
./c.js 30 bytes [built] [code generated]
chunk (runtime: main) bundle4.js (main) 101 bytes (javascript) 8.46 KiB (runtime) >{537}< >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated]
4 chunks (Rspack x.x.x) compiled successfully in X.23"
chunk (runtime: main) bundle4.js (main) 261 bytes (javascript) 8.46 KiB (runtime) >{272}< >{76}< [entry] [rendered]
./index.js 101 bytes [built] [code generated] [1 warning]
Xdir/limit-chunk-count-plugin|sync 160 bytes [dependent] [built] [code generated]
WARNING in ./index.js
⚠ Critical dependency: the request of a dependency is an expression
╭─[2:8]
1 │ require.ensure([\\"./a\\"], function() {});
2 │ require([\\"./b\\"]);
· ────────
3 │ import(/* webpackChunkName: \\"c\\" */ \\"./c\\");
╰────
4 chunks (Rspack x.x.x) compiled with 1 warning in X.23"
`;
exports[`StatsTestCases should print correct stats for logging-debug 1`] = `
Expand Down

This file was deleted.

4 changes: 0 additions & 4 deletions tests/webpack-test/cases/amd/namedModules/test.filter.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4313"}
module.exports = () => {return "compile time eval split"}


8 changes: 4 additions & 4 deletions tests/webpack-test/cases/cjs-tree-shaking/bailouts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ it("should be able to do stuff with the module object", () => {
expect(require("./accessing-module?1").def).toBe("def");
});

// it("should be able to use AMD to define exports", () => {
// expect(require("./using-amd?2").abc).toBe("abc");
// expect(require("./using-amd?1").def).toBe("def");
// });
it("should be able to use AMD to define exports", () => {
expect(require("./using-amd?2").abc).toBe("abc");
expect(require("./using-amd?1").def).toBe("def");
});

This file was deleted.

4 changes: 0 additions & 4 deletions tests/webpack-test/cases/parsing/amd-rename/test.filter.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = function (config) {
module.exports = () => {
return [
FilteredStatus.PARTIAL_PASS,
"https://github.com/web-infra-dev/rspack/issues/4313",
"amd require context",
"require(String.raw``)"
];
};
4 changes: 0 additions & 4 deletions tests/webpack-test/cases/parsing/declared-api/test.filter.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ module.exports = function (config) {
};
*/
module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4313"}
module.exports = () => {return "amd require context"}


Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4313"}
module.exports = () => {return "amd require context"}


Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4313"}
module.exports = () => {return "__webpack_amd_options__ module variable"}


4 changes: 0 additions & 4 deletions tests/webpack-test/cases/parsing/issue-2084/test.filter.js

This file was deleted.

22 changes: 11 additions & 11 deletions tests/webpack-test/cases/parsing/issue-2641/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ it("should call error callback on missing module", function(done) {
});
});

it("should call error callback on missing module in context", function(done) {
(function(module) {
require(['./' + module], function(file){}, function(error) {
try {
expect(error).toBeInstanceOf(Error);
expect(error.message).toBe("Cannot find module './missingModule'");
done();
} catch(e) { done(e); }
});
})('missingModule');
});
// it("should call error callback on missing module in context", function(done) {
// (function(module) {
// require(['./' + module], function(file){}, function(error) {
// try {
// expect(error).toBeInstanceOf(Error);
// expect(error.message).toBe("Cannot find module './missingModule'");
// done();
// } catch(e) { done(e); }
// });
// })('missingModule');
// });

it("should call error callback on exception thrown in loading module", function(done) {
require(['./throwing'], function(){}, function(error) {
Expand Down
2 changes: 1 addition & 1 deletion tests/webpack-test/cases/parsing/issue-2641/test.filter.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4313"}
module.exports = () => {return "error callback on missing module in context"}


Loading

2 comments on commit 66c5e0d

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 66c5e0d Jan 23, 2025

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 (2025-01-21 c06787b) Current Change
10000_big_production-mode_disable-minimize + exec 36.7 s ± 515 ms 38.9 s ± 1.12 s +5.90 %
10000_development-mode + exec 1.86 s ± 52 ms 1.85 s ± 96 ms -0.77 %
10000_development-mode_hmr + exec 681 ms ± 9.8 ms 687 ms ± 27 ms +0.86 %
10000_production-mode + exec 2.45 s ± 138 ms 2.38 s ± 57 ms -2.62 %
10000_production-mode_persistent-cold + exec 2.61 s ± 155 ms 2.56 s ± 83 ms -2.03 %
10000_production-mode_persistent-hot + exec 1.77 s ± 116 ms 1.77 s ± 81 ms +0.06 %
arco-pro_development-mode + exec 1.76 s ± 98 ms 1.8 s ± 164 ms +2.16 %
arco-pro_development-mode_hmr + exec 388 ms ± 2.3 ms 387 ms ± 1.2 ms -0.10 %
arco-pro_production-mode + exec 3.64 s ± 257 ms 3.74 s ± 313 ms +2.82 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.75 s ± 314 ms 3.7 s ± 106 ms -1.31 %
arco-pro_production-mode_persistent-cold + exec 3.8 s ± 165 ms 3.81 s ± 171 ms +0.37 %
arco-pro_production-mode_persistent-hot + exec 2.34 s ± 79 ms 2.57 s ± 94 ms +9.99 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.67 s ± 99 ms 3.64 s ± 57 ms -0.62 %
large-dyn-imports_development-mode + exec 2.11 s ± 29 ms 2.1 s ± 73 ms -0.30 %
large-dyn-imports_production-mode + exec 2.16 s ± 40 ms 2.15 s ± 25 ms -0.33 %
threejs_development-mode_10x + exec 1.53 s ± 22 ms 1.54 s ± 27 ms +0.85 %
threejs_development-mode_10x_hmr + exec 788 ms ± 39 ms 789 ms ± 5.4 ms +0.06 %
threejs_production-mode_10x + exec 5.25 s ± 61 ms 5.47 s ± 252 ms +4.17 %
threejs_production-mode_10x_persistent-cold + exec 5.35 s ± 65 ms 5.58 s ± 280 ms +4.29 %
threejs_production-mode_10x_persistent-hot + exec 4.5 s ± 44 ms 4.81 s ± 310 ms +6.84 %
10000_big_production-mode_disable-minimize + rss memory 8680 MiB ± 34.2 MiB 8717 MiB ± 54 MiB +0.42 %
10000_development-mode + rss memory 656 MiB ± 19.6 MiB 662 MiB ± 28.1 MiB +0.92 %
10000_development-mode_hmr + rss memory 1264 MiB ± 163 MiB 1281 MiB ± 253 MiB +1.34 %
10000_production-mode + rss memory 639 MiB ± 31 MiB 641 MiB ± 15.7 MiB +0.26 %
10000_production-mode_persistent-cold + rss memory 744 MiB ± 15.6 MiB 744 MiB ± 23.5 MiB -0.04 %
10000_production-mode_persistent-hot + rss memory 723 MiB ± 37.7 MiB 730 MiB ± 12.4 MiB +0.98 %
arco-pro_development-mode + rss memory 553 MiB ± 38.1 MiB 559 MiB ± 31.1 MiB +1.06 %
arco-pro_development-mode_hmr + rss memory 631 MiB ± 36 MiB 671 MiB ± 21.5 MiB +6.29 %
arco-pro_production-mode + rss memory 738 MiB ± 66.3 MiB 712 MiB ± 25.7 MiB -3.49 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 730 MiB ± 23.1 MiB 718 MiB ± 24.6 MiB -1.62 %
arco-pro_production-mode_persistent-cold + rss memory 844 MiB ± 36.6 MiB 831 MiB ± 47 MiB -1.48 %
arco-pro_production-mode_persistent-hot + rss memory 695 MiB ± 19.9 MiB 701 MiB ± 14.7 MiB +0.86 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 727 MiB ± 32.5 MiB 725 MiB ± 44.3 MiB -0.26 %
large-dyn-imports_development-mode + rss memory 642 MiB ± 4.5 MiB 640 MiB ± 8.09 MiB -0.23 %
large-dyn-imports_production-mode + rss memory 523 MiB ± 4.79 MiB 523 MiB ± 3.46 MiB -0.03 %
threejs_development-mode_10x + rss memory 551 MiB ± 32.9 MiB 540 MiB ± 21.9 MiB -1.85 %
threejs_development-mode_10x_hmr + rss memory 1145 MiB ± 125 MiB 1130 MiB ± 90.2 MiB -1.29 %
threejs_production-mode_10x + rss memory 865 MiB ± 36.9 MiB 822 MiB ± 28.7 MiB -5.02 %
threejs_production-mode_10x_persistent-cold + rss memory 968 MiB ± 136 MiB 939 MiB ± 66.8 MiB -2.95 %
threejs_production-mode_10x_persistent-hot + rss memory 870 MiB ± 25.8 MiB 854 MiB ± 28.1 MiB -1.80 %

Threshold exceeded: ["10000_big_production-mode_disable-minimize + exec","arco-pro_production-mode_persistent-hot + exec","threejs_production-mode_10x_persistent-hot + exec"]

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 66c5e0d Jan 23, 2025

Choose a reason for hiding this comment

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

📝 Ecosystem CI detail: Open

suite result
modernjs ❌ failure
rspress ✅ success
rslib ❌ failure
rsbuild ✅ success
rsdoctor ✅ success
examples ✅ success
devserver ✅ success
nuxt ✅ success

Please sign in to comment.