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

Making RHL more React 16.3 compatible #918 #927

Merged
merged 5 commits into from
Apr 18, 2018
Merged

Conversation

theKashey
Copy link
Collaborator

@theKashey theKashey commented Apr 6, 2018

removing componentWillReceiveProps #918, #860

"sync" flush moved to componentDidUpdate.
"render" also removes scheduled update, cos we are not "double rendering" updated components as we did before.
removing componentWillReceiveProps from AppContainer might change the way error overlay is shown, but we do not actually support that overlays in version 4.

TODO:

  • - add react-lifecycle-compact
  • - double manually test

@codecov-io
Copy link

codecov-io commented Apr 6, 2018

Codecov Report

Merging #927 into master will increase coverage by 0.16%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #927      +/-   ##
==========================================
+ Coverage    87.8%   87.96%   +0.16%     
==========================================
  Files          30       30              
  Lines         648      665      +17     
  Branches      151      154       +3     
==========================================
+ Hits          569      585      +16     
- Misses         64       65       +1     
  Partials       15       15
Impacted Files Coverage Δ
src/internal/stack/hydrateLegacyStack.js 0% <ø> (ø) ⬆️
src/reconciler/index.js 100% <ø> (ø) ⬆️
src/internal/stack/hydrateFiberStack.js 100% <ø> (ø) ⬆️
src/hot.dev.js 91.42% <ø> (ø) ⬆️
src/proxy/transferStaticProps.js 94.44% <ø> (ø) ⬆️
src/AppContainer.dev.js 82.35% <100%> (-1.86%) ⬇️
src/reconciler/proxyAdapter.js 100% <100%> (ø) ⬆️
src/proxy/createClassProxy.js 99.13% <100%> (+0.03%) ⬆️
src/reconciler/hotReplacementRender.js 88.53% <88.88%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36e8ac7...4f17b94. Read the comment docs.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Works fine

@gregberge
Copy link
Collaborator

Thanks for taking care of that!

@Fer0x
Copy link

Fer0x commented Apr 13, 2018

Steps to use this PR (for webpack users):

  1. Add dependency with "react-hot-loader": "gaearon/react-hot-loader#927/head" in package.json.
  2. Add to scripts directory in package.json following line:
    "postinstall": "(cd node_modules/react-hot-loader && yarn && yarn build)"
  3. Finally, in your webpack config, add this to resolve.alias section:
    react: './node_modules/react'

@colinrobertbrooks
Copy link

Thanks, @Fer0x. Will these steps still be required once this PR lands in master?

@theKashey
Copy link
Collaborator Author

@colinrcummings - sure not.
I'll try to handle this one today. 🤞

@theKashey
Copy link
Collaborator Author

🕷 do not merge. It's killing React 15.

@Fer0x
Copy link

Fer0x commented Apr 17, 2018

@theKashey what about release 5.x major version? 4.x will remain for React 15

@theKashey
Copy link
Collaborator Author

Should be ok by now. instanceof React.Component is not properly working to detect SFC, and I have no idea why.
As result ProxyComponent tracks it state by itself via static property. That property sometimes, on component update got copied from function to classes, and I have no idea why.

I've added "stack trace" to hot-render, to help myself debug this odd behavior. As result "the fix" - is just to add isStatelessFunctionalProxy to the RESERVED_KEYWORDS and stop coping it.
This fixes the issue, even if I did not found when and why my stateful components were soiled. So - something bad could still exists. But tests are green, and most complex application I've tried - also works well.

As result I keen stack tracing, as long it is still useful, as long RHL actually could not "merge children" from time to time. And, yet again, and I have no idea why.

Folks, you have few hours to test last version in React 15 (nothing was changed for React 16) and next I merge.

@theKashey
Copy link
Collaborator Author

There is SFC component related but, introduced by #873 :(
I will try to fix it, or have disable this feature (it is not in production yet)

@theKashey theKashey merged commit 948138a into master Apr 18, 2018
@gregberge gregberge deleted the react-16-strict branch April 24, 2018 13:26
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.

5 participants