Skip to content

Proposal for react-meteor-hooks #263

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

Closed
wants to merge 5 commits into from
Closed

Conversation

leoc
Copy link

@leoc leoc commented Nov 7, 2018

I played around a little with react-hooks.

This is what I came up with. I would love to discuss this solution.

I created a little sample project here https://github.com/leoc/react-meteor-hooks-example

Unfortunately the test script does not exist. But I would love to write tests for these hooks.

Hooks

useMeteorSubscription

It takes all arguments you would put into Meteor.subscribe and runs
the subscription and ready() checks in a useEffect and returns the
value of a state hook for ready().

useMeteorData

This function takes a function as first parameter and runs that as an
effect hook within the meteor tracker. The second parameter is an
array of inputs that influence the effect hook.

It returns the value of the state hook that represents the return
value of the given function. This can either be an object to be
destructured later or a single value (array of collection documents or
single collection document).

Usage

This package exports the hooks useMeteorSubscription and useMeteorData.

export const Page = function (props) {
  const loading = useMeteorSubscription('links');
  const links = useMeteorData(() => Links.find().fetch());

  if(loading) return (<div>Loading links ...</div>);

  return (
    <ul>
      {links.map((link) => (
        <li key={link._id}>
          <a href={link.url} target="_blank">{link.title}</a>
        </li>
      ))}
    </ul>
  );
}

@apollo-cla
Copy link

@leoc: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@leoc leoc changed the title Add first proposal for react-meteor-hooks Add proposal for react-meteor-hooks Nov 7, 2018
@leoc leoc changed the title Add proposal for react-meteor-hooks Proposal for react-meteor-hooks Nov 7, 2018
@leoc leoc force-pushed the react-meteor-hooks branch from 5a7620f to 98f1e6a Compare November 7, 2018 15:44
@StorytellerCZ
Copy link
Collaborator

Looks pretty good. Couple of things though:

Maybe use array to define multiple subscriptions for useMeteorSubscription and return true only after all subscriptions are ready.

Add a general withTracker hook so that people can easily migrate from existing usage or could useMeteorData be considered an equivalent?

@fawzisf
Copy link

fawzisf commented Feb 12, 2019

This looks really promising, i already opened an issue to propose using hooks before noticing your PR.
would love to see this implemented as it would make React and Meteor much more enjoyable together.

Copy link

@OlivierJM OlivierJM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update react version to latest stable

@@ -2,7 +2,7 @@ import { checkNpmVersions } from 'meteor/tmeasday:check-npm-versions';

checkNpmVersions({
react: '16.7.0-alpha.0',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to set the react version to current stable in this case 16.8.3

@CaptainN
Copy link
Contributor

CaptainN commented Mar 5, 2019

I would think the most direct analog for withTracker would be useTracker in which we could place anything "Meteor reactive" including subscriptions and data queries. That's how I use withTracker now, and I think it's consistent with most React in Meteor tutorials.

@boomfly
Copy link

boomfly commented Jul 8, 2019

this implementation not working as expected.
check this #262

@reinisriekstins
Copy link

reinisriekstins commented Sep 19, 2019

This seems like a good idea for the most part. One potential problem I see is that there is no way (at least I can't think of any) to access the subscription handle directly with this API, which would make it impossible to use features of libraries such as such as peerlibrary:subscription-scope, as it is exposed through handle.scopeQuery() (see: https://github.com/peerlibrary/meteor-subscription-scope#examples).

EDIT:
A solution to this would be to expose the entire sub handle instead of just the loading property

export const Page = function (props) {
  const handle = useMeteorSubscription('links');
  const loading = !handle.ready();
  const links = useMeteorData(() => Links.find(handle.scopeQuery()).fetch(), [handle]);

  if(loading) return (<div>Loading links ...</div>);

  return (
    <ul>
      {links.map((link) => (
        <li key={link._id}>
          <a href={link.url} target="_blank">{link.title}</a>
        </li>
      ))}
    </ul>
  );
}

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Arthur Leonard Andersen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Arthur Leonard Andersen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@filipenevola
Copy link
Contributor

closing because of [email protected] #273

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

Successfully merging this pull request may close these issues.

10 participants