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.

Sign up for GitHub

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

Jump to bottom

Update react-redux to v5.0.7 to fix a warning with the new component API #24422

Merged
merged 1 commit into from
Apr 24, 2018
Merged

Update react-redux to v5.0.7 to fix a warning with the new component API #24422

merged 1 commit into from
Apr 24, 2018

Conversation

Tug
Copy link
Contributor

@Tug Tug commented Apr 24, 2018 •
edited
Loading

Fixes the following warning printed in the console on the client:

Warning: Unsafe legacy lifecycles will not be called for components using new component APIs.

Connect(GoogleMyBusinessStatsChart) uses getDerivedStateFromProps() but also contains the following legacy lifecycles:
  componentWillReceiveProps
  componentWillUpdate

The above lifecycles should be removed. Learn more about this warning here:
https://fb.me/react-async-component-lifecycle-hooks

Encountered on #24236 when migrating the GoogleMyBusinessStatsChart component to use the new react component API.

See reduxjs/react-redux#915 (comment)

You can test that it solves the problem by applying #24236, going to /google-my-business/stats/:site and looking at the console

@Tug Tug self-assigned this Apr 24, 2018
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=update/react-reduc-507

@Tug Tug requested review from yurynix and stephanethomas April 24, 2018 12:08
@Tug Tug added the Framework label Apr 24, 2018
@Tug Tug requested review from ockham and jsnmoon April 24, 2018 12:15
@Tug Tug added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 24, 2018
@stephanethomas stephanethomas added this to the Google My Business: M2 milestone Apr 24, 2018
@stephanethomas
Copy link
Contributor

Changes look good, and work fine. I launched E2E tests, and they all passed.

@stephanethomas stephanethomas added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 24, 2018
@Tug Tug merged commit d9a35e4 into master Apr 24, 2018
@Tug Tug deleted the update/react-reduc-507 branch April 24, 2018 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurynix yurynix Awaiting requested review from yurynix

@stephanethomas stephanethomas Awaiting requested review from stephanethomas

@ockham ockham Awaiting requested review from ockham

@jsnmoon jsnmoon Awaiting requested review from jsnmoon

Assignees

@Tug Tug

Labels
Framework
Projects
None yet
Milestone
Google My Business: M2
Development

Successfully merging this pull request may close these issues.
3 participants
@Tug @matticbot @stephanethomas