Replace componentWillReceiveProps with componentDidUpdate in docs and async component #4109

manvydasu · 2020-07-04T11:56:21Z

partial fix for 4094 (Remove UNSAFE methods from docs).

Other UNSAFE methods are removed with #4313

changeset-bot · 2020-07-04T11:56:23Z

⚠️ No Changeset found

Latest commit: 6bd0001

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

codesandbox-ci · 2020-07-04T11:57:51Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6bd0001:

Sandbox	Source
react-codesandboxer-example	Configuration

bladey · 2020-07-06T02:25:48Z

Thanks for this PR @manvydasu!

TrejoCode · 2020-07-08T03:12:42Z

<3

jleider · 2020-08-05T19:52:12Z

Any timeline on getting this merged and releasing a new version? With strict mode on this reports as a console error which winds up polluting the console with library based messaging making it hard to notice actual errors in your app.

xuniaoji · 2020-08-31T07:34:53Z

Do we have any plan to release a new version to fix this?

pervezalam83 · 2020-09-02T07:57:21Z

Please update when this PR will be merged. It seems we can not get rid of the "Warning: Using UNSAFE_componentWillReceiveProps in strict mode is not recommended and may indicate bugs in your code" until this is merged

brenobaptista · 2020-11-13T02:51:21Z

This PR would be very useful, please merge it if possible 😄 thanks, guys!

Methuselah96 · 2020-11-14T16:21:35Z

~~I've started a fork of react-select. Feel free to resubmit this PR on the fork and we can get it merged and released.~~

EDIT: 🎉 I've archived the fork now that we've got some momentum in this repo and Jed is involved again. Sorry for the disturbance!

Methuselah96

Thanks for this PR! As noted below, the work for Async will probably become redundant soon and the Header modification needs to be changed. Really appreciate your help!

Methuselah96 · 2020-12-12T01:42:23Z

docs/App/Header.js

-  UNSAFE_componentWillReceiveProps({ location }: HeaderProps) {
+  componentDidUpdate(prevProps: HeaderProps) {
    const valid = ['/', '/home'];
-    const shouldCollapse = !valid.includes(this.props.location.pathname);
-    if (location.pathname !== this.props.location.pathname && shouldCollapse) {
+    const shouldCollapse = !valid.includes(prevProps.location.pathname);
+
+    if (prevProps.location.pathname !== this.props.location.pathname && shouldCollapse) {


Doing this on update means that the first time you click away from the home page, the transition won't work (which is actually true of the status quo as well).

Instead I would suggest that we call this.setState({ contentHeight: this.content.scrollHeight }); immediately in componentDidMount and get rid of toggleCollapse altogether.

You're right. I removed toggleCollapse alltogether and removed contentHeight from the state itself. My suggested approach will get rid of one additional re-render after componentDidMount and the functionality remains the same.

@Methuselah96 I tested this locally and doesn't seem to impact anything negatively. Any other concerns or can this conversation be resolved?

packages/react-select/src/Async.js

ebonow · 2021-02-12T17:57:35Z

It seems a bit sluggish when transitioning between pages. I ran a profiler and it seems that it is noticable. It could also likely be due to these change being applied against v3 as opposed to v4 (perhaps giving some credit to the performance gains from the lifecycle method replacements from @Methuselah96) , but I'll try these changes against v4 and see if the differences flatten out.

manvydasu · 2021-02-12T18:46:05Z

@ebonow
Not sure how these changes could increase rendering time of the component/page.
I removed additional re-render and didn't introduce any setStates or anything.
I can rebase this PR on 4.* if that's needed.

ebonow · 2021-02-12T19:07:12Z

I just ran it against v4 and it seems negligible. Could just be an environmental difference between running local dev server against production server.

One of the changes I noticed that would show up if you rebase is that jsx is now instead imported from

import { jsx } from '@emotion/react';

Also, would you be opposed to helping us rid all of the lifecycle method warnings from the current docs?

Looks like a few of these components could use a version bump specifically in react-router.

Also you will likely want to run the npm changeset command so that we can make sure you are properly acknowledged.

ebonow · 2021-02-12T20:44:29Z

TLDR: Don't worry about the other dependencies...

Putting this here just to document findings...

Did some more research. Other dependencies causing warnings which need to be updated in both the package.json, and docs/package.json:

    "react-helmet": "^6.1.0",
    "react-router-dom": "^5.2.0",

Outside of that are the atlas dependencies, but these also introduce a peer dependency on styled-components:

    "@atlaskit/button": "^15.1.4",
    "@atlaskit/icon": "^21.2.4",
    "@atlaskit/modal-dialog": "^11.2.6",
    "@atlaskit/spinner": "^15.0.6",
    "@atlaskit/tooltip": "^17.1.2",
    "styled-components": "^5.2.1"

Started getting react key errors in the console, and we're replacing the documentation in the near future, so not worth putting more time into it.

Update header.js to use emotion/react jsx render method

Stale

Updated file with prettier formatting

JedWatson · 2021-03-18T22:42:49Z

Thanks for the PR @manvydasu and the help getting this done everyone, merging now 🙂

manvydasu force-pushed the fix/4094-partial branch 2 times, most recently from fa4e50b to bde4330 Compare July 4, 2020 12:06

bladey added pr/bug-fix PRs that are specifically addressing a bug pr/needs-review PRs that need to be reviewed to determine outcome labels Jul 6, 2020

bladey added pr/needs-review PRs that need to be reviewed to determine outcome and removed pr/needs-review PRs that need to be reviewed to determine outcome labels Aug 24, 2020

Methuselah96 previously requested changes Dec 12, 2020

View reviewed changes

manvydasu force-pushed the fix/4094-partial branch from bde4330 to d26c2e2 Compare December 26, 2020 19:49

4094 remove UNSAFE method from docs (Header.js)

fff94e7

manvydasu force-pushed the fix/4094-partial branch from d26c2e2 to fff94e7 Compare December 26, 2020 19:56

Merge branch 'master' into fix/4094-partial

664ba38

manvydasu requested a review from Methuselah96 December 26, 2020 20:04

ebonow added this to the 4.2 milestone Mar 4, 2021

ebonow removed the pr/needs-review PRs that need to be reviewed to determine outcome label Mar 5, 2021

Methuselah96 removed this from the 4.2 milestone Mar 5, 2021

ebonow and others added 2 commits March 5, 2021 11:07

Update header.js to use emotion/react jsx render method

6a79efc

Update header.js to use emotion/react jsx render method

Merge branch 'master' into fix/4094-partial

bbacc13

use Prettier

6bd0001

Updated file with prettier formatting

ebonow approved these changes Mar 8, 2021

View reviewed changes

ebonow added this to the 4.3 milestone Mar 8, 2021

ebonow added the category/documentation Issues or PRs about documentation or the website itself label Mar 10, 2021

JedWatson approved these changes Mar 18, 2021

View reviewed changes

JedWatson merged commit 41118ca into JedWatson:master Mar 18, 2021

ebonow mentioned this pull request Mar 19, 2021

Version Packages #4502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace componentWillReceiveProps with componentDidUpdate in docs and async component #4109

Replace componentWillReceiveProps with componentDidUpdate in docs and async component #4109

manvydasu commented Jul 4, 2020 •

edited

changeset-bot bot commented Jul 4, 2020 •

edited

codesandbox-ci bot commented Jul 4, 2020 •

edited

bladey commented Jul 6, 2020

TrejoCode commented Jul 8, 2020

jleider commented Aug 5, 2020

xuniaoji commented Aug 31, 2020

pervezalam83 commented Sep 2, 2020

brenobaptista commented Nov 13, 2020

Methuselah96 commented Nov 14, 2020 •

edited

Methuselah96 left a comment •

edited

Methuselah96 Dec 12, 2020

manvydasu Dec 26, 2020

ebonow Mar 5, 2021

ebonow commented Feb 12, 2021

manvydasu commented Feb 12, 2021

ebonow commented Feb 12, 2021 •

edited

ebonow commented Feb 12, 2021 •

edited

JedWatson commented Mar 18, 2021

Replace componentWillReceiveProps with componentDidUpdate in docs and async component #4109

Replace componentWillReceiveProps with componentDidUpdate in docs and async component #4109

Conversation

manvydasu commented Jul 4, 2020 • edited

changeset-bot bot commented Jul 4, 2020 • edited

⚠️ No Changeset found

codesandbox-ci bot commented Jul 4, 2020 • edited

bladey commented Jul 6, 2020

TrejoCode commented Jul 8, 2020

jleider commented Aug 5, 2020

xuniaoji commented Aug 31, 2020

pervezalam83 commented Sep 2, 2020

brenobaptista commented Nov 13, 2020

Methuselah96 commented Nov 14, 2020 • edited

Methuselah96 left a comment • edited

Choose a reason for hiding this comment

Methuselah96 Dec 12, 2020

Choose a reason for hiding this comment

manvydasu Dec 26, 2020

Choose a reason for hiding this comment

ebonow Mar 5, 2021

Choose a reason for hiding this comment

ebonow commented Feb 12, 2021

manvydasu commented Feb 12, 2021

ebonow commented Feb 12, 2021 • edited

ebonow commented Feb 12, 2021 • edited

JedWatson commented Mar 18, 2021

manvydasu commented Jul 4, 2020 •

edited

changeset-bot bot commented Jul 4, 2020 •

edited

codesandbox-ci bot commented Jul 4, 2020 •

edited

Methuselah96 commented Nov 14, 2020 •

edited

Methuselah96 left a comment •

edited

ebonow commented Feb 12, 2021 •

edited

ebonow commented Feb 12, 2021 •

edited