-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Atomic Fragment Subscriptions Discussion #314
Comments
I think this is a subset of the kind of batch functionality that has been seen in a few talks before - something that basically lets you say "Do this operation, then do this other operation based on the result." I feel like a solution based around fragments and referencing fragments goes pretty far from how GraphQL works today. The snippet above looks pretty similar to your option 3! |
The same issue was raised on the original RFC thread: #267 (comment) Another option discussed there was to provide a way for a
All in one. For example, something like: type Subscription {
userAvatarChange: UserAvatarSubscriptionResponse
}
# UserAvatarFirstReponse is returned for the initial request,
# UserAvatarUpdate is returned for later updates.
union UserAvatarSubscriptionResponse = UserAvatarFirstResponse | UserAvatarUpdate
type UserAvatarFirstResponse {
user: User
}
type UserAvatarUpdate {
avatarURL: String
}
# ...
subscription {
# Subscribe to the `userAvatarChange` event
userAvatarChange {
# On the initial response, load the user data:
... on UserAvatarFirstResponse {
user {
name
avatarURL
}
}
# On later updates, get the new avatar URL:
... on UserAvatarUpdate {
avatarURL
}
} |
I do think it would be cool to be able to decouple the initial result from the subscription though, and just use any query from the schema. |
It's pretty convenient to get the updated values after running a mutation. This case seems similar to me, and I think fragments would work well for code sharing! |
Yeah fair point! This could also work just like the selection set on a mutation, except one of the fields on a query could be a subscription. Hmmm. |
@stubailo Ha yeah that batching approach was the inspiration for option 3 actually! I do see a few issues with it though:
@rmosolgo That comment is actually from my colleague also working on this architecture 😄 As he mentioned, we have versioning built right now where the server responds with a version that the client sends back on every request to the server. The server can then replay any dropped events from its events cache. This approach currently works great! But it has required an investment on both the client and server to make this as easy to use as possible and I don't think the GraphQL community as a whole should advocate adding this complexity. I think we can do better with a few syntactical changes. In terms of the subscription returning the current state of the server, this can work for "refetch" subscriptions that update parts of the response. But how does it work where the subscription returns a description of data to update? Consider the following very basic scenario: query {
users {
todos {
todo {
id
name
description
completed
}
}
}
} And then inside our client side code, for each user: subscription {
todoOrderChanged(userID: <id of user> ) {
todo {
id
}
newIndex
}
} In this scenario if we were to have the subscription return the initial state, it would have to return all Again this is a very basic example. To get around this particular scenario with the current way this is all implemented, I may add an Fundamentally all of these issues arise due to the way that subscriptions are created. Because they're separate operations that aren't closely linked to the pieces of data initializing them, these types of arise. Subscriptions were modelled after mutations which while they do update pieces of data on the client, they are instantiated off data living on the client. Subscriptions are instantiated off data coming from the server and thus doesn't make sense to be instantiated from the client. I feel like if we embrace this within the language itself, we can avoid these issues entirely and provide a more robust and efficient realtime experience within GraphQL. That being said, thanks for bringing up these approaches! Let me know if I can clarify anything. |
Also for cases where there is an update to existing data, I think the |
For the sake of conversation, here's an adaption of the query + subscription above as a single subscription which returns initial state: type Todo { ... }
type TodoOrderChangedInitialState {
user: User
}
type TodoOrderChangedPayload {
todo: Todo
newIndex: Int
}
# ...
subscription LiveTodos {
todoOrderChanged(userId: 1) {
# The first response contains the list of todos
... on TodoOrderChangedInitialState {
user {
todos { ... }
}
}
# Later updates contain id-index patches for updating the client data
... on TodoOrderChangedPayload {
todo { id }
newIndex
}
}
} Does that address some of the concerns mentioned above? I can imagine that, even if this workflow was supported, it would add some complexity to data fetching, since some data would come from Personally, I love the simplicity of GraphQL's syntax and semantics, so I'm inclined to look for a solution within the current language! |
Of course, would love a solution within the language! However I'm not seeing one that satisfies the requirements. Your adaptation still has the fundamental issue of overfetching the Also consider the following scenario: Todos are added, updated, removed, and reordered. Let's say we have four subscriptions for these events respectively. Once the These subscription requests all are received by the server at different intervals. If the server receives some subscription requests before others, and certain events change the result of further subscriptions, it could create an inconsistent experience on the client. Going back to your proposal of having subscriptions return the initial state, it could help with this problem but could also create an inconsistent experience for the client. For example, a I still believe that fundamentally because these are all separate operations originating from the initial query, we run into these types of issues. I'm unsure if we can tackle this with the current semantics, but let me know if I'm missing something or if it can be done differently! |
Ohhh I didn't properly understand the issue before, but now I think I understand! If you have two subscriptions that observe changes to the same set of initial state, then returning initial state for both subscriptions is wasteful. For example: subscription TodoOrderChanged {
todoOrderChanged(userId: 1) {
# The first response contains the list of todos
... on TodoOrderChangedInitialState {
user {
todos { ... }
}
}
# Later updates contain id-index patches for updating the client data
... on TodoOrderChangedPayload {
todo { id }
newIndex
}
}
}
# AND
subscription TodoDeadlineChanged {
todoDeadlineChanged(userId: 1) {
# The first response contains the list of todos
... on TodoDeadlineChangedInitialState {
user {
todos { ... }
}
}
# Later updates contain id-index patches for updating the client data
... on TodoDeadlineChangedPayload {
todo {
id
deadline
}
}
}
} In a case like that, you end up with a double-response of the initial state, when only a single response would have done the job! Beyond that, the fact that they are separate operations means that they could return different results -- even though they're requesting the same data! And reconciling that scenario is 🙅 . I see why initial response from |
No problem @rmosolgo, I appreciate you bringing this up as I'm sure others that may be reading this are thinking the same! |
I'm going to close this issue since this discussion ran its course! |
Hey everyone,
I spoke about this at GraphQL Europe this past weekend and would like to take the discussion of how to solve the issue that I spoke about here. Overall, I'm unsure if the solution is to extend directives to do what I proposed (and therefore Arguments), to introduce a new construct for fragments, or if there are other ways to solve this problem entirely.
Description of the Problem
Fundamentally, queries and subscriptions are two separate operations. Therefore if there is a subscription that is created as the result of a query, or a subscription that needs to be re-created as the result of changed data from another subscription, there could be dropped events in the gap between the two operations. Below is a diagram I used in my presentation between a client and a server with the network in the middle:
We've currently circumvented this by adopting versioned responses for both queries and subscriptions, thus replaying any dropped events on the subscription itself once it is setup. However ideally I would like this to be a single atomic operation, thus looking more like this:
I quite like subscriptions as the body of the subscription can effectively describe any type of operation for the client to perform; be it a direct data update, a data manipulation, or a description of operations to perform on data. Thus I do not believe that
@live
directives or similar truly solve this issue for all cases that subscriptions can.Proposed Solutions for Discussion
@subscribeTo
directive that references the subscription body fragment:As directives are using
Arguments
inside them, this change would requireArguments
to be able to reference other fragments which could have adverse effects forArguments
used in any other context and therefore I'm not entirely sure that extendingArguments
is the right approach here.This proposal doesn't touch
Arguments
as it's a whole new extension. However it does introduce new concepts to the GraphQL spec which come with its own considerations.I personally like a solution like this the best as it can leverage existing constructs. However it's unclear how the subscription can be linked to each occurrence of the fragment in the query.
Next Steps
I've opened this issue for discussion purposes before hashing out whatever needs to be changed in the spec itself. Let me know your thoughts, concerns, questions, and ideas.
Looking forward to solving this issue, simplifying our stack, and offering the community with a path forward on these types of issues!
The text was updated successfully, but these errors were encountered: