cult-of-coders / grapher-react

Provides easy to use React Components that are suitable for grapher package.
https://atmospherejs.com/cultofcoders/grapher-react
38 stars 19 forks source link

Expected Component in field loadingComponent #25

Closed davidsavoie1 closed 6 years ago

davidsavoie1 commented 6 years ago

Hi!

I just started using Grapher and its React HOC and I have this error coming up when I try to set a component for the loading state: "Match error: Expected Component in field loadingComponent".

Here's the code, which is heavily inspired by the documentation:

import React from 'react';
import { withQuery } from 'meteor/cultofcoders:grapher-react';
import allRoutes from '/common/queries/allRoutes';
import Calendar from '../dumb/Calendar';

function LoadingComponent() {
  return <div>Please wait...</div>;
}

const CalendarWrapper = withQuery(
  () => allRoutes.clone(),
  {
    loadingComponent: LoadingComponent,
  },
)(Calendar);

export default CalendarWrapper;

Am I missing something obvious?

And by the way, I just started, but I'm AMAZED by what this library seems to offer out of the box. I've worked on a big project recently without Grapher and I had to do a lot to denormalize my collection data in order to avoid multiple subscriptions and assembling all the data is always very complicated. Can't wait to try Grapher more on a new small project, but it seems so promising. Thanks a lot!

davidsavoie1 commented 6 years ago

loadingComponent is checked with the check library as so:

check(options, {
  loadingComponent: Match.Maybe(React.Component),
});

But in the documentation example, LoadingComponent is a function that returns a component. The example would probably fail too, I guess. Any advice on that, @theodorDiaconu?

theodorDiaconu commented 6 years ago

@davidsavoie1 yes you're right. Documentation needs to be updated, and the check needs to be more permissive and allow anything. Currently you can use a React.Component as a work-around. Sorry about this. Currently very very busy, will be more free after 10th and will take care of this. You can also submit a PR if you like and I can publish it in few mins

theodorDiaconu commented 6 years ago

@davidsavoie1 solved.

Released 0.1.3

davidsavoie1 commented 6 years ago

Yup, that's working! Thanks!

You're right, I should make a pull request when I encounter such issues, especially if I nail the problem down to the source code. The thing is, I'm still learning and I've never contributed to open source projects so far...

What's the procedure? I clone the repo, make the change, test it properly and then submit a PR from Git (or any Git interface)?