fix: commonjs may miss use strict directive #1386
Conversation
Walkthrough本次变更主要集中在对 Changes
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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 (1)
crates/mako/src/visitors/common_js.rs (1)
20-53: 实现逻辑看起来不错，但请翻译注释。
VisitMuttrait 的实现正确地转换了模块并应用了必要的配置。然而，为了保持一致性，建议将中文注释翻译成英文。- // NOTE: 这里后面要调整为注入自定义require + // NOTE: This needs to be adjusted later to inject custom require
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- crates/mako/src/generate/transform.rs (3 hunks)
- crates/mako/src/visitors/common_js.rs (1 hunks)
- crates/mako/src/visitors/mod.rs (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/expect.js (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/index.ts (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/mako.config.json (1 hunks)
Files skipped from review due to trivial changes (5)
- crates/mako/src/generate/transform.rs
- crates/mako/src/visitors/mod.rs
- e2e/fixtures/javascript.transform.commonjs/expect.js
- e2e/fixtures/javascript.transform.commonjs/index.ts
- e2e/fixtures/javascript.transform.commonjs/mako.config.json
Additional comments not posted (3)
crates/mako/src/visitors/common_js.rs (3)
1-12: 导入部分看起来不错。这些导入包含了模块转换和实用函数所需的依赖项，适合当前实现的功能。
14-18: 结构体定义看起来不错。
Commonjs结构体定义了三个字段：context,unresolved_mark, 和import_interop，清晰且适当。
56-66: 函数定义看起来不错。
common_js函数定义明确且适当，返回一个带有提供参数的Commonjs实例。
|
|
||
| impl VisitMut for Commonjs { | ||
| fn visit_mut_module(&mut self, n: &mut Module) { | ||
| let use_strict = n |
There was a problem hiding this comment.
这个判断不准确 directive 需要是 module 的最开始的string
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- crates/mako/src/visitors/common_js.rs (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/expect.js (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/index.ts (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/mako.config.json (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/scripts.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- e2e/fixtures/javascript.transform.commonjs/mako.config.json
Files skipped from review as they are similar to previous changes (2)
- e2e/fixtures/javascript.transform.commonjs/expect.js
- e2e/fixtures/javascript.transform.commonjs/index.ts
Additional comments not posted (1)
crates/mako/src/visitors/common_js.rs (1)
39-61: 确保配置项的正确性在设置
Config时，确保所有配置项都符合预期，特别是allow_top_level_this和strict_mode的设置。请确认
allow_top_level_this和strict_mode的设置是否符合预期，特别是在 ESM 模式下。
| // @ts-ignore | ||
| fn('url', { method: 'get' }).url.indexOf('u') |
There was a problem hiding this comment.
避免忽略 TypeScript 错误
@ts-ignore 注释会忽略 TypeScript 错误，可能会掩盖潜在的问题。建议修复类型错误而不是忽略它们。
// @ts-ignore
fn('url', { method: 'get' }).url.indexOf('u');| fn visit_mut_module(&mut self, n: &mut Module) { | ||
| let mut use_strict = false; | ||
| if utils::is_esm(n) { | ||
| use_strict = true | ||
| } else { | ||
| for item in n.body.iter() { | ||
| if let Some(stmt) = item.as_stmt() { | ||
| if stmt.is_directive() { | ||
| if stmt.is_use_strict() { | ||
| use_strict = true; | ||
| } | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
优化 "use strict" 指令检查
目前的实现会遍历整个模块的 body 来查找 "use strict" 指令。可以优化为只检查模块的开头部分。
fn visit_mut_module(&mut self, n: &mut Module) {
let mut use_strict = false;
for item in n.body.iter().take_while(|item| item.is_directive()) {
if let Some(stmt) = item.as_stmt() {
if stmt.is_use_strict() {
use_strict = true;
break;
}
}
}
...
}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
crates/mako/src/visitors/optimize_define_utils.rs (1)
158-173: 新函数实现合理，但可以改进可读性函数
is_directive_judged_by_stmt_value_and_raw实现了根据语句的值和原始字符串判断是否为指令的逻辑，符合保留use strict等指令的目标。为了提高可读性，可以将嵌套的
match语句改为更清晰的逻辑结构：fn is_directive_judged_by_stmt_value_and_raw(stmt: Stmt) -> bool { match stmt.as_ref() { Some(Stmt::Expr(expr)) => match &*expr.expr { Expr::Lit(Lit::Str(Str { raw: Some(raw), .. })) => { raw.starts_with("\"use ") || raw.starts_with("'use ") } Expr::Lit(Lit::Str(Str { value: v, raw: None, .. })) => { v.to_string().starts_with("use ") } _ => false, }, _ => false, } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/mako/src/visitors/optimize_define_utils.rs (3 hunks)
- e2e/fixtures/generate.chunks-hash-replace/expect.js (1 hunks)
- e2e/fixtures/javascript.transform.commonjs/scripts.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- e2e/fixtures/generate.chunks-hash-replace/expect.js
Files skipped from review as they are similar to previous changes (1)
- e2e/fixtures/javascript.transform.commonjs/scripts.ts
Additional comments not posted (1)
crates/mako/src/visitors/optimize_define_utils.rs (1)
2-2: 导入语句添加合理添加了
Stmt和Str的导入，这些导入在新函数is_directive_judged_by_stmt_value_and_raw中使用。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/visitors/optimize_define_utils.rs (3 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/src/visitors/optimize_define_utils.rs
stormslowly
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/visitors/optimize_define_utils.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/visitors/optimize_define_utils.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/visitors/optimize_define_utils.rs (2 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/src/visitors/optimize_define_utils.rs
https://www.unpkg.com/browse/[email protected]/lib/core/Axios.js L30-L35
webpack 编译正常执行，mako 会导致 config.url 成为一个对象，期望是 String
原因 webpack 保留了use strict
对于ESM模块，默认情况下，所有模块都是以严格模式（strict mode）运行的，这是ESM规范的一部分。因此，即使源代码中没有明确写入"use strict"，加载和执行ESM模块时也会自动采用严格模式, 这里也按照规范给所有的esm添加了 use strict 指令
close #839
Summary by CodeRabbit
新增功能
Commonjs，实现了VisitMut接口，用于处理CommonJS模块的转换。expect.js中，添加了解析构建结果和断言内容匹配特定模块模式的功能。index.ts和scripts.ts中，添加了一个可以配置并返回修改后的config对象的函数。重构
optimize_define_utils.rs中，重构了visit_mut_module_items方法，使用新函数来判断语句是否为指令。文档
mako.config.json。