Skip to content

Commit 1c59deb

Browse files
IWANABETHATGUYIWANABETHATGUY
authored
fix: 🐛 recursive visitor state (#4637)
* fix: 🐛 recursive visitor state * test: 💍 add testcase * chore: 🤖 clean * chore: 🤖 update check.js * chore: 🤖 recover
1 parent b7840ef commit 1c59deb

File tree

8 files changed

+216
-7
lines changed

8 files changed

+216
-7
lines changed

‎crates/rspack/tests/check.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ const falsePositiveMap = {
3131
"static-class": "align webpack unused binding behavior https://github.com/web-infra-dev/rspack/pull/4463/files",
3232
"webpack-inner-graph-export-default-named": "align webpack unused binding behavior https://github.com/web-infra-dev/rspack/pull/4463/files",
3333
"class-extend": "align webpack unused binding behavior https://github.com/web-infra-dev/rspack/pull/4481/files",
34-
"export_star": "same as webpack https://gist.github.com/IWANABETHATGUY/1ee8aa4c2889a9246d19d7be0ac75bb7"
34+
"export_star": "same as webpack https://gist.github.com/IWANABETHATGUY/1ee8aa4c2889a9246d19d7be0ac75bb7",
35+
"issue-4637": "align webpack https://github.com/web-infra-dev/rspack/pull/4637/files#diff-d434486532fb1507da93a26ce108dca465337b6af0ee86b4ab94fd788810d288"
3536
};
3637

3738
const normalizedList = filteredList.map((item) => {

‎crates/rspack/tests/tree-shaking/issue-4637/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import test from "./util.js";

‎crates/rspack/tests/tree-shaking/issue-4637/snapshot/new_treeshaking.snap

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
source: crates/rspack_testing/src/run_fixture.rs
3+
---
4+
```js title=main.js
5+
(self['webpackChunkwebpack'] = self['webpackChunkwebpack'] || []).push([["main"], {
6+
"./index.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
7+
"use strict";
8+
__webpack_require__.r(__webpack_exports__);
9+
/* harmony import */var _util_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/* ./util.js */"./util.js");
10+
11+
}),
12+
"./util.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
13+
"use strict";
14+
__webpack_require__.r(__webpack_exports__);
15+
/* harmony import */var _swc_helpers_create_class__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/* @swc/helpers/_/_create_class */"../../../../../node_modules/@swc/helpers/esm/_create_class.js");
16+
17+
var ConsoleExporterWeb;
18+
ConsoleExporterWeb = function() {
19+
"use strict";
20+
function ConsoleExporterWeb() {
21+
this.stoped = false;
22+
}
23+
(0, _swc_helpers_create_class__WEBPACK_IMPORTED_MODULE_0__._)(ConsoleExporterWeb, [
24+
{
25+
key: "export",
26+
value: function _export(evts, cb) {
27+
if (this.stoped) return;
28+
evts.forEach(adaptToBrowserConsole);
29+
if (cb) cb(ExportResult.SUCCESS);
30+
}
31+
},
32+
{
33+
key: "shutdown",
34+
value: function shutdown() {
35+
this.stoped = true;
36+
}
37+
}
38+
]);
39+
return ConsoleExporterWeb;
40+
}();
41+
var __WEBPACK_DEFAULT_EXPORT__ = (/* unused pure expression or super */ null && (ConsoleExporterWeb));
42+
}),
43+
"../../../../../node_modules/@swc/helpers/esm/_create_class.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
44+
"use strict";
45+
__webpack_require__.r(__webpack_exports__);
46+
__webpack_require__.d(__webpack_exports__, {
47+
_: function() { return _create_class; }
48+
});
49+
function _defineProperties(target, props) {
50+
for(var i = 0; i < props.length; i++){
51+
var descriptor = props[i];
52+
descriptor.enumerable = descriptor.enumerable || false;
53+
descriptor.configurable = true;
54+
if ("value" in descriptor) descriptor.writable = true;
55+
Object.defineProperty(target, descriptor.key, descriptor);
56+
}
57+
}
58+
function _create_class(Constructor, protoProps, staticProps) {
59+
if (protoProps) _defineProperties(Constructor.prototype, protoProps);
60+
if (staticProps) _defineProperties(Constructor, staticProps);
61+
return Constructor;
62+
}
63+
64+
}),
65+
66+
},function(__webpack_require__) {
67+
var __webpack_exec__ = function(moduleId) { return __webpack_require__(__webpack_require__.s = moduleId) }
68+
var __webpack_exports__ = (__webpack_exec__("./index.js"));
69+
70+
}
71+
]);
72+
```

‎crates/rspack/tests/tree-shaking/issue-4637/snapshot/output.snap

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
source: crates/rspack_testing/src/run_fixture.rs
3+
---
4+
```js title=main.js
5+
(self['webpackChunkwebpack'] = self['webpackChunkwebpack'] || []).push([["main"], {
6+
"./index.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
7+
"use strict";
8+
__webpack_require__.r(__webpack_exports__);
9+
/* harmony import */var _util_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/* ./util.js */"./util.js");
10+
11+
}),
12+
"./util.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
13+
"use strict";
14+
__webpack_require__.r(__webpack_exports__);
15+
/* harmony import */var _swc_helpers_create_class__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/* @swc/helpers/_/_create_class */"../../../../../node_modules/@swc/helpers/esm/_create_class.js");
16+
17+
var ConsoleExporterWeb;
18+
ConsoleExporterWeb = function() {
19+
"use strict";
20+
function ConsoleExporterWeb() {
21+
this.stoped = false;
22+
}
23+
(0, _swc_helpers_create_class__WEBPACK_IMPORTED_MODULE_0__._)(ConsoleExporterWeb, [
24+
{
25+
key: "export",
26+
value: function _export(evts, cb) {
27+
if (this.stoped) return;
28+
evts.forEach(adaptToBrowserConsole);
29+
if (cb) cb(ExportResult.SUCCESS);
30+
}
31+
},
32+
{
33+
key: "shutdown",
34+
value: function shutdown() {
35+
this.stoped = true;
36+
}
37+
}
38+
]);
39+
return ConsoleExporterWeb;
40+
}();
41+
var __WEBPACK_DEFAULT_EXPORT__ = ConsoleExporterWeb;
42+
}),
43+
"../../../../../node_modules/@swc/helpers/esm/_create_class.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
44+
"use strict";
45+
__webpack_require__.r(__webpack_exports__);
46+
__webpack_require__.d(__webpack_exports__, {
47+
_: function() { return _create_class; }
48+
});
49+
function _defineProperties(target, props) {
50+
for(var i = 0; i < props.length; i++){
51+
var descriptor = props[i];
52+
descriptor.enumerable = descriptor.enumerable || false;
53+
descriptor.configurable = true;
54+
if ("value" in descriptor) descriptor.writable = true;
55+
Object.defineProperty(target, descriptor.key, descriptor);
56+
}
57+
}
58+
function _create_class(Constructor, protoProps, staticProps) {
59+
if (protoProps) _defineProperties(Constructor.prototype, protoProps);
60+
if (staticProps) _defineProperties(Constructor, staticProps);
61+
return Constructor;
62+
}
63+
64+
}),
65+
66+
},function(__webpack_require__) {
67+
var __webpack_exec__ = function(moduleId) { return __webpack_require__(__webpack_require__.s = moduleId) }
68+
var __webpack_exports__ = (__webpack_exec__("./index.js"));
69+
70+
}
71+
]);
72+
```

‎crates/rspack/tests/tree-shaking/issue-4637/snapshot/snap.diff

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- expected
2+
+++ actual
3+
@@ -38,7 +38,7 @@
4+
]);
5+
return ConsoleExporterWeb;
6+
}();
7+
-var __WEBPACK_DEFAULT_EXPORT__ = ConsoleExporterWeb;
8+
+var __WEBPACK_DEFAULT_EXPORT__ = (/* unused pure expression or super */ null && (ConsoleExporterWeb));
9+
}),
10+
"../../../../../node_modules/@swc/helpers/esm/_create_class.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
11+
"use strict";

