Fixes #23690 - update react to 16.3 #5651

amirfefer · 2018-06-05T14:55:36Z

React 16.3 updates its lifecycle methods, and deprecated

componentWillMount
componentWillReceiveProps
componentWillUpdate

Katello uses the new getDerivedStateFromProps lifecycle method.
Also, react-patternfly 2.5.1 uses react 16.3.1 as a peer dependency

theforeman-bot · 2018-06-05T14:55:44Z

Issues: #23690

amirfefer · 2018-06-05T14:58:11Z

webpack/assets/javascripts/react_app/components/BreadcrumbBar/components/BreadcrumbSwitcher.js

@@ -9,10 +9,10 @@ import BreadcrumbSwitcherPopover from './BreadcrumbSwitcherPopover';
 import BreadcrumbSwitcherToggler from './BreadcrumbSwitcherToggler';

 class BreadcrumbSwitcher extends React.Component {
-  componentWillReceiveProps(nextProps) {


componentWillReceiveProps is depracted, according to react documentations:

...This (componentDidUpdate) is also a good place to do network requests as long as you compare the current props to previous props

waldenraines · 2018-06-05T14:58:57Z

I believe we need this for 1.18 and will also need the relevant packages built.

mmoll · 2018-06-05T15:00:33Z

I believe we need this for 1.18

Why?

waldenraines · 2018-06-05T15:04:32Z

Why?

Because it fixes an issue in katello which is dependent on the version defined here. @amirfefer please correct me if I'm wrong.

amirfefer · 2018-06-05T15:12:03Z

@waldenraines
You are correct, as long as Katello will use the new subscriptions page in the upcoming version.
so in order that we won't have two distinct react versions, I think we need it on 1.18

boaz0

LGTM 👍

ekohl · 2018-06-07T08:20:41Z

We have multiple libraries that are used which have a peerDependency that isn't satisfied or worse: an actual dependency on react that wouldn't be satisfied. It'd be nice if we also looked into that and provided upstream patches where needed.

amirfefer · 2018-06-10T10:49:18Z

[test foreman]

waldenraines · 2018-06-12T01:15:42Z

[test katello]

ohadlevy · 2018-06-12T10:54:00Z

@ekohl do you have that list handy? I tried running it myself:

 npm i                                                                                                                                                git:(5651|… 

> TheForemanDevDeps@1.19.0 postinstall /home/ohad/git/foreman
> node ./script/npm_install_plugins.js

npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.0 (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.0: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

added 84 packages from 63 contributors in 14.007s

and as you can see I didn't get any warning? IMHO regardless, we should merge this.
since this is required for backporting I am ok with merging as is, but imho we should merge to latest version of react which is already at 16.4.

ekohl · 2018-06-12T12:32:15Z

@ekohl do you have that list handy?

Not at the moment. I'll have a look later.

I should note that I had some issues with packaging this. npm2rpm didn't understand the dependencies of d3 and I haven't had time to look into why. For some reason it can't figure out what the correct URL to download the tarballs from. There's probably some library that could do a better job than our manual way.

ohadlevy · 2018-06-12T13:06:16Z

@ekohl I'm not sure I understand exactly the issue you are facing? I can't imagine that react y upgrade would relate to d3?

ekohl · 2018-06-12T13:26:03Z

@ohadlevy it looks like I wasn't quite awake and mixing up react and patternfly. Apologies.

ohadlevy · 2018-06-12T13:29:43Z

@ekohl mind approving then? :)

waldenraines · 2018-06-13T13:47:21Z

@ekohl mind approving then? :)

Or @tstrachota? Would like to get this merged soon so I can merge the associated katello PR Katello/katello#7429.

ekohl · 2018-06-13T13:49:16Z

package.json

@@ -75,10 +75,10 @@
    "patternfly": "^3.42.0",
    "patternfly-react": "^2.3.3",
    "prop-types": "^15.6.0",
-    "react": "^16.2.0",
+    "react": "^16.3.1",


This also allows react 16.4.0 which has been released. Should I package that or did you intend ~16.3.1?

I intended 16.3.1, but I don't mind to package 16.4.0

If it stays ^16.3.1 then I'll package 16.4.0 since that's what Travis will also install while testing.

ekohl

theforeman/foreman-packaging#2649 is for react. react-dom is already 16.4.0.

tbrisker · 2018-06-14T12:41:10Z

if we need this in 1.18-stable please open a CP PR

waldenraines · 2018-06-14T19:12:39Z

if we need this in 1.18-stable please open a CP PR

@amirfefer do you mind handling this?

Fixes #23690 - update react to 16.3

4d5a371

theforeman-bot added Needs testing Not yet reviewed labels Jun 5, 2018

amirfefer commented Jun 5, 2018

View reviewed changes

amirfefer mentioned this pull request Jun 5, 2018

Fixes #23790 - Upgrade and pin patternfly-react #5641

Merged

waldenraines mentioned this pull request Jun 6, 2018

Fixes #23691 - upgrade react to 16.3 Katello/katello#7429

Merged

boaz0 approved these changes Jun 7, 2018

View reviewed changes

theforeman-bot removed the Not yet reviewed label Jun 7, 2018

ohadlevy approved these changes Jun 12, 2018

View reviewed changes

ekohl reviewed Jun 13, 2018

View reviewed changes

tbrisker added this to the 1.18.0 milestone Jun 13, 2018

ekohl approved these changes Jun 13, 2018

View reviewed changes

ohadlevy merged commit c50b21e into theforeman:develop Jun 13, 2018

tbrisker removed this from the 1.18.0 milestone Jun 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #23690 - update react to 16.3 #5651

Fixes #23690 - update react to 16.3 #5651

amirfefer commented Jun 5, 2018 •

edited

theforeman-bot commented Jun 5, 2018

amirfefer Jun 5, 2018

waldenraines commented Jun 5, 2018

mmoll commented Jun 5, 2018

waldenraines commented Jun 5, 2018

amirfefer commented Jun 5, 2018

boaz0 left a comment

ekohl commented Jun 7, 2018

amirfefer commented Jun 10, 2018

waldenraines commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

ekohl commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

ekohl commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

waldenraines commented Jun 13, 2018

ekohl Jun 13, 2018 •

edited

amirfefer Jun 13, 2018

ekohl Jun 13, 2018

ekohl left a comment

tbrisker commented Jun 14, 2018

waldenraines commented Jun 14, 2018

Fixes #23690 - update react to 16.3 #5651

Fixes #23690 - update react to 16.3 #5651

Conversation

amirfefer commented Jun 5, 2018 • edited

theforeman-bot commented Jun 5, 2018

amirfefer Jun 5, 2018

Choose a reason for hiding this comment

waldenraines commented Jun 5, 2018

mmoll commented Jun 5, 2018

waldenraines commented Jun 5, 2018

amirfefer commented Jun 5, 2018

boaz0 left a comment

Choose a reason for hiding this comment

ekohl commented Jun 7, 2018

amirfefer commented Jun 10, 2018

waldenraines commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

ekohl commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

ekohl commented Jun 12, 2018

ohadlevy commented Jun 12, 2018

waldenraines commented Jun 13, 2018

ekohl Jun 13, 2018 • edited

Choose a reason for hiding this comment

amirfefer Jun 13, 2018

Choose a reason for hiding this comment

ekohl Jun 13, 2018

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

tbrisker commented Jun 14, 2018

waldenraines commented Jun 14, 2018

amirfefer commented Jun 5, 2018 •

edited

ekohl Jun 13, 2018 •

edited