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

Fix react16 warning #25

Merged
merged 3 commits into from
Jun 14, 2017
Merged

Fix react16 warning #25

merged 3 commits into from
Jun 14, 2017

Conversation

silentcloud
Copy link
Member

No description provided.

@silentcloud silentcloud changed the title [WIP]Fix react16 Fix react16 May 31, 2017
@silentcloud silentcloud changed the title Fix react16 Fix react16 warning May 31, 2017
@silentcloud
Copy link
Member Author

这个谁处理一下?

@paranoidjk
Copy link
Member

npm owner ls rmc-picker      
silentcloud <rjmuqiang@gmail.com>
yiminghe <yiminghe@gmail.com>

@paranoidjk
Copy link
Member

报错的问题解决了?

@silentcloud
Copy link
Member Author

花了好久时间,没定位到问题,试着将 rc-tools 改成和 antd-tools 一样的配置(ts 版本等)都不行,囧

@paranoidjk
Copy link
Member

那 CI 为什么是过的?

或者干脆改成 es6 算了。

另外我没有 npm owner 权限

@silentcloud
Copy link
Member Author

改成 es6了,改了好多

另:@paranoidjk 给你加 owner 了

const MultiPicker = React.createClass<MultiPickerProps, any>({
mixins: [MultiPickerMixin],
class MultiPicker extends React.Component<MultiPickerProps, any> {
getValue: () => any;
Copy link
Member

Choose a reason for hiding this comment

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

这里应该不需要定义吧?

Copy link
Member Author

Choose a reason for hiding this comment

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

不定义就会报错,getValue 在 mixin 里

@@ -38,11 +40,11 @@ const PopupPicker = React.createClass<IPopupPickerProps, any>({
animationType="slide-up"
wrapStyle={styles.modal}
visible={this.state.visible}
onClose={this.onDismiss}
onClose={this.onDismiss.bind(this)}
Copy link
Member

Choose a reason for hiding this comment

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

都改成 es6 class, 加上 arrow function,应该不再需要 bind this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@paranoidjk 好多 mixin,不加找不到的,最开始就是没加,发现有问题才加了

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

这个 bind 跟 typescript mixin 没啥关系应该,报错的是代码逻辑,不是 ts

Copy link
Member

Choose a reason for hiding this comment

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

恩,我的意思是如无必要,可以去掉 bind

Copy link
Member Author

Choose a reason for hiding this comment

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

去掉 onDismiss 里的就执行不了,或者有其他方式避免?

@paranoidjk paranoidjk merged commit 7ee8cff into master Jun 14, 2017
@paranoidjk paranoidjk deleted the fix-react16 branch June 14, 2017 04:10
@paranoidjk
Copy link
Member

@silentcloud 你发版本吧。代码以后再优化算了。

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.

2 participants