‎crates/rspack/tests/tree-shaking/issue-4637/test.config.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"optimization": {
3+
"sideEffects": "true"
4+
},
5+
"builtins": {
6+
"treeShaking": "true",
7+
"define": {
8+
"process.env.NODE_ENV": "'development'"
9+
}
10+
}
11+
}

‎crates/rspack/tests/tree-shaking/issue-4637/util.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import {_} from '@swc/helpers/_/_create_class'
2+
var ConsoleExporterWeb;
3+
ConsoleExporterWeb = function() {
4+
"use strict";
5+
function ConsoleExporterWeb() {
6+
this.stoped = false;
7+
}
8+
_(ConsoleExporterWeb, [
9+
{
10+
key: "export",
11+
value: function _export(evts, cb) {
12+
if (this.stoped) return;
13+
evts.forEach(adaptToBrowserConsole);
14+
if (cb) cb(ExportResult.SUCCESS);
15+
}
16+
},
17+
{
18+
key: "shutdown",
19+
value: function shutdown() {
20+
this.stoped = true;
21+
}
22+
}
23+
]);
24+
return ConsoleExporterWeb;
25+
}();
26+
27+
28+
29+
export default ConsoleExporterWeb;

‎crates/rspack_core/src/tree_shaking/visitor.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,10 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
860860
unimplemented!()
861861
}
862862
Decl::Var(var) => {
863-
self.state |= AnalyzeState::EXPORT_DECL;
863+
let pre_state = self.state;
864+
self.state.insert(AnalyzeState::EXPORT_DECL);
864865
var.visit_with(self);
865-
self.state.remove(AnalyzeState::EXPORT_DECL);
866+
self.state = pre_state;
866867
}
867868
Decl::TsInterface(_) | Decl::TsTypeAlias(_) | Decl::TsEnum(_) | Decl::TsModule(_) => {
868869
unreachable!("We have been converted Typescript to javascript already")
@@ -964,14 +965,18 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
964965
});
965966
}
966967

