-
Notifications
You must be signed in to change notification settings - Fork 494
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
update react refs to use callback instead of strings #282
Conversation
for more info see the documentation at https://facebook.github.io/react/docs/refs-and-the-dom.html
This is a vital PR that unblocks devs that are using React 16, along with react-component/select#212 and react-component/trigger#63 @afc163 can we please get these merged asap? |
src/Calendar.jsx
Outdated
@@ -202,6 +202,9 @@ const Calendar = createReactClass({ | |||
showTimePicker: false, | |||
}); | |||
}, | |||
saveDateInput(dateInput) { | |||
this.dateInputInstance = dateInput; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this ref is not going to be used, just remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed, thanks for taking a look
src/RangeCalendar.js
Outdated
@@ -413,6 +413,10 @@ const RangeCalendar = createReactClass({ | |||
return month.isSameOrBefore(value[0], 'month'); | |||
}, | |||
|
|||
saveRoot(root) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this saveRoot
function to commonMixin
? since this.rootInstance
is used in commonMixin
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved, had to remove it from CalendarMixin as well
thanks!
maintainers:
[ 'benjycui <[email protected]>',
'yiminghe <[email protected]>' ], |
@benjycui |
remove saveRoot from Calendar and CalendarMixin
$ npm owner ls rc-calendar
paranoidjk <[email protected]>
benjycui <[email protected]>
yiminghe <[email protected]> |
$ npm info rc-calendar
...
maintainers:
[ 'benjycui <[email protected]>',
'yiminghe <[email protected]>' ],
... |
It seems that it is a bug of npm, just try to publish first. |
= = 这个就直接升 major 了啊。 |
@yesmeck 不敢保证是否有用户会使用我们内部的 ref,稳妥起见这边升 major。 ant-design/ant-design#3979 (comment) 这不是你自己写的么 😅 |
是的,但是这个不是公开的 API。 |
这里升 major 的话 antd 那边就很尴尬啊, antd 也得升 major? |
antd 我感觉不用, antd 更上层, api 和使用文档什么的相对比较固定和清晰 ant-design/ant-design@bcb82a5 rc-component 这层严格保证 semver,牵扯面太广了 |
As of React v16 using strings to create refs will no longer work. This is is forward looking change to address future compatibility issues as well as fixing existing deprecation warning thrown by react v15.
For more info on refs, see https://facebook.github.io/react/docs/refs-and-the-dom.html