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

Connect: Update for React 16 and 17 #10 #3

Open
basham opened this issue Jan 28, 2019 · 7 comments
Open

Connect: Update for React 16 and 17 #10 #3

basham opened this issue Jan 28, 2019 · 7 comments

Comments

@basham
Copy link
Member

basham commented Jan 28, 2019

csbasham commented on Apr 10, 2018


There's a lot of changes with React's lifecycle methods and asynchronous rendering, that's happening in React 16.3 and 17. Start to work on compatibility.

@basham
Copy link
Member Author

basham commented Jan 28, 2019

csbasham commented on Apr 27, 2018


componentWillReceiveProps is the only lifecycle method we'll need to figure out.

getDerivedStateFromProps is being added as a safer alternative to the legacy componentWillReceiveProps.

References:

@basham
Copy link
Member Author

basham commented Jan 28, 2019

csbasham commented on Apr 27, 2018


Perhaps there's a way to use getDerivedStateFromProps if it's there; otherwise, use componentWillReceiveProps. That way, it doesn't introduce a hard change.

@basham
Copy link
Member Author

basham commented Jan 28, 2019

csbasham commented on Apr 27, 2018


From what I can tell, there doesn't need to be any changes to prepare for async rendering. It just means that connect() may be simplified, because React, instead of RxJS, could handle rendering only on animation frame.

@basham
Copy link
Member Author

basham commented Jan 28, 2019

anderjak commented on Apr 30, 2018


If I'm understanding the docs correctly, for the work that's done in Conduit's connect function's componentWillReceiveProps we should be able to just replace componentWillReceiveProps with getDerivedStateFromProps, possibly with the addition of a return null; to indicate there should be no state update.

This could be another change for which we just have obvious documentation about which version of React works with which version of Conduit.

@basham
Copy link
Member Author

basham commented Jan 28, 2019

csbasham commented on Apr 30, 2018


Yes, I think the code impact will be minimal. But it does require the newer version of React. Perhaps v0.4 is the release to do that forced change?

@basham
Copy link
Member Author

basham commented Jan 28, 2019

anderjak commented on Apr 30, 2018


Based on what I've seen so far about async rendering I'm not sure that there isn't still some benefit to keeping the animationFrame-based stream emissions both for state and lifecycle functions. I think it might require some experimentation to see how well it works just relying on React to make all the rendering smooth.

@basham
Copy link
Member Author

basham commented Jan 28, 2019

anderjak commented on Apr 30, 2018


Forcing React changes with v0.4 of Conduit seems reasonable to me. I expect we'd want to bundle all the updates that require a React upgrade together. Maybe upgrading for React 16.3+ compatibility could be the main goal or just one of the goals for the v0.4 release.

james-anderson pushed a commit that referenced this issue Dec 14, 2021
related to #3 update for React 16 and 17
james-anderson added a commit that referenced this issue Jan 13, 2022
* fix #103. update dependencies
- related to #3 update for React 16 and 17
- mention RxJS import change in changelog
- remove max version from React peerDependency for conduit-rxjs-react
- combine rxjs imports into one line per file
- fix imports in non-CHANGLOG ".md" files
- add package-lock.json to .gitignore
- add link to RxJS v7.2 CHANGELOG
- fix examples. update CHANGELOG

Co-authored-by: James Anderson <[email protected]>
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

No branches or pull requests

1 participant