relay-tools / relay-hooks

Use Relay as React hooks
https://relay-tools.github.io/relay-hooks/docs/relay-hooks.html
MIT License
540 stars 56 forks source link

React warning: "Can't perform a React state update on a component that hasn't mounted yet" #233

Closed alex-statsig closed 1 year ago

alex-statsig commented 1 year ago

When using React 18 with Suspense, I get a warning "Can't perform a React state update on a component that hasn't mounted yet. This indicates that you have a side-effect in your render function that asynchronously later calls tries to update the component. Move this work to useEffect instead.".

I'm guessing the query is finishing while the component is suspended, and this library is triggering state updates when it's not really mounted. It is nice that the library starts querying right away before its technically mounted, but should refrain from actually updating state I think

morrys commented 1 year ago

Can you create a minimal sample project that reproduces the error so I can investigate?

gino commented 1 year ago

Having the same issues here unfortunately.

Using Vite (4.1.0), with vite-plugin-relay (2.0.0) with React (18.2.0) and relay-hooks (7.2.0).

CleanShot 2023-03-02 at 13 14 15@2x

Edit: I realized that the error disappears when I use useLazyLoadQuery instead of useQuery. Also, I was trying to create a sample project to reproduce the error but the error doesn't show up for some reason in that project 😅

morrys commented 1 year ago

Hi @gino, I have many tests that try the render problems usingLazyLoadQuery and none detect these problems. Not even my example project https://github.com/relay-tools/relay-hooks/tree/master/examples/suspense/nextjs-ssr which I also just upgraded to the latest versions of react, nextjs and relay-hooks.

To better understand the problem I need a minimal example project where I can check it out

alex-statsig commented 1 year ago

I'm trying to set up a repro but it seems maybe not as simple as I expected. I had thought the cause was straightforward: a component suspending while the useQuery hook has a request in flight. I'll try and get something together soon.

alex-statsig commented 1 year ago

Ah looks like I've done it, I'm not sure why one of the changes was necessary. Also seemed to uncover a (likely related) side-effect of the issue, that the component will repeatedly re-render/re-query while suspended.

https://github.com/alex-statsig/relay-hooks-bug-repro

morrys commented 1 year ago

Hi @alex-statsig, I just saw the repo now. Sure it's a very special case as you use both useQuery and useLazyLoadQuery.

The problem derives from the fact that useLazyLoadQuery is suspended and therefore also the component with useQuery but it is neither mounted nor unmounted so when the useQuery request ends it forces the update (warning).

This update causes useLazyLoadQuery to be rendered again and suspended again, this loop repeats until useLazyLoadQuery has completed the query.

alex-statsig commented 1 year ago

Hi @alex-statsig,

I just saw the repo now. Sure it's a very special case as you use both useQuery and useLazyLoadQuery.

The problem derives from the fact that useLazyLoadQuery is suspended and therefore also the component with useQuery but it is neither mounted nor unmounted so when the useQuery request ends it forces the update (warning).

This update causes useLazyLoadQuery to be rendered again and suspended again, this loop repeats until useLazyLoadQuery has completed the query.

Yes, that sounds correct. I use both as I prefer suspense APIs, but unfortunately react-relay doesn't provide a way to opt out for things like autocomplete options (which don't need to suspend the whole UI).

I think it's worth trying to fix as there may be many other reasons an app suspends other than mixing these two packages.

morrys commented 1 year ago

the only way is to allow the update only after the components have mounted.

The solution is also simple. Just change the forceUpdate:

import { useCallback, useEffect, useReducer, useRef } from 'react';
export function useForceUpdate() {
  var _a = useReducer(function (x) {
    return x + 1;
  }, 0),
      forceUpdate = _a[1];

  var isMounted = useRef(false);
  useEffect(function () {
    isMounted.current = true;
    return function () {
      isMounted.current = false;
    };
  }, []);
  var update = useCallback(function () {
    if (isMounted.current) {
      forceUpdate();
    }
  }, [isMounted, forceUpdate]);
  return update;
}

The tests run successfully and your project works fine too, but at the moment I have doubts that there may be other cases missing that I need to check

morrys commented 1 year ago

fixed in version 8.0.0 https://github.com/relay-tools/relay-hooks/releases/tag/v8.0.0