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

Bump React to 16.3.2 to fix IE11 warning #6664

Merged
merged 2 commits into from
May 10, 2018

Conversation

brandonpayton
Copy link
Member

Description

This PR bumps our React version to 16.3.2 to address the following warning seen in IE11:
screen shot 2018-05-08 at 6 37 12 am

React 16.3.2 uses the Map( iterable ) constructor to create a map of valid Fragment props. This is a
problem in IE11 because IE11 supports a form of Map but not the constructor taking the iterable. The result is that the map is empty and React warns that children is not a valid Fragment prop. React 16.3.1 and later contain a fix for this, and I'm bumping to React 16.3.2 as the latest point release.

The React issue:
facebook/react#12500

The fix:
facebook/react#12504

How has this been tested?

  • Ran unit tests.
  • Ran e2e tests.
  • Loaded Gutenberg in IE11 and verified the warning is gone.
  • Manually worked with Gutenberg in Chrome as a quick check that nothing major appears broken with the version bump.

Checklist:

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

React 16.3.2 uses the `Map( iterable )` constructor
to create a map of valid Fragment props. This is a
problem in IE11 because IE11 supports a form of Map
but not the constructor taking the iterable. The result
is that the map is empty and React warns that `children`
is not a valid Fragment prop. React 16.3.1 and later
contain a fix for this, and I'm bumping to React 16.3.2
as the latest 16.3 version.
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

May bring a nice performance boost as well: facebook/react#12510 Curious if this is where the noticeable regression in development lately had started.

Confirmed appears to work well in IE11 and Chrome.

Travis seems grumpy today, probably a false positive (failing to install PHP?).

Do we need to update URLs in class-vendor-script-filename-test.php as well?

Otherwise LGTM 👍

@gziolo gziolo added this to the 2.9 milestone May 10, 2018
@gziolo gziolo added the Framework Issues related to broader framework topics, especially as it relates to javascript label May 10, 2018
@gziolo
Copy link
Member

gziolo commented May 10, 2018

Travis complains about PHP 5.2 targeted build, but everything looks good. Moving on regardless.

@gziolo
Copy link
Member

gziolo commented May 10, 2018

Do we need to update URLs in class-vendor-script-filename-test.php as well?

Done wite e1990eb.

@gziolo gziolo merged commit 90af4d1 into master May 10, 2018
@gziolo gziolo deleted the fix/ie11-fragment-props-warning branch May 10, 2018 07:57
@brandonpayton
Copy link
Member Author

Thanks for the review, @aduth, and for the update to class-vendor-script-filename-test.php, @gziolo!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants