-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: https://github.com/umijs/mako/issues/1007 #1738
base: master
Are you sure you want to change the base?
Conversation
support BinaryExpression
概述演练这次更改主要增强了 变更
对链接问题的评估
诗歌
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 (
|
WalkthroughThis pull request addresses issue #1007 by adding support for handling Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
e2e/fixtures/resolve.failed-in-try-statement/src/index.tsx (1)
10-12
: 建议在异常处理中记录错误信息或提示。
当前仅在try
/catch
中使用了require('./faa')
,如果引入失败会静默处理,没有额外的日志或提示。建议在catch
块中加上相应的日志或错误处理逻辑,以便在调试或排查问题时更直观地发现具体的异常原因。crates/mako/src/visitors/try_resolve.rs (1)
99-111
: 在二元表达式的左右两侧均支持处理CallExpr
,增强了潜在场景覆盖。
此处的实现对于Expr::Bin
判断全面,能够分别检测左右两侧是否有require('...')
,并进行必要的处理,符合预期需求。若后续有更多逻辑运算符(如&&
、三元操作符等)也需要类似处理,可考虑进一步抽象这个逻辑以保持可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/mako/src/visitors/try_resolve.rs
(3 hunks)e2e/fixtures/resolve.failed-in-try-statement/expect.js
(1 hunks)e2e/fixtures/resolve.failed-in-try-statement/src/index.tsx
(1 hunks)
🔇 Additional comments (3)
e2e/fixtures/resolve.failed-in-try-statement/expect.js (1)
24-29
: 检查测试正则表达式与实际模块名称是否保持一致。
此处断言检查的错误信息是Cannot find module './hoo'
,但在index.tsx
中引入的实际模块却是'./faa'
。请确认是否存在拼写或路径不一致的问题,以免断言无法准确覆盖新逻辑。crates/mako/src/visitors/try_resolve.rs (2)
5-6
: 新导入的BinExpr
对应代码处理已添加,逻辑清晰。
添加对BinExpr
的导入有助于在后续处理二元表达式时更快调用相关的 AST API,当前用法与需求保持一致,没有明显问题。
227-242
: 新测试用例覆盖了二元表达式中出现require('foo')
的场景。
此测试能够验证对0 || require('foo')
形式的代码是否能够正确地进行依赖解析并抛出对应的异常提示,进一步提升了对异常场景的覆盖率。实现思路正确且与整体用例风格一致。
fix #1007
support BinaryExpression
Summary by CodeRabbit
新功能
require
调用的场景。测试
require
的场景。代码改进