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: pollingInterval update logic in useRequest-usePolling #1463

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

fix: pollingInterval update logic in useRequest-usePolling #1463

Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5e81c8f
update pollingInterval logic
xiaoYuanDun Feb 14, 2022
da9adcb
fix: update pollingInterval logic
xiaoYuanDun Feb 14, 2022
ee7b019
fix: handle canceled timer when pollingInterval change
xiaoYuanDun Feb 14, 2022
9398576
fix: only reset timer when request not cancel and document not hidden
xiaoYuanDun Feb 15, 2022
008d3e1
fix: add logic to reset timer when document is hidden
xiaoYuanDun Feb 15, 2022
816a734
Merge branch 'master' into master
xiaoYuanDun Feb 15, 2022
5edb2dc
feat: usePollingPlugin support interval dynamic change
brickspert Feb 15, 2022
3df81cb
Merge branch 'master' of github.com:xiaoYuanDun/hooks into xiaoYuanDu…
brickspert Feb 15, 2022
8ea6187
fix: improve logic of pollingInterval change
xiaoYuanDun Feb 15, 2022
c8f1f20
fix: improve logic of pollingInterval change
xiaoYuanDun Feb 15, 2022
64aefe4
fix: add cancel flag
xiaoYuanDun Feb 15, 2022
969fa1c
fix: add cancel flag
xiaoYuanDun Feb 16, 2022
40da696
fix: code format
xiaoYuanDun Feb 16, 2022
0dfe054
fix: code format
xiaoYuanDun Feb 16, 2022
f0c65da
fix: handle cancel when pollingInterval is zero
xiaoYuanDun Feb 16, 2022
f7ae5e4
Merge branch 'master' into master
xiaoYuanDun Feb 25, 2022
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
7 changes: 7 additions & 0 deletions packages/hooks/src/useRequest/src/plugins/usePollingPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@ const usePollingPlugin: Plugin<any, any[]> = (
const stopPolling = () => {
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = undefined;
}
unsubscribeRef.current?.();
};

useUpdateEffect(() => {
if (!pollingInterval) {
stopPolling();
} else if (timerRef.current) {
Copy link
Author

@xiaoYuanDun xiaoYuanDun Feb 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用 timeRef.current 可能不准确
测试过程中发现，如果 pollingInterval 变化发生在一次请求的 onBefore 之后，onFinally 之前，此时 timeRef.current 是被清除了的（undefined），导致不能正确的进入这里的逻辑来启动新定时器。

要启动新定时器，应该满足下面两个条件：

  1. 当前处于 非 cancel 状态（timeRef.current !== -1）
  2. 页面显示； 或者页面隐藏，但 pollingWhenHidden 必须是 true （isDocumentVisible() || pollingWhenHidden）

我结合了一下我们的PR，调整了一下，你看看，能不能接受这样改呢

另外，当 pollingInterval 为 0 时也不能直接返回空对象，此时不是 cancel 状态，应该要返回 onCancel 和 onBefore 来处理可能到来的 pollingInterval 变更和 cancel 请求

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

稍后我看下，目前的代码变更还是偏复杂，我好好想想。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok，感觉这里和 options.ready 的情景有点相似

// if pollingInterval is changed, restart polling
clearTimeout(timerRef.current);
timerRef.current = setTimeout(() => {
fetchInstance.refresh();
}, pollingInterval);
}
}, [pollingInterval]);

Expand Down