Firestore use*Once methods seem to be susceptible to race conditions and "state update on an unmounted component".

[mrtnkb]

This looks very similar to #35, but that was marked fixed in 2019.

For slow queries and fast UI changes it is possible for either the wrong result to be displayed or "state update on an unmounted component" warnings to be shown.

This seems to be caused by the async state update in the *Once method useEffects. setValue is called asynchronously and if the get responses are returned out of order, the value state will be set to the wrong result. It can also trigger an invalid call after a component is unmounted.

get(ref.current).then(setValue).catch(setError);

A possible solution would be to have these listen==false branches return a cleanup which would cause the get response to be ignored. A work around is switching to using the non-*Once methods.

react-firebase-hooks/firestore/useDocument.ts

Line 95 in 11aa6dc

get(ref.current).then(setValue).catch(setError);

react-firebase-hooks/firestore/useCollection.ts

Line 95 in 11aa6dc

get(ref.current).then(setValue).catch(setError);

[chrisbianca]

@mrtnkb how do you expect the get responses to be returned out of order? get returns the latest value of the document or collection once and only once.

The invalid call after a component is unmounted is a fair challenge, although is a no-op in React so won't actually harm your application.

[mrtnkb]

Isn't it because under the hood they're network requests? There aren't any guarantees on how long each will take to get serviced and of two queries placed in quick succession (e.g. in response to rapid UI state changes) the second query then clause could get called first - leaving setValue to be called with the now invalid response from the first query.

You can reproduce this problem if you set up a component that triggers different collection queries depending on a some UI state. If one state results in a simple query with only a few document responses and another state results in a complex query with a lot of document responses you can cause the hook to return the wrong value by toggling the state quickly.

One solution to both problems would be to track if the response corresponds to the current request:

    } else {
      let valid = true;
      const get = getDocsFnFromGetOptions(
        options ? options.getOptions : undefined
      );
      get(ref.current).then((value) => {
           if (valid) { setValue(value); }
        }).catch(setError);
      return () => { valid = false; };
    }

[chrisbianca]

@mrtnkb Ok, that makes sense. I was just struggling to see how you'd end up in a race condition, but you're right this could happen when you're changing the query within a component as described. I can take a look into this for a future release.

[mrtnkb]

That would be awesome! Or if you accept contributions/have a guide for new contributors, then I'd be happy to send a PR for you to look at.

[chrisbianca]

@mrtnkb more than happy to accept contributions and review them. There aren't any guidelines as such, just make sure you have prettier running if possible.

[chrisbianca]

This has been fixed and released as part of https://github.com/CSFrequency/react-firebase-hooks/releases/tag/v4.0.2

Thanks @mrtnkb for highlighting and resolving the issue