Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useTracker problem - Can't set timers inside simulations #396

Open
tennox opened this issue Jun 2, 2023 · 8 comments
Open

useTracker problem - Can't set timers inside simulations #396

tennox opened this issue Jun 2, 2023 · 8 comments
Labels

Comments

@tennox
Copy link

tennox commented Jun 2, 2023

I'm using useTracker like this:

<div>{!contact ? null : (
  <AsyncValue
    subscription={{
      name: 'mail.contactStats',
      args: [contact.ID],
      query: () => MailStats.findOne({ _id: `for-contact-${contact.ID}` }),
    }}
  />
)}</div
export function AsyncValue({ promise, subscription, render, children, onError }) {
  const [error, setError] = useState(null);
  const { name, args, query } = subscription;
  const value = useTracker(
    () => {
      LOG('METEOR TRACKER?', subscription);
      if (error) return undefined; // otherwise we get an infinite loop trying to resubscribe
      
      // I COMMENTED MY WHOLE CODE TO NARROW DOWN THE PROBLEM, IT STILL APPEARS
      // const sub = Meteor.subscribe(name, ...args, {
      //   onStop: error => {
      //     if (onError) {
      //       DEBUG(`AsyncValue subscription error, sending to onError:`, error);
      //       onError(error, setError);
      //     } else {
      //       if (error) console.error('AsyncValue Subscription error:', error);
      //       setError(error);
      //     }
      //   },
      // });
      // DEBUG(`sub: ready=${sub.ready()}, entries:`, query);
      // return sub.ready() ? query() : undefined;
      return undefined;
    }
    []
  );
  DEBUG(`sub value:`, value);

  const renderValue = () => (render ? render(value) : value);
  return <>{value !== undefined ? renderValue() : placeholder()}</>;
}

Works fine if contact is set from the start - but whenever contact changes from being null to non-null, this error occurs:

Uncaught Error: Can't set timers inside simulations
    at withoutInvocation (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:621:13)
    at bindAndCatch (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:633:33)
    at Meteor.defer (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:694:24)
    at useTracker.ts:163:12
    at mountMemo (modules.js?hash=bfdeb58d2ee948751f18b6466ad43663c2847a1e:133794:19)
    at Object.useMemo (modules.js?hash=bfdeb58d2ee948751f18b6466ad43663c2847a1e:134090:16)
    at useMemo (modules.js?hash=bfdeb58d2ee948751f18b6466ad43663c2847a1e:113407:21)
    at useTrackerWithDeps (useTracker.ts:137:3)
    at useTrackerClient (useTracker.ts:206:12)
    at useTrackerDev (useTracker.ts:244:16)

Also tried useTracker with subscription as dep (same error as above) and without deps - which causes this error:

Uncaught Error: Can't set timers inside simulations
    at withoutInvocation (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:621:13)
    at bindAndCatch (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:633:33)
    at Meteor.defer (meteor.js?hash=3fe70c93a2fcd225dc5cc6572b8c3e39756a507b:694:24)
    at useTrackerNoDeps (useTracker.ts:81:12)
    at useTrackerClient (useTracker.ts:204:12)
    at useTrackerDev (useTracker.ts:244:16)
@Grubba27 Grubba27 added the bug label Jun 2, 2023
@StorytellerCZ
Copy link
Collaborator

StorytellerCZ commented Jul 28, 2023

Ran into the same problem as well. Strangely it only happens to me on Firefox and works fine in Brave.

Meteor 2.13

The code:

import { Meteor } from 'meteor/meteor'
import { useEffect } from 'react'
import { useSubscribe } from 'meteor/react-meteor-data'
import { ProfilesCollection } from 'meteor/socialize:user-profile'
import type { Profile } from '../../users/definitions'
import { Tracker } from 'meteor/tracker'
import { ReactiveVar } from 'meteor/reactive-var'

const profileData = new ReactiveVar<Profile | null>(null)

export function useProfile(subscription?: string): {
  isLoading: boolean
  profile: Profile | null
} {
  const isLoading = useSubscribe(subscription || 'profile')
  useEffect(() => {
    const computation = Tracker.autorun(() => {
      if (!isLoading()) {
        const profile = ProfilesCollection.findOne(Meteor.userId())
        profileData.set(profile) // same issue if I use React.useState
      }
    })
    return () => {
      computation.stop()
    }
  }, [])
  return { isLoading: isLoading(), profile: profileData.get() }
}

