-
Notifications
You must be signed in to change notification settings - Fork 74
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: #1478 support react class-component hot-update #1489
Conversation
Walkthrough此次改动显著提升了 React 组件的热更新机制,尤其是在类组件中的表现。通过引入 Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant ReactRuntime
participant ClassComponent
Developer->>ReactRuntime: 修改类组件代码
ReactRuntime->>ClassComponent: 检测到代码变更
ClassComponent-->>ReactRuntime: 返回组件信息
ReactRuntime-->>Developer: 自动更新视图
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 Configuration File (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/visitors/react.rs (3 hunks)
Additional comments not posted (2)
crates/mako/src/visitors/react.rs (2)
126-126
: 请验证$RefreshSig$
的更改是否正确。将
$RefreshSig$
修改为箭头函数可能会影响刷新签名的生成。请确保此更改符合预期功能,并不会引入新的问题。Verification successful
更改
$RefreshSig$
的影响已验证。在代码库中,
$RefreshSig$
被一致地用作箭头函数,并且相关的测试也验证了这一更改。这表明此更改符合预期功能且没有引入新问题。
$RefreshSig$
在多个地方使用,并且与旧值进行比较。- 测试代码中包含对新行为的断言。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the `$RefreshSig$` change. # Test: Search for `$RefreshSig$` usage in the codebase. Expect: Consistent usage of the new arrow function. rg --type rust -A 5 $'\\$RefreshSig\\$'Length of output: 1569
164-175
: 请验证$RefreshIsReactComponentLike$
函数逻辑的更改。简化逻辑以直接检查
moduleExports
,这可能提升性能。请确保此更改不会影响组件类型的正确识别。Verification successful
验证通过:
$RefreshIsReactComponentLike$
函数逻辑的更改代码逻辑正确地集成了对 React 组件类型的识别,并通过
RefreshRuntime
进行注册。此更改保持了原有功能,没有发现问题。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic change in `$RefreshIsReactComponentLike$`. # Test: Search for function calls and ensure correct logic is applied. Expect: Correct identification of React component types. rg --type rust -A 5 $'fn \\$RefreshIsReactComponentLike\\$'Length of output: 58
Script:
#!/bin/bash # Description: Search for `RefreshRuntime` usage in Rust files to identify logic related to React component type checking. # Search for `RefreshRuntime` usage in Rust files. rg --type rust 'RefreshRuntime' -A 10Length of output: 3167
here is my two cents: maybe we can try patch ast after swc's react visit, to register all the component like class. |
Yeah, I suspect the main issue is that the class component, after being compiled with SWC, becomes a module and can't be registered correctly. Solutions I can think of include:
I still have some issues with my current approach, and I'll make some adjustments later. |
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- crates/mako/src/visitors/react.rs (2 hunks)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js (1 hunks)
- examples/react/src/App.tsx (1 hunks)
- examples/react/src/ClassCmp.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js
Additional comments not posted (8)
examples/react/src/App.tsx (2)
5-11
: 组件渲染逻辑看起来不错。
App
组件的渲染逻辑已正确更新以包含新组件。
2-2
: 确保组件导入路径正确。请确认
./ClassCmp
路径下存在ClassCmp
,ChildFnCmp
, 和ClassCmp2
组件。Verification successful
组件导入路径正确
在
./examples/react/src/ClassCmp.tsx
文件中找到了ClassCmp
,ClassCmp2
, 和ChildFnCmp
组件的定义,导入路径正确,无需更改。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of components in the specified file. # Test: Search for the component definitions. Expect: Definitions for ClassCmp, ChildFnCmp, and ClassCmp2. rg --type tsx -A 2 $'export default class ClassCmp' ./examples/react/src/ClassCmp.tsx rg --type tsx -A 2 $'export class ClassCmp2' ./examples/react/src/ClassCmp.tsx rg --type tsx -A 2 $'export function ChildFnCmp' ./examples/react/src/ClassCmp.tsxLength of output: 334
Script:
#!/bin/bash # Description: Verify the existence of components in the specified file. # Search for the component definitions in the .tsx file. rg -A 2 $'export default class ClassCmp' ./examples/react/src/ClassCmp.tsx rg -A 2 $'export class ClassCmp2' ./examples/react/src/ClassCmp.tsx rg -A 2 $'export function ChildFnCmp' ./examples/react/src/ClassCmp.tsxLength of output: 553
examples/react/src/ClassCmp.tsx (3)
3-7
: 默认类组件定义正确。
ClassCmp
类组件定义和导出无误。
9-13
: 导出类组件定义正确。
ClassCmp2
类组件定义和导出无误。
15-17
: 导出函数组件定义正确。
ChildFnCmp
函数组件定义和导出无误。crates/mako/src/visitors/react.rs (3)
163-171
: 新函数registerExportsForReactRefresh
实现正确。该函数确保模块导出的组件正确注册以支持热更新。
Line range hint
173-185
: 优化组件检测逻辑。
$RefreshIsReactComponentLike$
函数的逻辑已优化,确保更高效的组件类型检测。
Line range hint
187-191
: 确保热更新逻辑正确集成。在模块加载后,确保组件类型正确注册并触发热更新。
what about this case |
It works great. |
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- crates/mako/src/visitors/react.rs (2 hunks)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js (1 hunks)
- examples/react/src/App.tsx (1 hunks)
- examples/react/src/ClassCmp.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js
Files skipped from review as they are similar to previous changes (2)
- examples/react/src/App.tsx
- examples/react/src/ClassCmp.tsx
Additional comments not posted (2)
crates/mako/src/visitors/react.rs (2)
Line range hint
172-184
: 简化逻辑的改动已获批准。对
$RefreshIsReactComponentLike$
的逻辑简化改动提高了代码的鲁棒性,特别是在处理无法访问的属性时。
Line range hint
153-155
: 建议:使用RefreshRuntime.createSignatureFunctionForTransform
。当前对
self.$RefreshSig$
的赋值方式可能会影响刷新签名的生成。建议使用RefreshRuntime.createSignatureFunctionForTransform
以避免潜在问题。
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
Outside diff range, codebase verification and nitpick comments (2)
crates/mako/src/visitors/react.rs (2)
163-174
: 建议:改进错误处理逻辑。
registerExportsForReactRefresh
函数中使用了 try-catch 结构来处理可能的异常,但 catch 块是空的,这可能会隐藏潜在的问题。建议在 catch 块中添加日志或其他处理措施,以便在发生异常时能够更好地进行调试。catch (e) { console.error('Error accessing moduleExports key:', key, e); }
Line range hint
176-187
:
建议:改进错误处理逻辑。在
$RefreshIsReactComponentLike$
函数中,try-catch 块用于处理可能的异常,但 catch 块是空的。建议在 catch 块中添加日志或其他处理措施,以便在发生异常时能够更好地进行调试。catch (e) { console.error('Error accessing moduleExports key:', key, e); }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- crates/mako/src/visitors/react.rs (2 hunks)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js (1 hunks)
- examples/react/src/App.tsx (1 hunks)
- examples/react/src/ClassCmp.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- e2e/fixtures/tree-shaking.failed.remove-unused-imports/expect.js
- examples/react/src/ClassCmp.tsx
Files skipped from review as they are similar to previous changes (1)
- examples/react/src/App.tsx
Additional comments not posted (1)
crates/mako/src/visitors/react.rs (1)
Line range hint
153-153
:
验证:检查self.$RefreshSig$
的变更影响。
self.$RefreshSig$
被修改为返回字符串"%exports%"
的箭头函数。请确保此更改不会影响热更新签名的生成和使用。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1489 +/- ##
=======================================
Coverage 60.91% 60.91%
=======================================
Files 126 126
Lines 15135 15135
=======================================
Hits 9219 9219
Misses 5916 5916 ☔ View full report in Codecov by Sentry. |
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/mako/src/visitors/react.rs (2 hunks)
Additional comments not posted (2)
crates/mako/src/visitors/react.rs (2)
163-175
: 建议:为registerClassComponent
添加异常处理。当前函数在访问
moduleExports
的属性时,可能会遇到无法访问的情况,建议在访问moduleExports[key]
时添加异常处理。for (const key in moduleExports) { if (key === "__esModule") continue; try { const exportValue = moduleExports[key]; if (RefreshRuntime.isLikelyComponentType(exportValue) && exportValue.prototype && exportValue.prototype.isReactComponent) { RefreshRuntime.register(exportValue, filename + " " + key); } } catch (e) { // 处理访问异常,例如依赖循环 } }
Line range hint
177-191
: 建议:改进$RefreshIsReactComponentLike$
函数的健壮性。当前函数在访问
moduleExports
的属性时,可能会遇到无法访问的情况,建议在访问moduleExports[key]
时添加异常处理。function $RefreshIsReactComponentLike$(moduleExports) { if (RefreshRuntime.isLikelyComponentType(moduleExports || moduleExports.default)) { return true; } for (var key in moduleExports) { try { if (RefreshRuntime.isLikelyComponentType(moduleExports[key])) { return true; } } catch (e) { // 处理访问异常,例如依赖循环 } } return false; }
close #1478
Summary by CodeRabbit
新功能
性能优化
文档