meteor / react-packages

Meteor packages for a great React developer experience
http://guide.meteor.com/react.html
Other
574 stars 159 forks source link

Can't perform a React state update on a component that hasn't mounted yet #386

Open make-github-pseudonymous-again opened 1 year ago

make-github-pseudonymous-again commented 1 year ago

In a useTracker-heavy part of my code I got the following warning with React 18 (react-meteor-data v2.6.2):

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.

Not sure if it shows up without <StrictMode/>. If I remember correctly I managed to get the warning both with the deps and no-deps versions of useTracker.

I have not been able to reproduce this behavior in a sandbox yet, and the original code was quite a pain to debug.

I managed to fix it by patching useTracker and useSubscribe to have skipUpdate return true when the component has been initialized but has not mounted yet, using the following hook and have skipUpdate return !hasMounted() || originalSkipUpdate():

import {useRef, useEffect} from 'react';

const useHasMounted = () => {
    const componentWasMounted = useRef(false);

    useEffect(() => {
        componentWasMounted.current = true;
    }, []);

    return () => componentWasMounted.current;
};

export default useHasMounted;

I assume the warning pops up because forceUpdate can be called before an element is mounted now that React can run state code without systematically rendering.

CaptainN commented 1 year ago

Are you using deps, or no deps with your useTracker implementation?

make-github-pseudonymous-again commented 1 year ago

Depends. Sometimes I do (useTracker(() => Users.findOne({_id}), [_id])) and sometimes I do not (useTracker(() => Meteor.userId())).

I had both in the code that was causing trouble. I had the warning until I removed all computations. I will try to go back to it and confirm that this happens with both implementations.

devkat commented 1 year ago

I am facing this issue as well. Calling forceUpdate only when the component is mounted (in useTrackerWithDeps) seems to solve the issue:

  useMemo(() => {
    // To jive with the lifecycle interplay between Tracker/Subscribe, run the
    // reactive function in a computation, then stop it, to force flush cycle.
    const comp = Tracker.nonreactive(() =>
      Tracker.autorun((c: Tracker.Computation) => {
        const data = refs.reactiveFn();
        if (c.firstRun) {
          refs.data = data;
        } else if (!skipUpdate || !skipUpdate(refs.data, data)) {
          refs.data = data;
          if (refs.isMounted === true) { // Avoid "Can't perform a React state update" warning
            forceUpdate();
          }
        }
      }),
    );
CaptainN commented 1 year ago

Are you trying to set state from within your computation? Can you share your useTracker example?

CaptainN commented 1 year ago

Basically, you shouldn't set state from within the useTracker computation. That should be considered an anti-pattern. If you do need to do something like that (and you probably don't), consider just creating the computation yourself in useEffect.

devkat commented 1 year ago

The warning is emitted in the following component. It doesn't use useTracker directly, only useSubscribe and useFind, so there are no custom side effects involved.

The warning is usually (maybe exclusively?) emitted directly after logging in.

function useNotificationCount() {
  const isLoading = useSubscribe('notifications.getForUser', { kind: 'course' });
  const notifications = useFind(() => Notifications.find({ kind: 'course' }, { sort: { lastChanged: -1 } }));

  const lastChange: number = isLoading() ? 0 : notifications[0]?.lastChanged.getTime() ?? 0;
  const notificationCount = notifications.length;

  return { notificationCount, lastChange };
}

export const MessageInboxCount = React.memo(function MessageInboxCount({
  offset,
}: MessageInboxCountProps) {
  const { notificationCount } = useNotificationCount();
  return <Badge count={notificationCount} offset={offset} />;
});

This causes the warning in useTrackerNoDeps, probably due to the forceUpdate call in the Tracker.nonreactive block.