[WIP] New hooks

[CaptainN]

A set of packages with new alternative, and much more streamlined hooks for integration between React and Meteor

Hooks	Package Name
useSubscription and useCursor	meteor/react-mongo
useMeteorState	meteor/react-meteor-state
useUser and useUserId	meteor/react-accounts

Basic implementations are in.

Todo:

• Flesh out README.md for each
• Add tests
• Add support for SSR - possibly, some generic SSR helpers
• Maybe add a Method call hook, with SSR helper integration

[CaptainN]

I did not mean to start a review.

[rijk]

@CaptainN really enjoying the react-mongo hooks, they feel like a way better match for React's hooks model.

I have run into a use case though that I'm not sure how to approach.

useSubscription( () => {
  if ( boardId ) return Meteor.subscribe( 'board.projects', boardId )
}, [ boardId ] )

This triggers a warning because currently the return value must be a SubscriptionHandle. Is it possible to make this optional in useSubscription? Or is there a better way to approach this?

[CaptainN]

Hmm, that's an interesting problem with useSubscription that I didn't think of. Basically, hooks can't be conditionally invoked, so there needs to be a way to support that within the hook itself.

One way to handle this as is, would be to use 2 components, and make your decision decide which to use. But that's an onerous requirement.

The easiest thing to do would be to support a falsy return value (such as "undefined" or "void" in your example), and simply do nothing in that case. I'll make that change.

[rijk]

An other way (which I'm using to work around it now) is to do this:

const noSubscription: Meteor.SubscriptionHandle = {
  stop() {},
  ready: () => true,
}

useSubscription( () => {
  if ( boardId ) {
    return Meteor.subscribe( 'board.projects', boardId )
  } else {
    return noSubscription
  }
}, [ boardId ] )

But this is a little too weird for a relatively common use case like this, and I had to go into the hook's source to find it. So support for a falsy return value would be a great usability improvement.

[rijk]

@CaptainN another thing I noticed, is when you do something like this:

useSubscription( () => Meteor.subscribe( 'board.projects', boardId ), [ boardId ] )

A new subscription is correctly initialized every time the boardId changes, but the old ones are not stopped:

Not sure if this is a bug or intended behaviour, but running the cleanup function (in this case, subscription.stop()) on every run of the hook would be what I expected, and would be more analogous to e.g. useEffect.

Edit: it seems the subscriptions aren't even stopped upon unmounting the component with the useSubscription hook. So I have the feeling this is definitely a bug.

[rijk]

Next interesting thing I noticed: the ready state for the new useSubscription hook doesn't seem very stable.

This is with useSubscription:

And the exact same code with useTracker instead:

Note that using useSubscription, it is marked as ready at some point, but then switches back to loading for some reason.

[rijk]

Do we need testing for these? These hooks will be pretty important, unwanted rerenders or invalid information could have big consequences in an application. I'm happy to write the tests if someone with more testing experience could help me set them up.

It might make development easier as well (removing the back and forth every time the code is changed).

[CaptainN]

We need more readmes, testing and probably some attention on the SSR side of things.

[CaptainN]

I've been swamped for a few days. Hoping to get to this by the weekend.

[rijk]

Tried out the latest version, and it seems to be working well. Already loving this API:

// useSubscription( () => Meteor.subscribe( 'boards' ), [] )
useSubscription( 'boards' ) // ✨

And coincidentally I also found this makes my Suspense version a perfect drop-in replacement! Not sure if this was intended, but a nice bonus.

import { useSubscription } from '/lib/react-mongo' // No suspense
// import useSubscription from 'meteor-subscribe-suspense' // Suspense

useSubscription( 'board.projects', boardId )

[CaptainN]

Big update:

• Removed useFindOne and useCount - we'll just encourage the use of useTracker.
• Moved new implementation of useTracker to react-meteor-data and replaced the old one.
  • Renamed old useTrackerNoDeps to useTrackerLegacy - open to suggestions. It could just be useTrackerNoDeps?
  • Made sure withTracker is using useTrackerLegacy.
  • Updated tests for the new split.
  • Removed some internal cruft and computationHandler
• For useFind added a warning if you return something other than a cursor in develop mode.

Notes:

• One test is sometimes failing with useTracker (but not with useTrackerLegacy). It's a subscription test ("resubscribe") that I ported, and didn't write, and I'm not really sure what it's doing enough to evaluate if it's the test or the implementation that's failing.
• The new useTracker requires deps - they are no longer optional. That and the change in performance characteristics means it should get a 3.0 update.

[CaptainN]

It looks like I'm running in to issues with the way subscriptions are recycled when using the cleaner useTracker implementation, based on those failing tests. I think it's because of what @sebakerckhof about how the Flush cycle works with Tracker/Subscriptions. It might indicate a serious problem with the current hook.

I think this means a couple of things

• There may not be a great way to integrate Tracker (specifically when using subscriptions) with React without doing it the way withTracker has always done - running everything synchronously, and rebuilding the computation on every render. This is the safest way to do it. useTracker without deps.
• There may be edge cases we are not currently detecting with the current "in the wild" version of useTracker... Specifically, in cases where the continuation feature fails to recover the initial tracker because the timeout period elapses. This may be causing subscription thrashing and reloading in some cases.
• Concurrent mode is likely to cause a lot of problems with the current approach because of the way Collection.subscribe handles all this.
• We could go all the way back to @yched's initial version of useTracker which monkey patched subscribed functions - however, I worry there are other implementations which tie in to the Flush cycle of Tracker, which will have similar issues (maybe unlikely? IDK).
• useMutableSource might eventually be the way forward.

Based on this, it might make the most sense to remove the deps version of useTracker altogether, and only support the deps-less version (the one that withTracker has always used, in this branch, currently called useTrackerLegacy).

That is, unless there is some way to properly manage this Flush lifecycle within React's lifecycle constraints.

[rijk]

At the risk of suggesting something wild: could it help to adjust Tracker? No need to treat this code as sacred IMO, it was written in a different time, before React even existed. It is of course important for it to stay frontend agnostic, but an optimization for one the most used frontends should not be a crazy idea, right?

[CaptainN]

@rijk I thought of modifying Tracker for other purposes - to create immutability for example. But it turned out to be significantly less effort to write the integration the way it is now, with much lower risk of effecting other projects which rely on Tracker.

For this particular problem, I think the algorithm codified in withTracker (and useTrackerNoDeps) is the one that makes the most sense, and we should just use that. It's not slow, even if it seems like it does a lot of work in every render (it probably is a bit slower in relative terms...).

However, I'd still like to have something a bit more optimized for these new hooks, even if that means we have the more special purpose Tracker implementation (which is not strictly compatible with Meteor.subscribe) only used internally. So we'd. go back to offering useCount and useFindOne based on that implementation as an internal detail.

For the main hook, we'd sunset the deps version of useTracker - this makes me sad. I'll create a new PR to explain the problem, update the tests, and maybe explore some alternative ways to fix it. I'd like to get a few more opinions about whether that's the right way to go, based on the problem I described in my previous comment.

[CaptainN]

I think I found a nice solution to the problems described above. Basically, I'm forcing a flush lifecycle for that initial computation in useMemo and that seems to work to clean up that particular test. I think it should be stable.

I did confirm that this problem can sometimes crop up in the current version of the hook. I think the way forward is to cherry pick the new useTracker from this PR, in a backward compatible way - still use the old combined useTracker which would switch modes if deps are used or not. There's a small problem with that specific aspect of the current hook, which makes me want to refactor it (as I did in this PR) - if the deps are changed in a lifecyle from extant to falsy, it'll cause confusing errors about hook reordering. Maybe that's not a big deal, IDK.

I created PR #306 for the useTracker overhall.

[leobastiani]

I just watched your talk in Impact Meteor 2020 and when you first introduced this guy I thought it would be global managed. I'm wondering that we are opening a computation every time we need userId in a component. I thought zustand would better fit this problem, although it might not be necessary because I don't know the impacts of openings many computations. What do you think? It could be applied in others states as well.

[CaptainN]

Computations are incredibly cheap in practice. In Blaze they are the backbone of the entire system. In Meteor, for years when using withTracker it would actively create and destroy a computation on every react render (it still does, as does useTracker when no deps are provided). It's basically adding one entry to an array internally, for the reactive source. Very cheap.

Still, to recycle some of the more expensive reactive sources (not the computation itself) like an expensive reactive subscription, you can always hoist the computation up your react tree (er, or down to root), and then use the context API to feed the result of that one computation to mulitple hooks. I showed how to do that in the hook introduction blog post (here is the gist). Be aware though, this can introduce some subtle timing issues with reactivity and the React lifecycle. It's easier (and cheap) to just use a lot of computations. ;-)

[CaptainN]

One point I mean to stress whenever we ship these, is they are not solving a problem with the current HOC/hook - useTracker is fast as is, and we'll still recommend its use with Collection.findOne for example. We/I simply couldn't come up with anything better for that.

These new hooks are the result of some exploratory effort, to see if we can gain some advantage by implementing things in a more custom way. What I found was that some of my earlier assumptions (which I did express in the Impact Meteor 2020 preso) did not pan out - just exposing a cursor was actually quite a bit slower and less efficient than useTracker...

The main advancements we did achieve here are in 2 very specific cases (so far anyway, there may be others when we/I get to them). useSubscription is more optimized for subscriptions in particular. In practice, it probably won't amount to much in terms of gains, but it has a cleaner API than useTracker for subscriptions in particular. The second one is the big win - useFind is now immutable, and doesn't require the subscription to be mixed in with the cursor. This has the potential to open up new use cases for high performance reactivity in lists over what useTracker and withTracker could deliver. I say "has the potential" because these statements have not been well tested, and there are likely tradeoffs here (as with any performance optimizations). But really, the main advantage of these 2 hooks is mostly that they (arguably) improve developer experience, through a more tailored/integrated feeling API between Meteor and React, and I always love that type of improvement.

Along the way, I also discovered some improvements we should make to useTracker which have been submitted as a separate PR.

[CaptainN]

Couple of things:

• It's been hard to make the unit tests work with useSubscription but also:
• We need subscription state to be available immediately, for cases of hydrated data. This is necessary to avoid a flash of unloaded when hydrating the React tree.

I'm not sure if that second bullet point works with useTracker - but I think it does when used with something like staringatlights:fast-render. For useSubscription we'd need to rethink what we return before the component mounts, in SSR/hydration scenarios. That'll make writing the unit tests even harder, to be honest.

Given this complexity, I'm leaning toward ditching useSubscription and simply suggesting that users use useTracker. useTracker when just used for a Meteor.subscribe call is actually quite light.

[gunn]

@CaptainN has anything been done with the idea of a third argument to useTracker that compares the output to the previous run, and decides whether to update?

I needed this for myself (some re-renders are very expensive), so I modified useTracker to add it. You can see the changes here: https://gist.github.com/gunn/90bf091fa6c714683abb6ad73448a53f/revisions?diff=unified

You can use it like so:

import * as fastDeepEqual from 'fast-deep-equal'
const versions = useTracker(()=> Versions.find().fetch(), [], fastDeepEqual)

Using fastDeepEqual is just for example of course. In my case, I have a collection transform that makes the returned records immutable so I can do a more shallow comparison.

I've been using it in production for a few months now, and it's working great for me.

[CaptainN]

I have a branch with that on it. Actually, let me open a WIP PR (#321).

I'd love to know more about how you made your records immutable (collection transform?). The useFind hook in this PR provides partial immutability (it makes sure to invalidate the array and the document reference of the changed document, without altering the references of untouched records in a collection, but doesn't provide property level immutability - it's all or none at the document level).

[gunn]

@CaptainN nice.

Actually I've only gone for document level immutability too because that's what looked to make sense performance-wise.
I have a sort of custom ORM built around collections, and this is a natural part of that.

Here's a (very) cut down piece of the approach:

const Records = new Mongo.Collection("records", {
  transform: RecordClass.load
})

class RecordClass {
  static load(data) {
    const unchanged = _dataCache[data._id] &&
      fastDeepEqual(data, _dataCache[data._id])
    
    if (unchanged) return _recordCache[data._id]
    return _recordCache[data._id] = new RecordClass(data)
  }
  /* ... */
}

A better approach might be to do the transform on every Records.find().observe({ added, changed}) etc., and then have the "transform" be a simple lookup: transform: (doc)=> _recordCache[doc._id]

Even better, have the cursor observer just invalidate the record cache, which the first transform for that would then repopulate, something like:

Records.find().observe({
  changed: doc=> delete _recordCache[doc._id],
  removed: doc=> delete _recordCache[doc._id]
})

const Records = new Mongo.Collection("records", {
  transform: doc=> _recordCache[doc._id] ||= new RecordClass(doc)
})

The upside is that every findOne() and find().fetch() is now returning records that can be compared with ==. Here's the comparison function I use with useTracker: https://gist.github.com/gunn/9e52d3914f6cfd561401ba8f3681582a

If I did want field-level immutability, I would either use the observer hook: changed(newDocument, oldDocument), and iterate over the fields looking for differences, or use immer.

[CaptainN]

What do you guys think about this plan:

• Nix the useSubscription hook. It's too hard to write tests for it, and I'll probably never get around to it (there may also be issues with it...).
• Add useFind to react-meteor-data, instead of it's own package.

Both of these on a separate PR/branch, so make merge simpler.

In use, it'd look something like:

import { Meteor } from 'meteor/meteor'
import React, { memo } from 'react'

import { Messages } from '/imports/api/collections/messages'
import { useTracker, useFind } from 'meteor/react-meteor-data'

const ChatScreen = () => {
  const roomId = 'Y5hoT957PKphnuqCA'
  const isLoading = useTracker(
    () => !Meteor.subscribe('messages', roomId), [roomId]
  )
  if (isLoading) return null
  else return <MessagesList roomId={roomId} />
}

const MessagesList = ({ roomId }) => {
  const testMessages = useFind(
    () => Messages.find({ roomId }, { sort: { createdAt: 1 } }),
    [roomId]
  )
  
  return (<>
    <article className="messages">
      {testMessages.map((message) => (
        <Message
          key={message._id}
          message={message}
        />
      ))}
    </article>
  </>)
}

const Message = memo(({ message }) => {
  return <div>{message.message}</div>
})

export default ChatScreen

This structures things so that it's optimized for memoization, and for concurrent rendering. One of the things to watch out for, is a lot of examples for setting up subscriptions do the subscription and the list rendering in the same component, but this actually blunts concurrent modes ability to do its work (a little), by forcing that first concurrent render to render something like null or a loading component, then switching it to render a list later on. By separating where the loading happens from the list rendering, we get optimal concurrent rendering. Another benefit is that if we already have content in the collection (if we are using ground:db for example) we can go ahead and kick off that list rendering immediately.

Anyway, I'd like to go ahead and get the useFind hook integrated, documented, and released, without letting the useSubscription hook gum up the works. What do you think?

[CaptainN]

@gunn I think the way useFind does it's observing magic is pretty close to what you are expressing above with the observer, except it does it on a query by query basis, instead of configuring the global store, and doesn't rely on a deep-equal comparator.

As far as the comparator, in general, I don't like a deep equal check as a standard avenue (even fast-deep-equal) because it's hard to predict the performance of it. It can work, but it needs to be used with care and consideration, because with certain types of data that comparison can be more expensive than rerenders. That said, I added a comparator option to useTracker in PR #321. We could probably add one to useFind too, for cases where you are doing partial queries, and don't want changes to unused properties to update your view, a comparator can be useful.

[rijk]

Couldn't we at least add a simple useSubscription hook with the implementation in your example? That would give users the the nice API, with the benefit that we could optimize its implementation later on.

Just something like this:

function useSubscription( name, ...args ) {
  useTracker( 
    () => !Meteor.subscribe( name, ...args ).ready(), 
    args 
  )
}

[edemaine]

@rijk That sounds/looks good in principle, but how do you know whether the final argument should be passed to useTracker or Meteor.subscribe? (In your code, you pass it to both, but that's probably not what you want.)

I found it a little weird that useFind needed to be passed a closure with a find call, instead of a collection and the find arguments, but the latter approach has a similar issue. Except Collection.find never takes an array argument, so we could actually tell what we should do with the last argument...

Related, is there a reason (e.g. efficiency) that useFind needs to be a different hook, instead of useTracker behaving specifically when the return value is a cursor or array of cursors? The latter is what I expected, coming from Blaze.

[rijk]

Yeah it's the reflex of adding deps like you have to do in useEffect, but it's not actually necessary in useTracker, and @CaptainN mentioned somewhere it might even be better to leave them off entirely (they're two completely different implementations, but to be honest it's still not clear to me when I should and should not use deps).

In that case it could be even simpler, something like this:

function useSubscription( name, ...args ) {
  useTracker( () => !Meteor.subscribe( name, ...args ).ready() )
}

The ...args will just round up all arguments as an array, and pass them to Meteor.subscribe as separate arguments again. So usage could be something like:

const loading = !useSubscription( 'messages', roomId )

Or if you don't care about loading state:

useSubscription( 'messages', roomId )

I think this goes a long way not only in developer experience and less keystrokes, but also how tight the Meteor/React integration feels.

const isLoading = useTracker(
  () => !Meteor.subscribe('messages', roomId), [roomId]
)

.. is something you'd probably have to look up,

const isLoading = !useSubscription( 'messages', roomId )

you can probably remember.

Edit: side note: It would also be nice if name is of type string | false, so it's easier to have conditional logic (as hooks can not be conditional).

[CaptainN]

useFind is all about efficiency with lists - especially big lists. When you use myCollection.find().fetch() you are doing a lot of object copying and iteration. For every reactive change, you are basically running your find query, then calling fetch on that and getting a full list of new object references, etc. (When using deps array, that happens outside of the react render frame after mount, which is the main benefit of using deps - without deps it happens inline with render, which is fine for small reactive operations like subscriptions or Meteor.user() type checks - maybe even preferred for a few reasons).

With useFind the query is run, and observers are set up to handle individual document updates, which updates a partially immutable store. On updates, it only invalidates the changed document - every other document remains untouched - no extra iteration, doesn't add dozens of new references, and the document level iteration makes memoization in the react tree super simple, to speed up reconciliation.

A simple wrapper for useSubscription sounds great.

const useSubscription = (name: string | null, ...args: any[]) => (
  useTracker( () => Meteor.subscribe(name, ...args).ready() )
);

[StorytellerCZ]

We can probably start at v1.0.0

[CaptainN]

Yeah, I'll make that change in a new PR

[StorytellerCZ]

Let's start at least at 1.12

[CaptainN]

The reason for this is simply that I can't get the tests to run on anything newer than Meteor 1.10...

[StorytellerCZ]

typo in hot

[CaptainN]

That whole readme needs to be written. :)

[StorytellerCZ]

@CaptainN You should probably write up a blog post to go together with this once it gets released.

[CaptainN]

Hey all - I've broken out the remaining work from this PR to #332 (useFind and a new version of useSubscribe added to react-meteor-data) and #333 (new package: react-meteor-state with useMeteorState hook).

I'll do the same for react-meteor-accounts and that will obsolete this PR.

Please review and comment on those new PRs! The useSubscribe hook in particular is different from the one in this PR, but is also cleaner, and should be very easy to test. It's built on useTracker and uses the new comparator option to address the concerns we discussed in this thread.

[CaptainN]

I added #335 for react-meteor-accounts. With that, this PR is no longer necessary. I'm going to close this one, and we can move any discussion to those other ones.