The logs seems to point to useSubscribe. It is also of note that this happens deep in the application in few spots, but elsewhere it works fine.
It also triggers another error with it: Error: Objects are not valid as a React child (found: [object Error]). If you meant to render a collection of children, use an array instead. which cascades over multiple error boundaries for some reason (or at least that is how it looks to me now).

@CaptainN @Grubba27 any ideas?

@StorytellerCZ
Copy link
Collaborator

Found the solution for the additional error triggered, unrelated to this. one. Also I have switched back to using instead of ReactiveVar, but that didn't seem to do much except become reactive again on parts of the data.

The suspense version:

import { Meteor } from 'meteor/meteor'
import { useEffect, useState } from 'react'
import { useSubscribe } from 'meteor/react-meteor-data/suspense'
import { ProfilesCollection } from 'meteor/socialize:user-profile'
import type { Profile } from '../../users/definitions'
import { Tracker } from 'meteor/tracker'

export function useProfile(subscription?: string): {
  profile: Profile | null
} {
  useSubscribe(subscription || 'profile')
  const [profile, setProfile] = useState()
  useEffect(() => {
    const computation = Tracker.autorun(async () => {
      const profileData = await ProfilesCollection.findOneAsync(Meteor.userId())
      setProfile(profileData)
    })
    return () => {
      computation.stop()
    }
  }, [])
  return { profile: profile || null }
}

Seems to resolve it in many places for me.

@CaptainN
Copy link
Collaborator

I'm not sure what a "simulation" is in this context, but it's probably this line in useTracker. Basically, we are using a trick to avoid some side effects in some edge cases. That deserves a closer look in React 18+. Most of those edge cases were problems with earlier versions of React's concurrency model. It's probably all different now.

@CaptainN
Copy link
Collaborator

@StorytellerCZ That example of using a computation in useEffect is basically a very simple implementation of a useTracker hook. It doesn't try to do any first-render magic that the main useTracker does, which has a lot of upside (simplicity especially) and only a few downsides, especially if your components are properly composed, and you don't do some giant single component iterator.

Basically, whenever you build your component output off something derived from state in useEffect, you essentially opt out of concurrent rendering (at least that's how it used to work). That's not always terrible - in the case of loading data, it's often preferable, as you avoid a lot of edge case problems, and you can opt back in to concurrent rendering by just defining the components that actually do the rendering separately (instead of using a branching statement and a lot of JSX in a single loading component). (So treat the component that does the loading as a container.)

What I've been finding working with react server components, suspense and some newer tech, is that the old HOC/container model is becoming the norm again, but it looks a lot different than the way it used to. It's built off hooks, not HOCs for the data loaders - HOC/container is defined in the project. It's pretty interesting to see this all come full circle. Nextjs's app router is looking a lot like Meteor these days.

@CaptainN
Copy link
Collaborator

Also, this error: Error: Objects are not valid as a React child (found: [object Error]). If you meant to render a collection of children, use an array instead. usually indicates some non-JSX return values from a React component (in this case, your component is returning an actual Error object). I can't see that part of the code from the posted snippet, but I'd look there, rather than in the hook for this particular problem.

@StorytellerCZ
Copy link
Collaborator

The error message is gone after some work on our Error Boundary.

We will also slowly start the process of separating data loading and the rest.

As mentioned above using the suspense version of useProfile seemed to work, I'm noticing that the regular version often gets stuck and doesn't load any data at all.

Maybe it is time to make a new breaking version release that removes the tricks for older Reacts and is only focused on React 18 (especially given that it has been around long enough).

@CaptainN
Copy link
Collaborator

CaptainN commented Aug 7, 2023

@StorytellerCZ That was part of the idea behind useFind and useSubscribe, though maybe we didn't go far enough. MOST of the challenge with implementing a general purpose useTracker comes from supporting the various Blaze compatible lifecycle hacks inside of the subscribe method. Everything else is kind of easy to support.

It's worth reiterating, the side-effect we create when supporting a Tracker instance is NOT ONLY data loading, it's creating and maintaining computations, which can draw data from many locations - usually synchronously. Data loading is handled by subscriptions, which happens behind a computation. That's what makes this all so challenging. Almost all of the newest tools introduced by React 18 and server components is about data loading, and direct subscriptions. They haven't really added much to help us support computations. (Apollo Client and URQL have similar challenges when using their advanced caching, but they also have constrained problem domains, since they are only dealing with data loading, and not general purpose computations.)

@CaptainN
Copy link
Collaborator

CaptainN commented Aug 7, 2023

If I were blue skying this, I'd probably look at coming up with fresh implementation of Meteor.subscribe and look at that entire data loading pipeline, and not try to keep computations as they currently exist around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants