Skip to content
This repository was archived by the owner on May 20, 2025. It is now read-only.

Conversation

@farwayer
Copy link
Contributor

@farwayer farwayer commented May 30, 2017

fix #865 #750

@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@msftclas
Copy link

@farwayer, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

Copy link

@chirag04 chirag04 left a comment

Choose a reason for hiding this comment

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

Maybe it's better explicitly ask for withRef option like how redux connect does

@farwayer
Copy link
Contributor Author

Ask for what? ref used internally only to make codePushStatusDidChange and codePushDownloadDidProgress functions work. With stateless components it is senselessly.

@chirag04
Copy link

Set the ref only if withRef option is provided and check if ref exists before using it in any function

@max-mironov max-mironov self-requested a review June 2, 2017 11:38
@max-mironov
Copy link
Contributor

Hi @farwayer , thank you so much for contribution, your PR looks good for me, I've also tested it one more time and haven't found any issues with this.

@chirag04 - could you please explain a little bit more your point - what are the benefits here with adding withRef option in such particular case?

@max-mironov
Copy link
Contributor

Guys - I believe we can merge this changes to Master and we can always enhance this in future if needed. Please feel free to voice your concerns or suggestions if any.

@max-mironov max-mironov merged commit 3d5bf1a into microsoft:master Jun 6, 2017
@farwayer farwayer deleted the ref-class-only branch June 7, 2017 11:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CodePush should avoid set ref if stateless component wrapped

4 participants