useFind and useSubscribe

[CaptainN]

This PR adds useFind and useSubscribe as discussed and prototyped in the new-hooks branch (PR #298).

The hooks are basically done. They just need docs and tests.

TODO:

• [x] Write tests
• [x] Add documentation (readme)

[CaptainN]

I added some initial tests for useFind and update the README!

The test covers the basic useFind usage. It isn't super exhaustive, but it probably covers most of what people will be doing with it.

I didn't add any tests to cover useSubscribe but that's such a simple wrapper over useTracker anyway. There is a small rerender optimization having to do with the .ready()/isLoading() method it returns that should probably be tested.

The only thing I think maybe I missed is that the useFind hook requires a cursor to be returned - I think there was a case where maybe we wanted the cursor to optional, in case we wanted to check for loading state within the cursor factory - but I don't know how necessary that is. It might even be beneficial to skip that check altogether, and just declaratively set up the query, and let React do the "isLoading" work. I'm not sure we need to add that functionality.

Anyway, this is probably ready for either an alpha or beta release, IMHO.

[yched]

the "optional cursor" bit was because you cannot conditionally call a hook (while you can conditionally do a cursor.find() in a useTracker() callback). It's not necessarily tied to the isLoading state of a previous subscription, but could be just any condition (does some prop have a value ?)

I'd think we do need this ability to conditionally return a cursor

[CaptainN]

I'll add support for null return values - but it might be an anti-pattern to do conditional queries. I'll expand on that another time.

[filipenevola]

Hey @CaptainN maybe this is out of the scope of this PR but I believe it's worth to mention.

I'm working in a branch providing a way to use mongo Meteor package without depending on APIs that are relying on Fibers.

What does that mean?

For example, if you need to call LinksCollection.find() and we know that in the future this find is going to depend on async code we are already creating a LinksAsyncCollection.findAsync() so you need to use await already and you don't need to change your code later when we remove Fibers from the implementation. You can read more about this work in this discussion here.

This is probably going to be optional in the client as we don't have Fibers in the client as well but if you want to keep your code isomorphic you should use the new async methods in the client as well.

And why I'm bringing this here?

Including an async call inside useTracker is going to add some challenges and maybe useFind could already solve this supporting async functions also as callbacks or even only supporting async functions as callbacks.

I didn't review the code of this PR yet to say how hard that would be or if this is possible keeping the same ideas but I decided to share this here as I'm working in the mongo package and I can see this impact coming.

[CaptainN]

I agree it'll be challenging to support async/await in useTracker.

Here is should be relatively easy to set it up as we need it. In useFind we really only use the normal fetch method once, to grab initial data. It creates a cursor and does the initial (non-reactive) data fetch in useMemo, and after that, it just uses the observe API on the cursor for updates.

[CaptainN]

I implemented and tested null return value for useFind - this should be good to go.

[StorytellerCZ]

@CaptainN ready to move this to the next stage?

[CaptainN]

Yep, ready for testing! Maybe a beta release?

[StorytellerCZ]

Let's resolve the conflicts and then we can do a beta release.

[filipenevola]

Hi @CaptainN sorry for the late feedback but shouldn't we have a way to skip a subscribe and even a find?

At least I have one useFind that I implemented myself and in some cases I need to skip the subscription because I don't have all the params and as we can't call hooks conditionally it's necessary to provide a way to skip inside the hook.

My example:

const { data: modules } = useFind({
    publicationName: 'modules',
    arg: { modeName },
    skip: !modeName,
    find: () => ModulesCollection.find({}, { sort: { order: 1 } }),
  });

My implementation (it's not a package, just the code in one of my apps):

export const useFind = ({
  publicationName,
  arg,
  find,
  skip,
  shouldSkip,
  dataReturnWhenLoading,
}) =>
  useTracker(() => {
    if (skip || (shouldSkip && shouldSkip())) {
      return {
        data: [],
        skipped: true,
      };
    }
    const handler = Meteor.subscribe(publicationName, arg);

    if (!handler.ready()) {
      return {
        loading: true,
        data: dataReturnWhenLoading || undefined,
      };
    }

    const result = find();

    // is findOne? so no fetch
    return { data: result.fetch ? result.fetch() : result };
  });

How could I do the same using the hooks from this package? How could I skip the subscription call when modeName is not available?

[CaptainN]

You can't conditionally call a subscription - but useSubscribe is mostly a simple wrapper around useTracker, and there's no reason you can't just use useTracker for advanced cases like this.

[filipenevola]

You can't conditionally call a subscription - but useSubscribe is mostly a simple wrapper around useTracker, and there's no reason you can't just use useTracker for advanced cases like this.

My point is not to block this PR, it is just to understand how common this could be.

I would say this is pretty common. But if you disagree, ok :)

[CaptainN]

I think it might be less common than it seems if you are organizing your code with smaller purpose built components, the way the two hooks - useSubscribe and useFind - would imply. A recommendation might be to not even include your useSubscribe and useFind hooks in the same component. These are of course patterns that we need to set and communicate.

But like I said, it's easy enough to fall back to useTracker in this case anyway.

We could refactor to allow the use of a factory, instead of or in addition to the non-factory hook signature we use now, if it's too confusing the way it is.

[filipenevola]

No, it is not confusing.

I was asking because it was a need that I had just a few days ago and I would like to confirm how common do we think this is.

thank you!

btw, @StorytellerCZ is probably going to publish a new version of this package soon including new hooks.

[CaptainN]

I really don't know how common it would be, but I do wonder whether it's part of Meteor's philosophy to reduce friction when coding, and if so, maybe we should add the factory option to useSubscribe? I don't think it'd be that difficult.

[CaptainN]

So actually, useSubscribe already supports conditional subscriptions - if the subscription name argument is falsy it will simply fail silently (and isReady() will return false). I can add documentation for this.

[StorytellerCZ]

I have bumped the versions for next beta release @filipenevola

[filipenevola]

Published react-meteor-data@2.4.0-beta.2.

[filipenevola]

I've refactored my internal useFind:

import { Meteor } from 'meteor/meteor';
import { getLanguage } from './languages';
import { useTracker } from 'meteor/react-meteor-data';

const subscribeCall = (publicationName, arg) => {
  const argWithLanguage = { ...(arg || {}), language: getLanguage() };
  return Meteor.subscribe(publicationName, argWithLanguage);
};

export const useFind = ({
  publicationName,
  arg,
  find,
  skip,
  shouldSkip,
  dataReturnWhenLoading,
}) =>
  useTracker(() => {
    if (skip || (shouldSkip && shouldSkip())) {
      return {
        data: [],
        skipped: true,
      };
    }

    if (publicationName) {
      const handler = subscribeCall(publicationName, arg);

      if (!handler.ready()) {
        return {
          loading: true,
          data: dataReturnWhenLoading || undefined,
        };
      }
    }

    const result = find();

    // is findOne? so no fetch
    return { data: result.fetch ? result.fetch() : result };
  });

to use the new hooks and they are working perfectly, now I'm calling my hook useData:

import { getLanguage } from './languages';
import { useFind, useSubscribe } from 'meteor/react-meteor-data';

export const useData = ({
  publicationName,
  arg,
  find,
  skip,
  shouldSkip,
  dataReturnWhenLoading,
  deps = [],
}) => {
  const argWithLanguage = { ...(arg || {}), language: getLanguage() };
  const isSkip = skip || (shouldSkip && shouldSkip());
  const isLoading = useSubscribe(
    isSkip ? null : publicationName,
    argWithLanguage
  );
  const result = useFind(find, deps);

  if (isLoading()) {
    return {
      loading: true,
      data: dataReturnWhenLoading || undefined,
    };
  }

  // is findOne? so no fetch
  return { data: result.fetch ? result.fetch() : result };
};

You can see it in action here.

[CaptainN]

That's a great example of functional composition. Love it.

I'm not sure findOne will work though - useFind doesn't support findOne. You never have to call fetch on the result of useFind. It always returns an array of items, even if there is only one. You'd probably want to detect whether it's findOne or find, then use useFind to get an array, and return only the first item if it's findOne, or the entire array if it's find. (Internally, findOne basically works this same way - it just wraps find and grabs the first element.)

[filipenevola]

I'm not sure findOne will work though - useFind doesn't support findOne. You never have to call fetch on the result of useFind. It always returns an array of items, even if there is only one. You'd probably want to detect whether it's findOne or find, then use useFind to get an array, and return only the first item if it's findOne, or the entire array if it's find. (Internally, findOne basically works this same way - it just wraps find and grabs the first element.)

It's working but I believe I just have one use case for findOne inside a React component, I will double-check it later.

[filipenevola]

v2.4.0 is published now.

@CaptainN do you want to start the blog post for this version?

[filipenevola]

BTW, as always, thank you for the great work.