Skip to content
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

Render to children instead of "render" prop #14

Closed
sampl opened this issue Apr 13, 2018 · 2 comments
Closed

Render to children instead of "render" prop #14

sampl opened this issue Apr 13, 2018 · 2 comments

Comments

@sampl
Copy link
Contributor

sampl commented Apr 13, 2018

Last one I promise :)

This one is really a matter of opinion, but I think rendering to this.props.children instead of a "render" prop makes things a little cleaner.

The React docs describe using this method: https://reactjs.org/docs/render-props.html#using-props-other-than-render

Just my 2¢

// CURRENT

<FirestoreCollection
  path="stories"
  sort="publishedDate:desc,authorName"
  render={({ isLoading, data }) => {
    return isLoading ? (
      <Loading />
    ) : (
      <div>
        <h1>Stories</h1>
        <ul>
          {data.map(story => (
            <li key={story.id}>
              {story.title} - {story.authorName}
            </li>
          ))}
        </ul>
      </div>
    );
  }}
/>


// PROPOSED

<FirestoreCollection path="stories" sort="publishedDate:desc,authorName">
  {({ isLoading, data }) => {
    return isLoading ? (
      <Loading />
    ) : (
      <div>
        <h1>Stories</h1>
        <ul>
          {data.map(story => (
            <li key={story.id}>
              {story.title} - {story.authorName}
            </li>
          ))}
        </ul>
      </div>
    );
  }}
</FirestoreCollection>
@green-arrow
Copy link
Owner

@sampl - I'd be happy to accept a PR that adds the children prop as another valid rendering method!

@sampl
Copy link
Contributor Author

sampl commented Apr 16, 2018

OK! I'll take a stab at it, stay tuned 😃

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

No branches or pull requests

2 participants