meteor / react-packages

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

Feat: useTracker with suspense #387

Closed Grubba27 closed 1 year ago

Grubba27 commented 1 year ago

Pointed to useFind with suspense because the ESLint was already configured there.

Still missing a few pieces (it works, but it has some things that need to be solved)

I will list the issue that I think needs to be solved before this goes out:

For testing this change ## client ```ts import { EJSON } from 'meteor/ejson'; import { Meteor } from 'meteor/meteor'; import React, { Fragment, Suspense, useCallback, useEffect, useMemo, useState } from 'react'; import { LinksCollection } from '../api/links'; import isEqual from 'lodash/isEqual'; const cachedSubscriptions: SubscriptionEntry[] = []; type SubscriptionEntry = { params: EJSON[]; name: string; handle?: Meteor.SubscriptionHandle; promise: Promise; result?: Meteor.SubscriptionHandle; error?: unknown; }; const removeFromArray = (list: T[], obj: T): void => { if (obj) { const index = list.indexOf(obj); if (index !== -1) list.splice(index, 1); } } function useSubscribeSuspense(name: string, ...params: EJSON[]) { const cachedSubscription = cachedSubscriptions.find(x => x.name === name && isEqual(x.params, params)); useEffect(() => { return () => { setTimeout(() => { if (cachedSubscription) { cachedSubscription?.handle?.stop(); removeFromArray(cachedSubscriptions, cachedSubscription); } }, 0) } }, [name, ...params]) if (cachedSubscription) { if ('error' in cachedSubscription) { throw cachedSubscription.error; } if ('result' in cachedSubscription) { return cachedSubscription.result as Meteor.SubscriptionHandle; } throw cachedSubscription.promise; } const subscription: SubscriptionEntry = { name, params, promise: new Promise((resolve, reject) => { const h = Meteor.subscribe(name, ...params, { onReady() { subscription.result = h; subscription.handle = h resolve(); }, onStop(error: unknown) { subscription.error = error; subscription.handle = h reject(); }, }); }), }; cachedSubscriptions.push(subscription); throw subscription.promise; } function UseTrackerClientSuspense({ active }: { active: boolean }) { useSubscribeSuspense('example', 10000); const docs = useTrackerSuspense('name',() => active ? LinksCollection.find().fetchAsync() : null ,[active]); const docs2 = useTracker(() => active ? LinksCollection.find().fetch() : null, [active]); // here should be always equal console.log({ docs, docs2 }); return
{ JSON.stringify(docs, null, 2) }
; } export function App() { const [active, setActive] = useState(false); const onActive = useCallback(() => setActive(active => !active), []); return ( Loader...
}> { } ); } ``` ## Server ```js import { Meteor } from 'meteor/meteor'; import { check } from 'meteor/check'; import { LinksCollection } from '/imports/api/links'; async function insertLink({ title, url }) { await LinksCollection.insertAsync({ title, url, createdAt: new Date() }); } Meteor.publish('example', (limit: unknown) => { check(limit, Number); return LinksCollection.find({}, { limit }); }); Meteor.methods({ insertLink }) Meteor.startup(async () => { if (await LinksCollection.find().countAsync() === 70) LinksCollection.remove({}) // If the Links collection is empty, add some data. if (await LinksCollection.find().countAsync() === 0) { await insertLink({ title: 'Do the Tutorial', url: 'https://www.meteor.com/tutorials/react/creating-an-app', }); await insertLink({ title: 'Follow the Guide', url: 'https://guide.meteor.com', }); await insertLink({ title: 'Read the Docs', url: 'https://docs.meteor.com', }); await insertLink({ title: 'Discussions', url: 'https://forums.meteor.com', }); } }); ```
Grubba27 commented 1 year ago

I've updated the skipUpdate func as well. I've opted to maintain its signature as <T>(prev: T, next: T) => boolean and added await to the parameters like this since we are in an async Tracker.autorun things should be fine:


!skipUpdate(await refs.trackerData, await data)
Grubba27 commented 1 year ago

FYI, a demo running with the code in this PR is the one below:

https://user-images.githubusercontent.com/70247653/225008710-c8179caf-f1a7-40a7-940b-a32c05912488.mov

CaptainN commented 1 year ago

What is the cache system for? Meteor.find (and it's collection/pub/sub system in general) already has caching (it basically IS a client side cache). Adding a cache to useFind seems like duplicated effort.

radekmie commented 1 year ago

The point is, we need a stable reference to a Promise we throw (Suspense for data fetching works like this). And as the fetchAsync creates a new one every single time, we need an additional "cache" here, to map the hook params into the stable reference.

CaptainN commented 1 year ago

Ah, that's one of the constraints that prevented me from getting something like this working from inside the useFind/useTracker hook (There's a similar issue with the unfinished useMeteorState hook).

But I think that's not the right strategy anyway. I think the better strategy is to hold the reference where it is caught, in a provider. This is similar to how SSR (or static gen) has always worked, except now you'd be using suspense. In this case, your Provider would also be the ErrorBoundary that catches the thrown promise. (If I remember how it all goes together - been a while.)

The challenge comes from nested, derivative calls. This was a challenge to get right in Loadable too.

/2cents

radekmie commented 1 year ago

The "provider approach" is not needed here, as the Suspense mechanism already covers that - we can keep the pending states local (i.e., to the first <Suspense> up in the tree). We could have a provider for the promise map, but that's in implementation detail - it'd still be "global", i.e., accessible everywhere in the tree.

Also, error boundary won't catch a thrown Promise - that's a special case for the "Suspense for data fetching" to make it work. (It's still experimental though.)