react-meteor-data useTracker can sometime return undefined

[bhunjadi]

I'm experiencing issues with react-meteor-data v2.2.0 and higher with useTracker which returns undefined instead of the expected array value which is returned from function passed to useTracker.

For example, I have something like this:

const [data, isReady] = useTracker<[ResultType[], boolean]>(() => {
    const subscriptionHandle = query.subscribe();
    const isReady = subscriptionHandle.ready();
    const data = isReady ? query.fetch() : [];
    return [data, isReady];
}, deps);

It works with 2.1.4 just fine, but with newer version it fails almost in 100% of the cases. It is not easy to figure out what causes this exactly because in our app we have a complex rendering tree. In isolation, the component works ok.

In the source, I suspect the way value is returned in useTrackerWithDeps might be an issue. This could point to the useMemo which could perhaps fail to set the data variable.

Any suggestions what to try next to debug this properly?

[CaptainN]

What is in your deps array?

It should return your value at least the first time in every case, and always after the component has been mounted.

The only reason I can see it getting confused is if your deps change on every render. Even then, it should never return undefined.

The only way I can see that happening would be if the render runs more than one time before mount, but I don't think React ever does that. Still, I could probably mitigate against that.

Are you using concurrent mode or strict mode?

[CaptainN]

Try this one: https://github.com/meteor/react-packages/blob/better-handle-data-with-refs/packages/react-meteor-data/useTracker.ts

I still can't see why you'd be getting undefined..

[bhunjadi]

Thanks for your response!

I'm not using strict nor concurrent mode. I tried the new useTracker, but still have the same error.

The actual code looks like this:

// constants, they are not changed at all
projectId: string = 'some id'
limit: number = 10
skip: number = 0

// Added just to check when the component is mounted and unmounted, to verify that useMemo is really called on each rerender.
useEffect(() => {
        console.log('mounted');
        return () => {
             // this is not called, therefore component is not unmounted
            console.log('unmounted');
        };
    }, []);

const queryParams = useMemo(() => {
        // even when these variables are not changed, this function is called
        console.log(projectId, skip, limit);
        return {
            projectId,
            limit,
            skip,
        };
    }, [projectId, skip, limit]);

    const [items = []] = useTracker(() => {
        // getProjectTimelineQuery is Grapher query
        const query = getProjectTimelineQuery.clone(queryParams);
        const subscriptionHandle = query.subscribe();
        const isReady = subscriptionHandle.ready();
        const data = isReady ? query.fetch() : [];
        return [data, isReady];
    }, [queryParams]);

   return (...);

Interesting thing I noticed is that useMemo does not use cache in this instance, i.e. even if deps are not changed, the memo function is called. I think this is what is described in the React docs:

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

The only thing bothering me is that it says "in the future".

My guess is that useMemo behaves in the same way in the useTrackerWithDeps. Does this help in any way? Looking at the useTracker implementation, I can't tell if this changes anything.

[CaptainN]

What version of React are you using, and how much system memory do you have? I don't think I've seen a case where useMemo is running on every render regardless of deps...

Still, this should not cause the error you are reporting. If useMemo is getting called every time, it should be returning your data from that tracker instance.

[CaptainN]

The only case I can think of that might be causing 'undefined' (though the hook I provided above should fix this) would be if you are triggering immediate rerender by invoking setState or similar in your render method before the first render completes. Other than that, I have no idea what's goin on here.

[bhunjadi]

React version is 16.13.1 and I have 64 gigs of ram so this should not be the problem. But digging deeper into the issue, I now think I was wrong with useMemo conclusion. You seem to be on the right path for this issue with what you said about rerenders.

However, it seems that rerenders are not causing the bugs but unmounting and mounting of the component.

I had something like this

export function useReactiveQuery<ResultType = any>(queryFn: () => QueryResult<ResultType>, deps: any[] = []): [ResultType[], any, boolean] {
    const [error, setError] = useState(null);

    // eslint-disable-next-line react-hooks/exhaustive-deps
    const query = useMemo(() => queryFn(), deps);
    const [data, isReady] = useTracker<[ResultType[], boolean]>(() => {
        const subscriptionHandle = query.subscribe({
            onStop(err) {
                if (err) {
                    setError(err);
                }
            },
            onReady() {
                setError(null);
            },
        });

        const isReady = subscriptionHandle.ready();

        const data = isReady ? query.fetch() : [];
        return [data, isReady];
    }, deps);

    return [data, error, !isReady];
}

