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

Composing requests within the connect function (enhancement) #193

Open
horyd opened this issue Jan 19, 2018 · 1 comment
Open

Composing requests within the connect function (enhancement) #193

horyd opened this issue Jan 19, 2018 · 1 comment

Comments

@horyd
Copy link

horyd commented Jan 19, 2018

I can see in the docs the ability to use PromiseState.all in the component to put together a number of existing Promise states on the props. This is cool, however I was thinking it would be particularly neat if it could be abstracted out to the connect function such that a composed PromiseState could be directly passed down into the props from there.

For example:

connect(props => ({
  usersFetch: props.users.map(user => `/users/${user.id}`)
}))(App)

Which is then interacted with in the component App like so:

render() {
  return this.props.usersFetch.fulfilled &&
  this.props.usersFetch.value.map(user => (<User user={user} />))
}

Here it defaults the composed PromiseState to the completion of all of the contained PromiseState as opposed to the race option (can't think off the top of my head of a way to allow this to be nicely configurable though, since passing the array is the neatest and most simplistic means at the moment)

Would love to contribute to this project, so if this is a good idea and we can agree on implementation I'd happily code up a PR 👍

@horyd horyd changed the title Composing requests in the same form as the current composing responses Composing requests within the connect function (enhancement) Jan 19, 2018
@ryanbrainard
Copy link
Contributor

This isn't a bad idea, but when I needed something similar, I ended up just making a component to do the fetch the individual record. For example, instead of having a connect in App, add a connect into User:

render() {
  return this.props.users.map(user => (<User userId={user.id} />))
}

and then inside User:

connect(props => ({
  userFetch: `/users/${props.userId}`
}))(User)

This isn't really a composition and won't block like PromiseState.all does, but just wanted to throw it out there in case it solves for your use case. I know the conventional wisdom in React is to have data fetching and "smartness" be at the top-level and have children be "dumb" components, but with the way react-refetch works, I've found it actually better to have fetching happen at the lowest level possible (i.e. just high enough so you're not fetching any shared data twice), so just something to consider.

With that said, if you want to submit a PR, I think it could be a nice addition.

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