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

组件文档treeSelect第一个例子有错误 #34437

Open
1 task
guiyu1230 opened this issue Mar 11, 2022 · 22 comments
Open
1 task

组件文档treeSelect第一个例子有错误 #34437

guiyu1230 opened this issue Mar 11, 2022 · 22 comments
Labels
🎱 Collaborate PR only Need confirm with Designer or Core member Inactive

Comments

@guiyu1230
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

Reproduction link

Edit on CodeSandbox

Steps to reproduce

初始state.value为undefined时. 组件虽然设置了value属性但并不受控
文档12行 setValue(value); console打印value. 发现实际上设置的是 setValue(undefined).
导致组件value永远不受控.

What is expected?

修正文档12行的地方. 使用onChange的value参数赋值

What is actually happening?

并没有实现

Environment Info
antd 4.19.1
React 16.14.0
System windows 10
Browser chrome 79.0.3945.88

解决方案: https://codesandbox.io/s/ji-ben-antd-4-19-1-forked-o62h13?file=/index.js

@MadCcc
Copy link
Member

MadCcc commented Mar 11, 2022

可以改成非受控的例子

@MadCcc MadCcc added help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request. 📝 Documentation labels Mar 11, 2022
@afc163
Copy link
Member

afc163 commented Mar 11, 2022

不应该改例子,这个是有问题的。#34351 (comment)

@github-actions
Copy link
Contributor

Hello @guiyu1230. We totally like your proposal/feedback, welcome to send us a Pull Request for it. Please send your Pull Request to proper branch (feature branch for the new feature, master for bugfix and other changes), fill the Pull Request Template here, provide changelog/TypeScript/documentation/test cases if needed and make sure CI passed, we will review it soon. We appreciate your effort in advance and looking forward to your contribution!

你好 @guiyu1230,我们完全同意你的提议/反馈,欢迎直接在此仓库 创建一个 Pull Request 来解决这个问题。请将 Pull Request 发到正确的分支(新特性发到 feature 分支,其他发到 master 分支),务必填写 Pull Request 内的预设模板,提供改动所需相应的 changelog、TypeScript 定义、测试用例、文档等,并确保 CI 通过,我们会尽快进行 Review,提前感谢和期待您的贡献。

giphy

@MadCcc MadCcc removed help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request. 📝 Documentation labels Mar 11, 2022
@MadCcc
Copy link
Member

MadCcc commented Mar 11, 2022

不应该改例子,这个是有问题的。#34351 (comment)

看起来是受控的 bug

@MadCcc
Copy link
Member

MadCcc commented Mar 11, 2022

Trace here: #34351

@guiyu1230
Copy link
Author

组件value受控属性判断应该是
'value' in prop ? props.value : state.value

@MoyuScript
Copy link

貌似是 React 自身的问题,可以看:https://codesandbox.io/s/dry-shadow-9rgoen?file=/src/App.js

@MadCcc
Copy link
Member

MadCcc commented Mar 11, 2022

貌似是 React 自身的问题,可以看:https://codesandbox.io/s/dry-shadow-9rgoen?file=/src/App.js

react 的行为没什么问题,undefined 就代表非受控状态。楼上可能是一种解决方案,但我想是不是把空字符串当空值好。。避免混用

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

undefined 在 React 原生组件里也是认为是非受控,不应该用 'value' in props 检查是否受控:
https://codesandbox.io/s/naughty-chaum-00suge?file=/src/App.js

@yoyo837
Copy link
Contributor

yoyo837 commented Mar 11, 2022

那这个呢 https://codesandbox.io/s/keen-bird-zid6qb

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

受控空值应该是 null,包括 clear 的时候 onChange 触发也应该是 null 而不是 undefined


Update:
clear 返回 null 可以在 minor 版本里改,另外 文档 例子的确是有问题的。

@MoyuScript
Copy link

MoyuScript commented Mar 11, 2022

那可以使用 null,但是目前 option 里头使用 null 值是合法的需要注意一下,也就是说如果有 value=null 的 option,清空会选中这个 option

@yoyo837
Copy link
Contributor

yoyo837 commented Mar 11, 2022

那这个呢 https://codesandbox.io/s/keen-bird-zid6qb

这个没受控。

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

那这个呢 https://codesandbox.io/s/keen-bird-zid6qb

这个没受控。

我意思是加个 null 作为显式受控

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

那可以使用 null,但是目前 option 里头使用 null 值是合法的需要注意一下,也就是说如果有 value=null 的 option,清空会选中这个 option

Hmmmm,我有点想起来为啥当年清空做成 undefined 的了。的确就是这个 null 可以作为 option 的问题。

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

之前重构的时候我就觉得这个 null 可以作为 option 会造成很多边界问题,但是为了兼容性不能动。重构还是保留了这个功能。我觉得在下个大版本可能是个时机去干掉这个。让 null 不再作为 option 的有效值。

@MadCcc
Copy link
Member

MadCcc commented Mar 11, 2022

Hmmmm,我有点想起来为啥当年清空做成 undefined 的了。的确就是这个 null 可以作为 option 的问题。

现在 value 还是只让填 string | number 吧。

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

@zombieJ zombieJ mentioned this issue Mar 11, 2022
@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

加到 Task 里,以防忘了 #34087

@guiyu1230
Copy link
Author

@zombieJ
Copy link
Member

zombieJ commented Mar 11, 2022

null as value: https://codesandbox.io/s/ji-ben-antd-4-19-1-forked-0rys3t?file=/index.js

出现一个有意思的东西 https://codesandbox.io/s/ji-ben-antd-4-19-1-forked-ue5o1r

嗯,undefined 不是有效值和 null 是有效值都是 by design 【手动斜眼】

@zombieJ zombieJ added the 🎱 Collaborate PR only Need confirm with Designer or Core member label Mar 15, 2022
@shezhangzhang
Copy link
Contributor

I'm working on it ! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎱 Collaborate PR only Need confirm with Designer or Core member Inactive
Projects
None yet
Development

No branches or pull requests

7 participants