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

Upgrade React to v16.0.0 #965

Merged
merged 6 commits into from
Nov 6, 2017
Merged

Upgrade React to v16.0.0 #965

merged 6 commits into from
Nov 6, 2017

Conversation

salehhamadeh
Copy link
Contributor

@salehhamadeh salehhamadeh commented Oct 25, 2017

This PR only upgrades React to v16.0.0. It does not use any new features, aside from changing React.render to React.hydrate in ClientController. I thought about using React.renderToNodeStream instead of React.renderToString in renderMiddleware.getElements, but I was not sure what is the best way to do it. I was thinking of having a stream for every RootElement that are all read and poured into a higher-level stream. I do not know if this will work. If you have ideas let me know, and I can either include it in this PR or in a separate one.

I used react-codemod (https://github.com/reactjs/react-codemod) to refactor PropTypes and React.createClass. I tested all react-server-test-pages. The only test page that behaves differently is the Error Logging Tests Page. Instead of the buttons logging errors and staying on the page, they are completely removed. This is the expected behavior since the introduction of error boundaries in React 16 https://reactjs.org/blog/2017/07/26/error-handling-in-react-16.html

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2017

CLA assistant check
All committers have signed the CLA.

@@ -43,7 +43,7 @@
"pem": "^1.8.1",
"q": "1.4.1",
"raw-loader": "^0.5.1",
"react-hot-loader": "~1.3.0",
"react-hot-loader": "^3.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this version compatible with the older versions of React that are supported below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment below. Yes it is compatible, but we will lose client-side HMR because react-hot-loader now needs us to use module.hot.accept and rendering the component tree in our entrypoints rather than using the no longer supported ReactMount.
See step 2 here http://gaearon.github.io/react-hot-loader/getstarted/

React = require("react");
React = require('react'),
PropTypes = require('prop-types'),
createReactClass = require("create-react-class");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we avoid this new dependency by making FragmentDataCache an es6 class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I'll do that.

@salehhamadeh
Copy link
Contributor Author

Bo and I talked offline. We decided to update react-hot-loader to v3.1.1. Server-side hot reloading will continue to work, so making a change and refreshing the page will show the changes. However, client-side hot reloading is broken because React 16 does not have ReactMount, which react-hot-loader v1 depended on. I will open an issue for the broken client-side hot module replacement.

@@ -26,7 +26,7 @@
"redux": "^3.6.0"
},
"peerDependencies": {
"react": "~0.14.2 || ^15.1.0"
"react": "~0.14.9 || ^15.3.0 || ^16.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the older version numbers changing here?

Lerna will only hoist if version strings match exactly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prop-types is only compatible with these versions. In previous versions, React will log console warnings if we use prop-types with previous versions https://www.npmjs.com/package/prop-types

@gigabo gigabo merged commit a8ebc3c into redfin:master Nov 6, 2017
@gigabo gigabo added the enhancement New functionality. label Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants