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

feat: 添加 useSafeState hook #826

Merged
merged 11 commits into from
Feb 23, 2021
Merged

feat: 添加 useSafeState hook #826

merged 11 commits into from
Feb 23, 2021

Conversation

Dolov
Copy link
Contributor

@Dolov Dolov commented Jan 11, 2021

避免组件卸载后,异步回调中执行 setState

@brickspert
Copy link
Collaborator

测试用例补一下?

@awmleer
Copy link
Collaborator

awmleer commented Jan 27, 2021

感觉叫 useSafeState 更合适一些……?

@Dolov
Copy link
Contributor Author

Dolov commented Jan 27, 2021

感觉叫 useSafeState 更合适一些……?

改了改了~

@Dolov
Copy link
Contributor Author

Dolov commented Jan 28, 2021

感觉叫 useSafeState 更合适一些……?

改了改了~

@awmleer 这个能合并么

Copy link
Collaborator

@awmleer awmleer left a comment

Choose a reason for hiding this comment

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

LGTM

@Dolov
Copy link
Contributor Author

Dolov commented Feb 2, 2021

LGTM

要是没啥问题合一下? 不然我本地还得再维护一份代码 ~~~ @awmleer

@@ -23,7 +23,10 @@ export default function useControllableValue<T>(props: Props = {}, options: Opti
const value = props[valuePropName];

const [state, setState] = useState<T | undefined>(() => {
if (valuePropName in props) {
if (
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.

这个地方会导致 defaultValue 不生效吧

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Dolov 是和这个 issue 相关的么: #848

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,这个地方的处理跟 antd 的处理有偏差吧,存在应该指的是初次值不为 undefined 的吧

Copy link
Contributor Author

@Dolov Dolov Feb 2, 2021

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.

@brickspert 老哥 这个地方是不是再考虑一下?目前 antd 的组件(以及原生 input )针对首次 value 为 undefined 的时候 defaultValue 都会生效,按照 useControllableValue 目前的实现。无法跟 antd 完美的结合呀。~~

@awmleer awmleer added the feature New feature or request label Feb 2, 2021
@brickspert
Copy link
Collaborator

LGTM

要是没啥问题合一下? 不然我本地还得再维护一份代码 ~~~ @awmleer

还有一些小问题,另外就是不要改 useControllableValue ~~

感觉这个 Hooks 有点细碎,不确定会不会造成选择困难症。

@awmleer awmleer changed the title feat: 添加 useAsyncState hook feat: 添加 useSafeState hook Feb 7, 2021
@awmleer awmleer merged commit d3cb0c2 into alibaba:master Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants