fix: replace the "typeof __webpack_require__" content #1688
Conversation
Walkthrough此次变更引入了一个新的访问者 Changes
Assessment against linked issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
crates/mako/src/visitors/webpack_require.rs (4)
5-20: 建议对常量进行分组以提高可维护性建议将相关的 webpack 常量按功能进行分组，例如：
- 核心常量（如
WEBPACK_REQUIRE、WEBPACK_MODULES）- 运行时常量（如
WEBPACK_HASH、WEBPACK_RUNTIME_ID）- 共享作用域常量（如
WEBPACK_SHARE_SCOPES、WEBPACK_INIT_SHARING）这样可以提高代码的可读性和维护性。
22-53: 建议添加文档注释为了提高代码的可维护性和可读性，建议为以下项添加文档注释：
WebpackRequire结构体的用途和职责new方法的参数说明is_ident_webpack方法的功能说明、参数和返回值解释示例文档注释：
/// 处理 webpack 相关标识符的 AST 访问器 pub struct WebpackRequire { /// 未解析标记，用于上下文匹配 pub unresolved_mark: Mark, }
55-65: 建议完善注释说明转换逻辑当前的注释只说明了要查找 "typeof webpack_require"，建议补充：
- 为什么要进行这种转换
- 转换的具体规则
- 转换后的预期效果
建议添加如下注释：
/// 访问并转换 AST 中的一元表达式 /// /// 将 `typeof __webpack_require__` 等形式的表达式转换为 `typeof void 0`， /// 以避免 webpack 特殊语法导致的问题。 fn visit_mut_unary_expr(&mut self, unary_expr: &mut UnaryExpr) {
67-99: 建议增加更多测试用例当前的测试覆盖了基本场景，建议添加以下测试用例：
- 测试所有 webpack 相关标识符的转换
- 测试非 webpack 标识符不被转换的情况
- 测试嵌套表达式中的转换
- 测试错误处理场景
示例测试用例：
#[test] fn test_nested_typeof() { assert_eq!( run(r#"foo(typeof __webpack_require__, typeof __webpack_modules__);"#), r#"foo(typeof void 0, typeof void 0);"# ); } #[test] fn test_non_webpack_ident() { let code = r#"typeof normal_require === 'function';"#; assert_eq!(run(code), code); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
crates/mako/src/visitors/webpack_require.rs(1 hunks)
🔇 Additional comments (1)
crates/mako/src/visitors/webpack_require.rs (1)
1-99: 代码实现符合需求，建议合并
整体实现完整且符合 PR 目标，成功解决了 webpack 特殊语法的处理问题。建议在完成上述优化后合并此 PR。
| use swc_core::ecma::ast::{Expr, Ident, UnaryExpr}; | ||
| use swc_core::ecma::visit::VisitMut; | ||
|
|
||
| const WEBPACK_REQUIRE: &str = "__webpack_require__"; |
There was a problem hiding this comment.
看看是不是可以搞成一个 const slice 参考
mako/crates/mako/src/build/load.rs
Line 34 in 428cdfa
| const WEBPACK_REQUIRE: &str = "__webpack_require__"; | ||
| const WEBPACK_HASH: &str = "__webpack_hash__"; | ||
| const WEBPACK_LAYER: &str = "__webpack_layer__"; | ||
| const WEBPACK_PUBLIC_PATH: &str = "__webpack_public_path__"; |
There was a problem hiding this comment.
public path 好像 mako 是兼容支持了的， 可以确认下
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1688 +/- ##
==========================================
+ Coverage 55.64% 55.71% +0.07%
==========================================
Files 173 174 +1
Lines 17530 17561 +31
==========================================
+ Hits 9754 9784 +30
- Misses 7776 7777 +1 ☔ View full report in Codecov by Sentry. |
|
|
||
| #[test] | ||
| fn test_webpack_module_ident() { | ||
| assert_eq!(run(r#"typeof __webpack_modules__;"#), r#"typeof void 0;"#); |
There was a problem hiding this comment.
just for fun， 你可以尝试一个 typeof typeof __webpack_modules__; 这样的 case
close #1291
webpack_require is a special variable in the webpack so it should be removed
Summary by CodeRabbit
新功能
WebpackRequire访问者，增强了 JavaScript 模块的处理能力。webpack_require模块，以扩展模块解析和处理功能。测试
WebpackRequire功能的测试，确保了特定表达式的转换正确性。