968+
let mut pre_state = self.state;
967969
self.state.insert(AnalyzeState::ASSIGNMENT_LHS);
968970
node.left.visit_with(self);
969-
self.state.remove(AnalyzeState::ASSIGNMENT_LHS);
971+
// cargo clippy told me to do this..
972+
std::mem::swap(&mut self.state, &mut pre_state);
973+
974+
let pre_state = self.state;
970975
if valid_assign_target {
971976
self.state.insert(AnalyzeState::ASSIGNMENT_RHS);
972977
}
973978
node.right.visit_with(self);
974-
self.state.remove(AnalyzeState::ASSIGNMENT_RHS);
979+
self.state = pre_state;
975980
self.current_body_owner_symbol_ext = before_owner_extend_symbol;
976981
}
977982

@@ -983,9 +988,10 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
983988
expr.visit_with(self);
984989
}
985990
_ => {
991+
let pre_state = self.state;
986992
self.state.insert(AnalyzeState::STATIC_VAR_DECL);
987993
expr.visit_with(self);
988-
self.state.remove(AnalyzeState::STATIC_VAR_DECL);
994+
self.state = pre_state;
989995
}
990996
}
991997
}
@@ -1027,6 +1033,7 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
10271033
}
10281034

10291035
fn visit_export_default_decl(&mut self, node: &ExportDefaultDecl) {
1036+
let pre_state = self.state;
10301037
self.state.insert(AnalyzeState::EXPORT_DEFAULT);
10311038
match &node.decl {
10321039
DefaultDecl::Class(_) | DefaultDecl::Fn(_) => {
@@ -1036,11 +1043,13 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
10361043
unreachable!("We have been converted Typescript to javascript already")
10371044
}
10381045
}
1039-
self.state.remove(AnalyzeState::EXPORT_DEFAULT);
1046+
self.state = pre_state;
10401047
}
10411048

10421049
fn visit_class_expr(&mut self, node: &ClassExpr) {
1050+
// TODO: handle
10431051
if self.state.contains(AnalyzeState::EXPORT_DEFAULT) {
1052+
let pre_state = self.state;
10441053
self.state.remove(AnalyzeState::EXPORT_DEFAULT);
10451054
let default_ident = self.generate_default_ident();
10461055
self.add_export(
@@ -1087,6 +1096,7 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
10871096
self.current_body_owner_symbol_ext = Some(body_owner_extend_symbol);
10881097
node.class.visit_with(self);
10891098
self.current_body_owner_symbol_ext = before_owner_extend_symbol;
1099+
self.state = pre_state;
10901100
} else {
10911101
// if the class expr is not inside a default expr, it will not
10921102
// generate a binding.
@@ -1142,6 +1152,7 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
11421152

11431153
fn visit_fn_expr(&mut self, node: &FnExpr) {
11441154
if self.state.contains(AnalyzeState::EXPORT_DEFAULT) {
1155+
let pre_state = self.state;
11451156
self.state.remove(AnalyzeState::EXPORT_DEFAULT);
11461157
let default_ident = self.generate_default_ident();
11471158
self.add_export(
@@ -1195,6 +1206,7 @@ impl<'a> Visit for ModuleRefAnalyze<'a> {
11951206
self.current_body_owner_symbol_ext = Some(body_owner_extend_symbol);
11961207
node.function.visit_with(self);
11971208
self.current_body_owner_symbol_ext = before_owner_extend_symbol;
1209+
self.state = pre_state;
11981210
} else {
11991211
// if the function expr is not inside a default expr, it will not
12001212
// generate a binding.

0 commit comments

Comments
 (0)