When component using useTracker is unmounted before the query becomes ready then onReady can still trigger and using setError (or any state variable) here can cause an issue.

I changed that function into

export function useReactiveQuery<ResultType = any>(queryFn: () => QueryResult<ResultType>, deps: any[] = []): [ResultType[], any, boolean] {

    // using ReactiveVar here
    const [subscriptionError] = useState(() => new ReactiveVar<Error | null>(null));

    // eslint-disable-next-line react-hooks/exhaustive-deps
    const query = useMemo(() => queryFn(), deps);
    const [data, isReady] = useTracker<[ResultType[], boolean]>(() => {
        const subscriptionHandle = query.subscribe({
            onStop(err) {
                if (err) {
                    subscriptionError.set(err);
                }
            },
            onReady() {
                subscriptionError.set(null);
            },
        });

        const isReady = subscriptionHandle.ready();

        const data = isReady ? query.fetch() : [];
        return [data, isReady];
    }, deps);

    // Using separate tracker not to re-run the query tracker above
    const error = useTracker(() => {
        return subscriptionError.get();
    }, [subscriptionError]);
    return [data, error, !isReady];
}

Closing this as it does not look like react-meteor-data issue. Not sure how that worked before with 2.1.4, though. Sorry for the confusion and thanks for you input.

[CaptainN]

That's interesting. The reason it probably worked before is that 2.2.2 will now intentionally create and discard a computation immediately in the first render, then recreate it on mount in useEffect. The reason for this change is that the old hook would retain one computation between first render and mount, but the code was super complicated, and there were still some edge cases where it would fail to recycle, and other edge cases where some things would get lost - subscriptions in particular.

But actually, your hook should be just fine, even with that change, so something else is probably going on here.

I'm not sure what your queryFn does, but if it's a reactive function (looks like it runs a mongo query), you'll definitely want to run that within your tracker function. I'm not sure why you are memoizing the result of that, but useTracker does almost exactly the same internally.

export function useReactiveQuery<ResultType = any>(queryFn: () => QueryResult<ResultType>, deps: any[] = []): [ResultType[], any, boolean] {

    // using ReactiveVar here
    const [subscriptionError] = useState(() => new ReactiveVar<Error | null>(null));

    const [data, isReady] = useTracker<[ResultType[], boolean]>(() => {
        const qRes = queryFn();
        const subscriptionHandle = qRes.subscribe({
            onStop(err) {
                if (err) {
                    subscriptionError.set(err);
                }
            },
            onReady() {
                subscriptionError.set(null);
            },
        });

        const isReady = subscriptionHandle.ready();

        const data = isReady ? qRes.fetch() : [];
        return [data, isReady];
    }, deps);

    // Using separate tracker not to re-run the query tracker above
    const error = useTracker(() => {
        return subscriptionError.get();
    }, [subscriptionError]);
    return [data, error, !isReady];
}

You probably also don't need that second one off useTracker for the error. Have you measured performance to determine a need for all this caching? I'd probably put all this directly in to the one useTracker.

[Floriferous]

We're seeing the same issue after upgrading to Meteor 2.0 and react-meteor-data 2.2.2.

Our previous custom hook looked like this:

export const useReactiveMeteorData = (
  { query, params, type = 'many' },
  deps = [],
) => {
  const [error, setError] = useState(undefined);
  const finalQuery = getQuery(query, params);

  const { loading, subscribedQuery } = useTracker(() => {
    if (!finalQuery) {
      return { loading: false };
    }

    const handle = finalQuery[getSubscriptionFunction(type)]({
      onStop(err) {
        if (err) {
          setError(err);
        }
      },
      onReady() {
        setError(undefined);
      },
    });
    const isReady = handle.ready();

    return { loading: !isReady, subscribedQuery: finalQuery };
  }, deps);

  const data = useTracker(() => {
    if (subscribedQuery) {
      return subscribedQuery[getStaticFunction(type)]();
    }

    return null;
  }, deps);

  return { loading: loading && !error, data, error };
};

I don't understand what's going on here well enough, but updating it to your last suggestion @CaptainN fixed the bug for us..

@bhunjadi I believe you are also using grapher subscriptions, maybe that's where it's coming from..?