Update description of Fragments to emphasize evolving data needs#1193
Update description of Fragments to emphasize evolving data needs#1193leebyron merged 6 commits intographql:mainfrom
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Definitely agree on the motivation for this change; thanks for the submission Janette! |
|
I think that is is great change overall. The challenge is a bit to make it understandable in the context of the spec. Using fragments to declare data needs (like in relay) always requires something on top of the spec to make it work. So we would have to make this clear somehow. |
martinbonnin
left a comment
There was a problem hiding this comment.
A couple comments but otherwise very supportive of updating that language!
benjie
left a comment
There was a problem hiding this comment.
Calling the logic friendProfile and the fragment friendProfile is confusing. Can we differentiate the logic - e.g. just call it "profile rendering logic" (prose) rather than using code? (Not sure if this is the best fix.)
| duplicated text in the document. Inline Fragments can be used directly within a | ||
| selection to condition upon a type condition when querying against an interface | ||
| or union. | ||
| Fragments allow for the definition of modular selection sets that make it easier |
There was a problem hiding this comment.
Need to flush out what modular means. Matt + Pascal's suggestion: single purpose.
|
Just updated based off some of the feedback from the working group meeting!
|
|
The video is just out. Can we merge this and iterate on the wording if needed. I'd love the message to be consistent. |
There was a problem hiding this comment.
I don't know how Lee feels about using _ in fragment names, but I'd be tempted to adopt Relay naming conventions here and be super explicit.
Perhaps we could lay out the entire scenario showing multi-level composition?
Example
Each data-consuming component (function, class, UI element, and so on) of a client application should declare its data needs in a dedicated fragment. These fragments may then be composed, following the usage of the components themselves, to form a GraphQL operation to issue to the server.
Consider a social media app. The userCard(user, layout) function renders a tiny details card about a user for inclusion in other components. For this, it requires certain information about the user which we can describe with a fragment dedicated to this purpose:
fragment userCard_user on User {
id
name
profilePic(size: 50)
}When rendering a user's profile with the userProfile(user) function, the userCard can be used to display each of the user's friends and mutual friends. The userProfile() function does not need to know about the specific requirements of the userCard() function since it can simply compose it's dedicated fragment in the relevant places:
fragment userProfile_user on User {
id
name
profilePic(size: 250)
biography
friends(first: 10) {
...userCard_user
}
mutualFriends(first: 10) {
...userCard_user
}
}This fragment can be used whenever a user profile is displayed, for example when hovering over a user, or on their dedicated user profile page (which may also need to compose fragments for additional concerns):
query userProfilePage($id: Int!) {
user(id: $id) {
...userProfile_user
}
...sideBar_query
...titleBar_query
}
I like this! The spec should be as framework and implementation agnostic as possible
Yes! This is great. I'm hearing from our users (RedwoodJS/CedarJS) that fragments are seen as a very advanced concept. A full example like this really helps. (And when this change lands, I'll definitely have to update the Cedar docs on fragments) |
Would like to keep examples camel case in the spec |
Co-authored-by: Benjie <benjie@jemjie.com>
Why these changes?
Fragments are not for reuse.
The current language in section 2.8 on Fragments, namely the sentence...
...encourages using fragments to deduplicate common selections, even if those selections represent independent data needs.
For example, let's consider these two functions
Given these two functions currently both use
authorNameandcontentText, the current language in the spec encourages one to write a fragmentIf
formatPostForFeednow needstimestamp, we will add naturally addtimestamptoPostDisplayFragmentIf we have the following queries
notice how
NotificationQueryis now over-fetching timestamp!The key observation is that
formatPostForFeedandformatPostForNotificationare two independent functions, so by having them both rely on a single fragment to express their data needs, we are creating a dependency where one should not exist (because that dependency does not exist in the product logic itself).What are the proposed changes?
Updated:
The goal is to emphasize that fragments support evolving data needs (as opposed to recommending people identify common selections that are currently in an executable document).
Open to any and all feedback on the motivation for the change and how it's communicated via changes in the spec language!