fix: ✅ ts annotated declare variable treat as top level variable #1682
Conversation
Walkthrough该拉取请求引入了对 Changes
Assessment against linked issues
Possibly related PRs
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 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 (2)
e2e/fixtures/config.define/src/index.tsx (2)
45-47: 建议优化测试用例描述当前的测试描述 "should replace ts declare shallowed value" 不够具体。建议修改为更明确的描述，以便更好地理解测试目的。
建议修改如下：
-it("should replace ts declare shallowed value", ()=>{ +it("should correctly resolve TypeScript declared variable FOO with define value", ()=>{ expect(FOO).toEqual("bar") });
3-3: 建议添加更多边界测试用例当前的实现验证了基本功能，但建议添加以下场景的测试用例以确保完整性：
- 在不同作用域中使用声明变量
- 使用联合类型的声明变量
- 验证类型推导是否正确
这将帮助确保 SWC resolver 在各种场景下都能正确处理 TypeScript 声明。
Also applies to: 45-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
e2e/fixtures/config.define/expect.js(1 hunks)e2e/fixtures/config.define/mako.config.json(1 hunks)e2e/fixtures/config.define/src/index.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- e2e/fixtures/config.define/expect.js
🔇 Additional comments (3)
e2e/fixtures/config.define/mako.config.json (2)
26-27: 配置格式正确且一致！
新添加的配置项 "FOO": "\"bar\"" 遵循了现有的字符串值转义规则，与其他配置项保持一致。
26-27: 验证配置是否正确应用
请确保此配置能够正确地被 SWC resolver 识别并替换 TypeScript 声明的变量。
运行以下脚本来验证相关配置的使用：
✅ Verification successful
配置已正确设置且被正确使用
通过验证发现：
- 在
e2e/fixtures/config.define/src/index.tsx中正确声明了FOO变量并在测试中使用 - 配置文件
mako.config.json中的"FOO": "\"bar\""值与测试预期的"bar"值相匹配 - 变量声明、配置值和使用场景都保持一致，说明 SWC resolver 能够正确处理这个配置
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 FOO 变量在项目中的声明和使用
# 测试：查找 FOO 变量的声明和使用位置
echo "查找 FOO 变量的声明："
rg -t typescript "declare.*FOO"
echo "查找 FOO 变量的使用位置："
rg "FOO" --type=typescript
Length of output: 208
Script:
#!/bin/bash
# 查找所有 TypeScript 和 JavaScript 文件中的 FOO 变量声明和使用
echo "查找 FOO 变量的声明和使用："
rg "FOO" -g "*.{ts,js,tsx,jsx}"
# 查找配置文件中的 FOO 相关配置
echo -e "\n查找配置文件中的 FOO："
rg "FOO" -g "*.{json,config.js}"
# 查找测试文件
echo -e "\n查找测试文件中的 FOO："
fd "test|spec" -e ts -e js -e tsx -e jsx --exec rg "FOO" {}
Length of output: 522
e2e/fixtures/config.define/src/index.tsx (1)
3-3: 变量声明看起来没有问题！
声明符合 TypeScript 的最佳实践，类型注解恰当。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1682 +/- ##
==========================================
+ Coverage 55.64% 55.66% +0.02%
==========================================
Files 173 174 +1
Lines 17530 17545 +15
==========================================
+ Hits 9754 9766 +12
- Misses 7776 7779 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
crates/mako/src/visitors/clean_ctxt.rs (2)
4-4: 建议为结构体添加文档注释为了提高代码可维护性，建议添加文档注释来解释这个结构体的用途。
+/// 清理语法上下文的访问者实现 struct CleanSyntaxContext;
6-8: 工厂函数实现正确，建议补充文档函数实现符合 Rust 的惯用法，使用
as_folder转换访问者为文件夹很恰当。建议添加文档说明其用途。+/// 创建一个新的语法上下文清理器 +/// +/// 返回一个实现了 `VisitMut` 和 `Fold` trait 的访问者， +/// 用于清理 TypeScript 声明变量的语法上下文。 pub fn clean_syntax_context() -> impl VisitMut + Fold { as_folder(CleanSyntaxContext {}) }crates/mako/src/build/transform.rs (2)
80-94: 建议优化条件判断逻辑在第80-94行的代码中，先判断了
if is_tsx || is_ts，然后在内部又分别判断if is_tsx和if is_ts。这种嵌套结构可能不够简洁。建议直接使用分支条件：if is_tsx { // ... } else if is_ts { // ... }这样可以减少嵌套层级，提高代码可读性。
33-33: 优化导入语句的组织方式第33行的导入语句
use crate::visitors::clean_ctxt::clean_syntax_context;位置稍显不统一。为保持代码的一致性和可读性，建议将其与其他visitors模块的导入语句放在一起，按照模块或功能进行分组。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
crates/mako/src/build/transform.rs(6 hunks)crates/mako/src/plugins/tree_shaking/shake/module_concatenate.rs(2 hunks)crates/mako/src/visitors.rs(1 hunks)crates/mako/src/visitors/clean_ctxt.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/mako/src/visitors.rs
🔇 Additional comments (5)
crates/mako/src/visitors/clean_ctxt.rs (2)
1-2: 导入看起来完整且合适！
导入了必要的 SWC 核心组件用于语法上下文处理和访问者模式实现。
10-14: 实现逻辑正确且符合需求
VisitMut trait 的实现通过将语法上下文设置为空来实现类型擦除，这正确解决了 TypeScript 声明变量的处理问题。建议添加测试用例验证其行为。
crates/mako/src/plugins/tree_shaking/shake/module_concatenate.rs (2)
15-15: 导入声明优化完成！
导入声明的修改使代码结构更加清晰，通过移除未使用的导入并添加必要的clean_syntax_context导入，提高了代码的可维护性。
Also applies to: 18-18, 30-30
Line range hint 367-367: 建议验证语法上下文清理的时机是否合适
clean_syntax_context的调用位置在模块转换之后，这个时机看起来是合理的。但是建议验证以下几点：
- 在内部模块和根模块转换后的清理是否都是必要的
- 是否会影响后续的解析器（resolver）处理
Also applies to: 436-436
✅ Verification successful
Let me analyze the results and generate additional verification scripts to understand the impact of clean_syntax_context better.
语法上下文清理的时机是合适的
通过分析代码实现和使用情况，可以确认：
clean_syntax_context的实现非常简单，它只是将所有语法上下文重置为空- 在模块转换后（内部模块和根模块）进行清理是必要的，因为：
- 它确保了转换后的代码不会携带任何不必要的语法上下文
- 这种清理不会影响后续的解析器处理，因为它只是重置上下文而不改变代码结构
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否存在其他地方也在使用clean_syntax_context
rg "clean_syntax_context" -A 3 -B 3
# 检查是否有相关的测试用例
fd -e rs -e tsx -e ts "test.*clean.*context"
Length of output: 4261
Script:
#!/bin/bash
# Check the implementation of CleanSyntaxContext visitor
cat crates/mako/src/visitors/clean_ctxt.rs
# Check if there are any related resolver calls near clean_syntax_context
rg "resolver.*clean_syntax_context|clean_syntax_context.*resolver" -A 3 -B 3
# Look for any test files that might test module concatenation
fd -e rs "test.*module.*concat|test.*concat.*module"
Length of output: 554
crates/mako/src/build/transform.rs (1)
373-374: 确保tsx_strip函数的错误处理
在第373-374行，调用了ast.visit_mut_with(&mut tsx_strip(cm, context, top_level_mark, unresolved_mark));。请确保tsx_strip函数内部具有完善的错误处理机制，以防止在访问和修改AST时出现未捕获的异常，影响程序稳定性。
|
why use |
stormslowly
changed the title
|
!bench |
|
📝 Benchmark detail: Open
|
close #1673
so we need to strip ts type first , then do the ctxt resolve
Summary by CodeRabbit
新功能
"FOO": "\"bar\""。FOO，并添加测试用例以验证其值为"bar"。clean_ctxt，用于清理语法上下文。文档
path模块以增强功能。重构
CleanSyntaxContext结构及其方法，简化了代码结构。