-
-
Notifications
You must be signed in to change notification settings - Fork 43
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: suspense repeat trigger #58
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #58 +/- ##
==========================================
- Coverage 98.46% 98.23% -0.24%
==========================================
Files 12 12
Lines 392 396 +4
Branches 110 111 +1
==========================================
+ Hits 386 389 +3
- Misses 6 7 +1 ☔ View full report in Codecov by Sentry. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
变更概述演练在 变更
针对链接问题的评估
诗歌
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 (
|
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
🧹 Nitpick comments (1)
src/hooks/useStatus.ts (1)
Line range hint
194-245
: 建议优化代码可维护性为了提高代码的可维护性和可测试性,建议:
- 添加详细的注释说明这个修复的具体场景
- 补充相关的单元测试用例
建议添加如下注释:
+ // visibleRef 用于解决 Suspense 场景下可见性状态重复触发的问题 + // 在 Suspense 边界内,组件可能会经历多次重渲染,但实际的可见性状态并未改变 + // 通过这个 ref 来跟踪实际的可见性变化,避免不必要的动画触发 const visibleRef = useRef<boolean | null>(null);🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 202-202: src/hooks/useStatus.ts#L202
Added line #L202 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useStatus.ts
(2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/hooks/useStatus.ts
[warning] 202-202: src/hooks/useStatus.ts#L202
Added line #L202 was not covered by tests
🔇 Additional comments (2)
src/hooks/useStatus.ts (2)
194-195
: 新增 visibleRef 用于追踪可见性状态
引入 visibleRef
来解决 Suspense 场景下重复触发的问题是个不错的方案。
245-245
: 确保状态同步更新
在 effect 结束时更新 visibleRef
的值是必要的,这样可以确保下次渲染时有正确的比较基准。
// When use Suspense, the `visible` will repeat trigger, | ||
// But not real change of the `visible`, we need to skip it. | ||
// https://github.com/ant-design/ant-design/issues/44379 | ||
if (mountedRef.current && visibleRef.current === visible) { | ||
return; | ||
} | ||
|
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.
💡 Codebase verification
需要添加 Suspense 场景下的测试用例
代码检查发现:
- 当前没有针对 Suspense 场景下的 visible 重复触发的测试用例
- 虽然代码中已经添加了注释说明问题来源(ant-design/issues/44379),但仍需要相应的测试来保证功能的正确性
建议:
- 在测试文件中添加 Suspense 相关的测试场景
- 测试用例应该覆盖 visible 值重复触发但实际未改变的情况
- 验证在这种情况下是否正确跳过了不必要的更新
🔗 Analysis chain
优化了 Suspense 场景下的状态更新逻辑
通过比较 visibleRef.current
和 visible
来避免不必要的更新是个好方法。但是需要注意以下几点:
- 这段新增的逻辑缺少测试覆盖
- 建议添加注释说明跳过更新的具体场景
让我们验证一下相关测试用例:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索与 Suspense 相关的测试用例
rg -l 'Suspense.*useStatus' --type=test
# 检查是否有测试文件包含对这个新增功能的测试
rg -l 'visible.*repeat|repeat.*visible' --type=test
Length of output: 153
Script:
#!/bin/bash
# 搜索与 Suspense 相关的测试用例
rg -l "Suspense.*useStatus"
# 检查是否有测试文件包含对这个新增功能的测试
rg -l "visible.*repeat|repeat.*visible"
# 查找所有测试文件
fd "test\.(ts|tsx|js|jsx)$"
# 查找包含 useStatus 的测试文件
rg -l "useStatus" -g "*test*"
Length of output: 303
Script:
#!/bin/bash
# 查找 useStatus 相关的测试文件
fd -e test.tsx -e test.ts -e test.jsx -e test.js | xargs rg -l "useStatus"
# 检查 useStatus.ts 文件中的测试相关内容
rg "test|describe|it\(" src/hooks/useStatus.ts
# 查看 useStatus.ts 的完整内容以了解测试覆盖情况
cat src/hooks/useStatus.ts
Length of output: 8413
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 202-202: src/hooks/useStatus.ts#L202
Added line #L202 was not covered by tests
Suspense 下,
useEffect
会重复触发。导致当visible=false
被重复触发时,认为是从可见变成不可见。导致触发移除动画。额外包一个 ref 来记录真正的状态。fix ant-design/ant-design#44379
Summary by CodeRabbit
visibleRef
,用于跟踪可见性状态,优化了组件在重新渲染时的更新效率。useIsomorphicLayoutEffect
钩子的逻辑,确保在可见性状态未更改时跳过不必要的更新。