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.

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

useCountDown Hook #619

Merged
merged 3 commits into from
Sep 7, 2020
Merged

useCountDown Hook #619

merged 3 commits into from
Sep 7, 2020

Conversation

linbudu599
Copy link
Contributor

Related Issue #576

@CLAassistant
Copy link

CLAassistant commented Aug 30, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@brickspert brickspert left a comment

Choose a reason for hiding this comment

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

src/index 需要导出一下

import React from 'react';
import useCountDown from '../index';

const timeEnd = Date.now() + 5000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const timeEnd = Date.now() + 5000;
const timeEnd = () => (Date.now() + 5000);

这里应该是个函数,否则会有问题。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我看了一下, 这里如果是函数, 会缓存掉这个Date.now(), 比如多次启用计时器间隔会递增, 比如5 10 15这样. 然后这种直接传入Date.now的方式, 每次反而会重新计算.
但我发现了另外一个问题, 传入Date.now(), 由于程序运行的时间, 有可能手动启用时的时间戳不到5000, 会是499x这种, 导致计时器少1s.
我尝试过传入函数然后在内部调用, 同样会有问题. 除非是像我另外一个包的写法直接传入秒数来计时: use-verify-code

有什么好的建议吗2333

Copy link
Collaborator

Choose a reason for hiding this comment

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

这样写会有问题吗?感觉不会吧~

<button
  onClick={()=> { setTargetDate( new Date() + 5000 ) }}
>
Start Interval
</button>

Copy link
Contributor Author

@linbudu599 linbudu599 Sep 2, 2020

Choose a reason for hiding this comment

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

会的 仍然是后续都会使用启用时的时间戳 只有首次正常

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
Contributor Author

Choose a reason for hiding this comment

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

又试了一些方法, 感觉只要以Date.now()来调用就会有问题, 比如上面的setTargetDate( new Date() + 5000 )调用方式, 我适配了后发现这个Date.now()是在组件首次渲染完成就固定的, 除非你立刻点击, 不然计时时间就会变长... 代码的实现我贴一下 Demo 只有立刻点击才是正常的. 可能还需要再思考下怎么解决这个问题

const dateFrom = options?.dateFrom ? new Date(options?.dateFrom).getTime() : Date.now();

const calcLeft = useCallback(
(targetDate?: TDate) => new Date(targetDate ?? target ?? Date.now()).getTime() - dateFrom,
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果 targetDate 和 dateFrom 都有值,那这里应该永远不会变化了。

Copy link
Collaborator

Choose a reason for hiding this comment

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

还是不要 dateFrom 了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

那是否要固定dateFrom为当前时间呢🤣

Copy link
Collaborator

Choose a reason for hiding this comment

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

calcLeft 是计算 当前时间(new Date()) 到 目标时间(targetDate) 的间隔的。

和 dateFrom 没啥关系吧。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

calcLeft 是计算 当前时间(new Date()) 到 目标时间(targetDate) 的间隔的。

和 dateFrom 没啥关系吧。

当前的实现里是允许用户定义起始时间的, 比如12月12日距离12月1日, 只有用户不传dateFrom的情况下才使用当前时间

Copy link
Collaborator

Choose a reason for hiding this comment

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

dateFrom 是指开始时间,如果没有到开始时间,返回的 countdown 应该是不变的。
如果已经到了开始时间,返回的 countdown 才会递减。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get√

Copy link
Contributor Author

Choose a reason for hiding this comment

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

那我这里就先移除dateFrom支持吧

@brickspert brickspert mentioned this pull request Sep 7, 2020
@brickspert
Copy link
Collaborator

先合并了,然后我再修正一波。

@brickspert brickspert changed the base branch from master to feat/useCountDown September 7, 2020 12:12
@brickspert brickspert merged commit 69046e3 into alibaba:feat/useCountDown Sep 7, 2020
@github-actions
Copy link

github-actions bot commented Sep 7, 2020

😭 Deploy PR Preview 69046e3 failed. Build logs

🤖 By surge-preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants