Switch hook on useMountedRef from useEffect to useLayoutEffect to support React 17

[lucabezerra]

Description

useEffect's cleanup function is no longer synchronous, so we switched to useLayoutEffect to preserve the behaviour of useMountedRef in React 17+.

Source: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing

Fixes #1959

Checklist

• I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

Tophat Instructions

On package.json, upgrade React and ReactDOM to the following:

    "react": "17.0.2",
    "react-dom": "17.0.2",

And the resolutions to:

    "@types/react": "17.0.11",
    "@types/react-dom": "17.0.8"

Then run yarn && yarn test react-hooks. The tests should pass!

[marutypes]

Checks out to me, but probably we want someone else to also review since I helped make it :P

[BPScott]

IIRC recall useLayoutEffect doesn't work on the server-side. If we make this change then I think we're going get a lot of logspam or errors about that.

#1813 seems to be a related issue where useEffect being async is a pain (though that was an issue in the mount rather than unmount phase). Perhaps that might offer some inspiration

Perhaps just using useIsomorphicLayoutEffect instead of useLayoutEffect is enough.

[marutypes]

@BPScott I'm not sure we need useMountedRef to work on the server, since "mounted" doesn't have a super clear meaning there. We could use the isomorphic version anyway I guess though.

[BPScott]

@TheMallen yep I agree this doesn't make much sense on the server as useLayoutEffect/useEffect doesn't do anything. however if you call useLayoutEffect on the server React emits the following warning to the console (while useEffect keeps quiet):

Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://fb.me/react-uselayouteffect-ssr for common fixes

My "it should work on the server" claim was really "it should not trigger loads of those warning logs for consumers" - which means we should either use useEffect or a noop function on the server side

[BPScott]

Gave this a tick . I'd love to see a test for this but I'm not sure how that would work, so that's not an issue for now.

[marutypes]

@BPScott the existing tests fail without this changeset, so we're covered imo

[BPScott]

Oh perfect! in that case ignore me :)