Skip to content
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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 19, 2024
+11 −0
Merged

fix: suspense repeat trigger #58

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/hooks/useStatus.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,17 @@
activeRef.current = active;

// ============================ Status ============================
const visibleRef = useRef<boolean | null>(null);

// Update with new status
useIsomorphicLayoutEffect(() => {
// 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;

Check warning on line 202 in src/hooks/useStatus.ts

View check run for this annotation

Codecov / codecov/patch

src/hooks/useStatus.ts#L202 

Added line #L202 was not covered by tests
}

Comment on lines +198 to +204
Copy link

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 来避免不必要的更新是个好方法。但是需要注意以下几点：

  1. 这段新增的逻辑缺少测试覆盖
  2. 建议添加注释说明跳过更新的具体场景

让我们验证一下相关测试用例：

🏁 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

setAsyncVisible(visible);

const isMounted = mountedRef.current;
Expand Down Expand Up @@ -232,6 +241,8 @@
// Set back in case no motion but prev status has prepare step
setStatus(STATUS_NONE);
}

visibleRef.current = visible;
}, [visible]);

// ============================ Effect ============================
Expand Down
Loading