Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Update react, withProps coverage, context api migration #474

Merged
merged 5 commits into from Oct 16, 2019

Conversation

tomtwttr
Copy link
Contributor

@tomtwttr tomtwttr commented Aug 16, 2019

This diff:

  1. Updates react
yarn add react
yarn add react-dom
yarn add react-addons-test-utils
  1. Adds code coverage to componentWillReceiveProps of FluxContainer. This is necessary before proper fix for UNSAFE_componentWillReceiveProps can be implemented. This relates to FluxContainer: Unsafe lifecycle methods were found within a strict-mode tree #452 .

  2. Updates to migrate to new Context API

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@tomtwttr tomtwttr changed the title tuster/452 Update react, withProps coverage, context api migration Aug 16, 2019
Copy link

@mariobanks mariobanks left a comment

Choose a reason for hiding this comment

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

LGTM

@waka
Copy link

waka commented Sep 5, 2019

This is the pull request with hope!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@tomtwttr tomtwttr marked this pull request as ready for review September 11, 2019 16:51
@tomtwttr
Copy link
Contributor Author

Hey @kyldvs , are you able to review?

@kyldvs
Copy link
Contributor

kyldvs commented Oct 12, 2019

Hey @tomtwttr, thanks for the PR! Sorry for the delay, I was holding off because it looked like a draft PR for a while :)

This looks good to me. Before I merge I will just need to make sure these changes don't cause any problems in our internal repos. This is because we like to have code on facebook org reflect what we are actually using in production, so I wouldn't want it to diverge too much. I will update you once I've pulled this in and checked.

@tomtwttr
Copy link
Contributor Author

Thanks @kyldvs , apologies for the draft mode, was waiting for that "CLA Signed" process to go through. I look forward to hearing your results.

@kyldvs kyldvs merged commit 466130c into facebookarchive:master Oct 16, 2019
@kyldvs
Copy link
Contributor

kyldvs commented Oct 16, 2019

Looks good :) thanks!

@tomtwttr tomtwttr deleted the tuster/452 branch October 16, 2019 23:37
@tomtwttr
Copy link
Contributor Author

@kyldvs What's the process for an NPM version bump?

@waka
Copy link

waka commented Mar 26, 2020

Isn't the version with this pull request published on npm?

@tomtwttr
Copy link
Contributor Author

@kyldvs Would you be able to bump npm?

Screen Shot 2020-03-26 at 8 50 58 AM

@waka
Copy link

waka commented Nov 13, 2020

@kyldvs @mariobanks Is this library no longer pushed to npm?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants