Skip to content

Update react 15.6.1#13672

Merged
sorenlouv merged 8 commits intoelastic:masterfrom
sorenlouv:update-react-15.6.1
Aug 28, 2017
Merged

Update react 15.6.1#13672
sorenlouv merged 8 commits intoelastic:masterfrom
sorenlouv:update-react-15.6.1

Conversation

@sorenlouv
Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv commented Aug 23, 2017 •
edited
Loading

React was recently upgraded to 15.6.1 for x-pack and this PR ensures the version in Kibana matches.
Closes #13610

Breaking changes between 15.4.2 and 15.6.1:

Changes made:

Plugins affected:

Further reading:

@sorenlouv sorenlouv added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v6.1.0 labels Aug 23, 2017
@sorenlouv sorenlouv force-pushed the update-react-15.6.1 branch from a17deae to db5a608 Compare August 23, 2017 19:58
weltenwort
weltenwort reviewed Aug 24, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Awesome, almost there. There are just a few uses of React.PropTypes remaining in ui_framework/src/components, which produces deprecation warnings.

@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Aug 24, 2017 •
edited
Loading

@weltenwort Good catch. I only ran the codemod on the src folder, so missed the ui-framework files.

@epixa
Copy link
Copy Markdown
Contributor

epixa commented Aug 24, 2017

@sqren Can you add prop-types to the package.json? I'm not really sure how this is working at the moment, but I assume it just happens to be installed due to being a dependency of one of our dependencies.

@epixa
Copy link
Copy Markdown
Contributor

epixa commented Aug 24, 2017

Scratch that, I see it now. I must have typo'd my grep.

@epixa
Copy link
Copy Markdown
Contributor

epixa commented Aug 24, 2017 •
edited
Loading

I'm still seeing 3 warnings about PropTypes usage in the jest tests:

 PASS  ui_framework/src/components/color_picker/color_picker.test.js
  ● Console

    console.warn node_modules/react/lib/lowPriorityWarning.js:40
      Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in  React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs

 PASS  ui_framework/src/components/popover/popover.test.js
 PASS  ui_framework/src/components/button/submit_button.test.js
 PASS  ui_framework/src/components/collapse_button/collapse_button.test.js
 PASS  ui_framework/src/components/tool_bar/tool_bar_search_box.test.js
 PASS  ui_framework/src/components/table/table.test.js
 PASS  ui_framework/src/components/accessibility/screen_reader.test.js
 PASS  src/core_plugins/kibana/public/dashboard/top_nav/clone_modal.test.js
  ● Console

    console.warn node_modules/react/lib/lowPriorityWarning.js:40
      Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in  React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs

 PASS  ui_framework/src/components/modal/confirm_modal.test.js (8.63s)
  ● Console

    console.warn node_modules/react/lib/lowPriorityWarning.js:40
      Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in  React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs

 PASS  ui_framework/src/components/button/button_icon/button_icon.test.js

Edit: node scripts/jest by the way

@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Aug 24, 2017 •
edited
Loading

I'm still seeing 3 warnings about PropTypes usage in the jest tests:

@epixa Yes, I'm not sure where those are coming from - I'm fairly certain it's not from the Kibana. Could be a dependency that we have to bump.

@weltenwort
Copy link
Copy Markdown
Member

weltenwort commented Aug 24, 2017

I couldn't find the source either, so I think a third-party dependency is the probable cause too.

weltenwort
weltenwort approved these changes Aug 24, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

I would be fine with tackling those warnings in a separate PR.

@cjcenizal
Copy link
Copy Markdown
Contributor

cjcenizal commented Aug 24, 2017

I think these warning are coming from the enzyme dependency. Looked like they were addressed in 2.8.1 (enzymejs/enzyme#876, enzymejs/enzyme@75d1390).

@cjcenizal
Copy link
Copy Markdown
Contributor

cjcenizal commented Aug 24, 2017

Oh wait, we upgraded enzyme in this PR... odd.

cjcenizal
cjcenizal approved these changes Aug 24, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

LGTM! I think I've narrowed down the source of these warnings to be our react-color dependency, which we use in the KuiColorPicker component here: https://github.com/elastic/kibana/blob/master/ui_framework/src/components/color_picker/color_picker.js#L4.

Removing this line and updating this component to not depend upon it makes all of the warnings go away. Which is weird because I don't see the relationship between this component and the modals.

Bumping react-color to 2.11.7 should fix this: casesandberg/react-color#346.

@weltenwort
Copy link
Copy Markdown
Member

weltenwort commented Aug 25, 2017

wow, thanks for taking the time to dig into this, @cjcenizal

@sorenlouv sorenlouv force-pushed the update-react-15.6.1 branch from 01fd862 to 2864a6d Compare August 27, 2017 17:45
@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Aug 27, 2017 •
edited
Loading

@cjcenizal Great finding, thanks! I've bumped react-color and it now passes without warnings locally. If it passes CI I'll merge it in.

@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Aug 27, 2017

jenkins test this

@sorenlouv sorenlouv merged commit 52552a9 into elastic:master Aug 28, 2017
@sorenlouv sorenlouv deleted the update-react-15.6.1 branch August 28, 2017 08:27
@weltenwort
Copy link
Copy Markdown
Member

weltenwort commented Aug 28, 2017

awesome, thanks again!

sorenlouv added a commit to sorenlouv/kibana that referenced this pull request Aug 28, 2017
@sorenlouv
Update react 15.6.1 (elastic#13672)
255b186
@sorenlouv sorenlouv mentioned this pull request Aug 28, 2017
Merged
sorenlouv added a commit that referenced this pull request Aug 28, 2017
@sorenlouv
Update react 15.6.1 (#13672) (#13724)
3095b3b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@weltenwort weltenwort weltenwort approved these changes

@epixa epixa Awaiting requested review from epixa

+1 more reviewer

@cjcenizal cjcenizal cjcenizal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v6.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Upgrade react to 15.6.1

4 participants

@sorenlouv @epixa @weltenwort @cjcenizal