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 16.3.2 to React 16.4.1 #7393

Merged
merged 4 commits into from Jun 20, 2018
Merged

Upgrade React 16.3.2 to React 16.4.1 #7393

merged 4 commits into from Jun 20, 2018

Conversation

abotteram
Copy link
Contributor

@abotteram abotteram commented Jun 20, 2018

Description

This PR updates react from version 16.3.2 to version 16.4.1.

Reason for this PR is because I would like to use react-test-renderer for testing components that use React's new forwardRef feature, which is not supported by enzyme. As you can read in the changelog for React 16.4, support for using react-test-renderer with forwardRef was added in this version.

Not being able to test components with forwardRef is currently blocking #6261.

Since this is a minor version bump no breaking changes are to be expected, and while testing this I did not encounter any unexpected behaviour.

How has this been tested?

The unit tests still work.

Screenshots

Types of changes

New feature.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@abotteram abotteram requested a review from a team June 20, 2018 08:18
@abotteram abotteram added the [Type] Enhancement A suggestion for improvement. label Jun 20, 2018
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

I don't think we should be specifying version ranges with the caret.

package.json Outdated
@@ -44,12 +44,12 @@
"prop-types": "15.5.10",
"querystringify": "1.0.0",
"re-resizable": "4.4.8",
"react": "16.3.2",
"react": "^16.4.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why the change to using caret? Locking down package versions as tightly as possible is a good idea, so I think this should be changed to 16.4.1 over ^16.4.1.

package.json Outdated
"react-autosize-textarea": "3.0.2",
"react-click-outside": "2.3.1",
"react-color": "2.13.4",
"react-datepicker": "1.4.1",
"react-dom": "16.3.2",
"react-dom": "^16.4.1",
Copy link
Member

Choose a reason for hiding this comment

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

Same here re: caret ranges.

@abotteram
Copy link
Contributor Author

@tofumatt Ah yes, excuse me. I fixed it.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

There are a few more occurrences of old version of React in php files. We need to update them before we proceed.

@@ -0,0 +1,146 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file should be removed. We don't want to maintain package lock files for individual packages. It's a bug with Lerna that it doesn't respect .npmrc file ... I observe that it happens where you run npm install when there are 2 different versions of the package in two package.json files.

@gziolo gziolo added the Framework Issues related to broader framework topics, especially as it relates to javascript label Jun 20, 2018
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Code looks good, let me give it a spin locally.

@gziolo gziolo dismissed tofumatt’s stale review June 20, 2018 11:01

It was addressed

@gziolo gziolo added this to the 3.1 milestone Jun 20, 2018
@gziolo
Copy link
Member

gziolo commented Jun 20, 2018

Sidenote: we probably should start using react-test-renderer more instead of Enzyme, because of all the issues Enzyme caused by not keeping up to date with React releases.

@gziolo gziolo merged commit c6cbfb0 into master Jun 20, 2018
@gziolo gziolo deleted the update/react-16.4.1 branch June 20, 2018 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants