This is inspired by #122, #110, #100 and acts a place to discuss API design
After some thinking and some tinkering. I think we can create a pretty awesome and reusable fragment enhancer. Here are my initial thoughts on it:
Fragment Enhancers do the following:
Usage example:
// enhancers.js
import { graphql } from 'react-apollo';
import gql from 'graphql-tag';
/*
GraphQL Enhancers (reusable and shareable)
*/
// this can be used by multiple parent queries (reusable)
// the $size variable could be hoisted but we would need a way to define type
const withPersonData = graphql(gql`fragment Avatar on Person { name, picUrl(size: $size) }`, {
// default and custom variables
options: (ownProps) => ({ variables: { size: ownProps.size || 200 }}),
// custom props
props: ({ data }) => ({ photo: data.picUrl, name: data.name }),
});
// this can be used by multiple parent queries (reusable)
// no variables
const withAddress = graphql(gql`fragment Address on Person { city, zip, street1 }`);
// this can use multiple fragment enhancers
// note the missing variable. Again we would need a way to define type if this is helpful?
const withPeople = graphql(gql`query(cursor: ID!) {
people(cursor: $cursor) { id, ...Avatar, ...Address }
}`, {
// query level variables
options: (ownProps) => ({ variables: { cursor: ownProps.cursor }}),
// included fragment enhancers
fragmentEnhancers: [withPersonData, withAddress]
}
);
export { withPeople, withAddress, withPersonData }
// components.js
import { withPeople, withAddress, withPersonData } from './enhancers.js'
/*
Components
*/
const Person = ({ person }) => (
<li>
<h1>{person.name}</h1>
<img src={person.picUrl} />
</li>
);
Person.PropTypes = {
person: PropTypes.shape({
name: PropTypes.string,
picUrl: PropTypes.string,
}),
refetch: PropTypes.func
}
const PersonWithData = withPersonData(Person);
const People = ({ data }) => (
<ul>
{data.people && data.people.map(person => (
<PersonWithData key={person.id} /> // notice no prop of person passed through
))}
</ul>
);
const PeopleWithData = withPeople(People);
People.PropTypes = {
data: PropTypes.shape({
loading: PropTypes.boolean,
people: PropTypes.array,
error: PropTypes.object,
refetch: PropTypes.func,
})
}
Benefits:
Sample use case:
Say I had an avatar component. It was responsible for showing an image and a first name for a user. This UI could be shared across a number of different data sets. Take for instance /profile. This fragment could be used on the top level query that returns a Person type. Now I navigate to a group page which lists GroupMembers made up of Person types. The same fragment UI component could be used where the fragment is on a nested level of the query. If I need to add the nick name instead of first name, its done in one place.
Now let's say I want to allow a user to update their avatar anywhere it is shown. I can request the user id of the Person type, and add a mutation operation to the enhanced component which will update the users photo and call refetch of the query the fragment is used in once finished.
With fragment enhancers, all of this presentational, data fetching, and data updating logic live within a shared component grouping (two enhancers and one presentational component). If I wanted to have two avatar styles I could reuse the enhancers between the two of them but show entirely different UI. This would also allow a react-web and react-native avatar component to share more than 2/3 of the same code and just render / pass the file in a uniform manner.
Thoughts?
@stubailo @tmeasday @rricard @saikat @tlvenn
One thing I'm not sure about - how do you associate get the result data into the child component? Like let's say you have an array of todo items, and you have a fragment for the todo fields. You have multiple todo items in the result, so how do you know which result goes with which child component?
I agree with @stubailo on this, it feels like magic, I have personally nothing against passing a person prop, it makes things clear from where the data is coming from...
const People = ({ data }) => (
<ul>
{data.people && data.people.map(person => (
<PersonWithData key={person.id} /> // notice no prop of person passed through
))}
</ul>
);
This part is really confusing to me... Is the person inference done through the key ?
@stubailo that isn't clear because it isn't possible 馃憥 yep this idea won't work.
@jbaxleyiii I'm not sure it's worth closing it though, I really think we can make it work, just with transmitting the props explicitly...
@rricard I'm just not sure of what the value is of something like this? Variables will still need to be declared on the operation level, fragments still need to be attached to the operation, and I don't think we could reliably refetch a parent query based on a fragment without manually passing those props down?
I mean, having something like @stubailo proposed earlier in #122 may be interesting: https://github.com/apollostack/react-apollo/issues/122#issuecomment-235949889
Maybe just making the fragments a bit smarter would do the trick (for instance, having the possibility to refetch or fetchMore on a fragment could be cool, this would be achieved by having the fragment refresh all of the queries it's used in ...)
@rricard I think my concern with that is fragments can be used in vastly different queries. I don't think refreshing all queries from a fragment is a good idea
yea, I agree, maybe do a similar system that what you did with mutations, target queries by name ... I dunno, just proposing
I think the
Oops, was gonna say: the problem with this is I think the issue has too many proposals. For example, I think having a way to statically attach fragments to components is great. Once we impement that, we can keep looking at other ways to improve. But I don't think we should come up with a grand multi-step plan for it yet, let's make some incremental improvements and see where it goes.
@stubailo fwiw, that already exists ;) If you pair an operation and fragments at least! I can make graphql(FRAGMENT) do the same if its helpful
@stubailo Apologies if this is the wrong place to ask this, but just for clarity: do I have to have a single query attached to my very topmost component in order to be able to use react apollo? I can't define separate GQL queries for each individual child component?
@masaeedu you can define individual queries for each component.
Ok, here is something I came up with recently, using the new React context API: https://github.com/lucasconstantino/react-apollo-fragments
Even though I mention in that project that this functionality should be part of react-apollo, maybe due to the amount of possible solution proposals to the problem perhaps it should sit on a companion project after all.
Things that are not yet clear as how I'll address:
Todo:
(p.s: I have no idea how did I unassigned @tmeasday on this :scream:)
What's the sitch with @lucasconstantino's proposals? It looks sound.
I've got something I'm working on also. Not super well documented, but I've been using it. https://github.com/brysgo/monoquery-workspace
Has anyone here used Apollo's query batching feature? Fragment composition would be a great feature and I'm all for it, but in the meantime I'm wondering if query batching would be a good solution to the problem of multiple queries (resulting from co-locating data dependencies with each component). Or is solving that problem not even a primary goal of this proposal?
Most helpful comment
Ok, here is something I came up with recently, using the new React context API: https://github.com/lucasconstantino/react-apollo-fragments
Even though I mention in that project that this functionality should be part of
react-apollo, maybe due to the amount of possible solution proposals to the problem perhaps it should sit on a companion project after all.Things that are not yet clear as how I'll address:
Todo: