-
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
Spec: Formal Subscriptions Definition #305
Changes from 13 commits
4bba743
7b4fb56
328644b
c4aeaf9
f38092b
ca8af4d
3933a5a
d3220d6
e61963c
6d9dc8d
1321024
73b364f
d3011a5
23bd790
f7581ef
f18c569
a0aac7f
35c6059
ca2a02e
36359fd
47404cb
6425fed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,6 +198,67 @@ query getName { | |
} | ||
``` | ||
|
||
### Subscription Operation Definitions | ||
|
||
#### Single root field | ||
|
||
**Formal Specification** | ||
|
||
* For each subscription operation definition {subscription} in the document | ||
* Let {rootFields} be the top level selection set on {subscription}. | ||
* {rootFields} must be a set of one. | ||
|
||
**Explanatory Text** | ||
|
||
Subscription operations must have exactly one root field. | ||
|
||
Valid examples: | ||
|
||
```graphql | ||
subscription sub { | ||
newMessage { | ||
body | ||
sender | ||
} | ||
} | ||
``` | ||
|
||
```graphql | ||
fragment newMessageFields on Message { | ||
body | ||
sender | ||
} | ||
|
||
subscription sub { | ||
newMessage { | ||
... newMessageFields | ||
} | ||
} | ||
``` | ||
|
||
Invalid: | ||
|
||
```!graphql | ||
subscription sub { | ||
newMessage { | ||
body | ||
sender | ||
} | ||
disallowedSecondRootField | ||
} | ||
``` | ||
|
||
Introspection fields are counted. The following example is also invalid: | ||
|
||
```!graphql | ||
subscription sub { | ||
newMessage { | ||
body | ||
sender | ||
} | ||
__typename | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not clear what we would name this one if we wanted to get rid of the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a description above. |
||
} | ||
``` | ||
|
||
## Fields | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,8 +103,9 @@ Note: This algorithm is very similar to {CoerceArgumentValues()}. | |
## Executing Operations | ||
|
||
The type system, as described in the “Type System” section of the spec, must | ||
provide a query root object type. If mutations are supported, it must also | ||
provide a mutation root object type. | ||
provide a query root object type. If mutations or subscriptions are supported, it must also provide a mutation and subscription root object type, respectively. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: line break to maintain roughly 80c? |
||
|
||
### Query | ||
|
||
If the operation is a query, the result of the operation is the result of | ||
executing the query’s top level selection set with the query root object type. | ||
|
@@ -123,6 +124,8 @@ ExecuteQuery(query, schema, variableValues, initialValue): | |
selection set. | ||
* Return an unordered map containing {data} and {errors}. | ||
|
||
### Mutation | ||
|
||
If the operation is a mutation, the result of the operation is the result of | ||
executing the mutation’s top level selection set on the mutation root | ||
object type. This selection set should be executed serially. | ||
|
@@ -143,6 +146,127 @@ ExecuteMutation(mutation, schema, variableValues, initialValue): | |
selection set. | ||
* Return an unordered map containing {data} and {errors}. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting a comment here because silly GitHub won't let me comment on the lines above. Up on line 106-107 it says:
That should probably say:
Or words to that effect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also noticed that a step should be added to |
||
|
||
### Subscription | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we include some example responses here, and perhaps an example of a lifecycle for a simple subscription? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, I've added an example after the unsubscribe section. |
||
|
||
If the operation is a subscription, the result of the operation is the creation | ||
of a persistent object on the server that exists for the lifetime of the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "object" is a strange word to use in this context, IMHO. Perhaps the creation of a persistent connection? |
||
subscription. | ||
|
||
#### Subscribe | ||
|
||
The result of executing a subscription operation is a subscription object with | ||
the following capabilities: | ||
|
||
* **Must** support observation of the associated publish stream (for example, | ||
via iteration, callbacks, or reactive semantics). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what "iteration" means in the context of observation: do you specifically mean async iteration? (and if so, any concern that that is allowing JS-specific terminology to leak into the spec?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was referring to the general iterator pattern (async iterators also count). For example, Unity handles async operations via iterators/generators, so I don't think it's JS-specific. |
||
* **Must** support cancellation of the observation, see {Unsubscribe}. | ||
* **Must** support a way to send data to the subscriber. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this referring to the event stream? I think this point could be more specific, since it's not clear what "data" is, or which entity is the "subscriber". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The event stream is an abstract concept for the client. All the client sees is a stream of 0-n "publishes", each of which contains a "payload". I feel the use of subscriber is fairly clear here. Opting to keep this as-is unless you feel very strongly otherwise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK I completely misinterpreted this. So "subscriber" refers to the client consuming GraphQL, and "send data" means "send GraphQL results". |
||
* **May** include an initial response associated with executing the selection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the initial response supposed to be distinguishable from a regular response sent immediately after the subscription is started, from the point of view of the client? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think the initial response should be distinguishable from a publish. |
||
set defined on the subscription operation. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this list of 4 things confusing and am having trouble understanding what exactly it is that it's describing. Specifically, these things seem to be describing an implementation detail that a client of GraphQL doesn't necessarily interact with or care about. For example, as a client of your GraphQL server, how would I test that you're adhering to these? For point 1, how would I know that the payloads I'm receiving are the associated to a "publish stream"? For point 2, why should I care? I can always just disconnect from the network. For point 3, this is obvious - nowhere else in the spec do we state a requirement that GraphQL must be used along with a way to send data to the requester. Point 4 is out of place as a "may" - it likely belongs elsewhere, probably as part of the "publish" section below. What about writing lines 157-165 as: The result of a subscription operation is a sequence of events over time, each event containing the result of executing the provided GraphQL selection on the data associated with that event. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with collapsing the bullet points into the more concise form you have, but I think we'll lose point 4 in the process. I'll address this in your comment further down about initial responses. |
||
|
||
Most subscriptions can only be evaluated when event data is available. For | ||
example, a subscription tells us when users log on and off would require event | ||
data that tells us *who* logged on or off. Without this event data, the | ||
subscription cannot be evaluated. However, it is possible to define | ||
subscriptions that can be evaluated without event data (for example, the current | ||
time on the server, synthetically triggered every second). Subscriptions that do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A lot of this paragraph is describing implementation details, and IMHO is muddying the term "event data" used in context elsewhere. The example about an event triggered every second does contain event data - that event data is the current time on the server. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some subscriptions use the event only as a trigger. These kinds of subscriptions can be evaluated whenever, and are capable of returning an initial value. The "server time" subscription could be modeled either way. For example, if subscription's resolver calls Date.now() rather than expecting the time to come from the event data. I can reword this section if you still think it's confusing, but I think it's important to call out this sub-class of subscription because this is why we bother broaching the topic of initial result further down. |
||
not require event data for evaluation can optionally return an initial response | ||
to the Subscribe() operation. | ||
|
||
Subscribe(schema, subscription, operationName, variableValues, initialValue): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
ExecuteQuery(query, schema, variableValues, initialValue): So perhaps: ExecuteSubscription(subscription, schema, variableValues, initialValue): |
||
|
||
* Let {subscriptionType} be the root Subscription type in {schema}. | ||
* Assert: {subscriptionType} is an Object type. | ||
* Let {selectionSet} be the top level Selection Set in {subscription}. | ||
* Let {rootField} be the first top level field in {selectionSet}. | ||
* Let {eventStream} be the result of running {MapSubscriptionEvents(rootField, variableValues)}. | ||
* Optionally: let {data} be the result of running {ExecuteRequest(schema, document, operationName, variableValues, initialValue)}, and return {data} on the subscription object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is initialValue here? What's the value of specifying here and above (lines 166-7) that we can have an initial result? This leads to a lot of questions about how the execution works without any event that we don't address anywhere. Or is there discussion of this elsewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on this discussion #283, if someone builds subscriptions that can be evaluated without event data, we would not necessarily want that to be in violation of the spec. I've added more text above to clarify the reasoning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this very confusing and probably not what we want. If I understand this correctly, sometimes this algorithm results in a value Also, this algorithm does not describe what should be done with As a concrete example - what should I expect to see differently between a subscription which has an initial response vs one that does not, say in terms of websocket payloads? Do I actually get a Perhaps another way to frame this is that "initial value" could be a first published event which simply occurs instantaneously after the creation of a subscription? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's valid feedback, but I think merging the initial value into the publish stream might weaken the value proposition here. Let me try to give some context. Chat room application: we want to subscribe to when users log on or off to keep the list of members in a chatroom up-to-date. But the subscription doesn't give us the set of initial users in the room. So we issue a query for all users in current chat room, and then fire up the subscription. But now we have a small race condition between when we fetched the chat participant list and when the subscription is created where we are no longer "basing" off the right state. It's even possible that the subscription sends its first payload before the query returns. The client-side code to handle this kind of scenario is tricky. Can we do better? Returning an "initial value" is an attempt to address these sorts of scenarios: in other words, to create a sort of atomic hybrid query/subscribe. @taion, please chime in if I've misrepresented your requirements. I had proposed that the "initial value" can be tacked onto the "ACK" that comes with successful subscription creation, but there was some push back in the discussion. If it's not part of the ACK, then it will need another protocol message type, with the guarantee that if the initial response is sent at all, it will precede any publish payloads. If we merge simply set the initial value as the first publish payload, then we'll end up with something like this (from @ rmosolgo): subscription {
commentCreated(postId: 4) {
# `null` on first request, but present in later updates:
comment {
body
}
# present on first request, but `null` in later updates:
initialPost {
title,
comments {
body
}
}
}
} Maybe this is fine. I wanted to mention that returning initial value independent of the publish stream would not violate the spec. But you're raising some good questions. Definitely open to suggestions here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had (and still have) the same reaction as @leebyron to this part of the spec. I find it confusing and I don't really understand the value we're adding by explicitly mentioning it. As I mentioned before, this option raises a lot of questions for me that we don't really answer elsewhere. By calling out this "atomic hybrid query/subscribe" operation here, we're making it seem like a central concept for subscriptions (which I don't think it is). What if we leave this out of the spec until we can get more data on the demand for it and the best way to do it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @robzhu and I talked about this offline, but I just wanted to point out that this initialValue addition improves but doesn't guarantee a solution to race conditions. Events can still potentially be repeated or dropped between the generation of the initial value payload and subsequent payloads, depending on the system design and the latency of events flowing through it. I think the kicker here is that most examples of wanting initial data before subscription payloads desire a different query shape - in the example above a list of comments vs an individual comment. The spec as written wouldn't allow for this distinction, so I actually think your example is more powerful while requiring less from the spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Upon further discussion with @laneyk and @leebyron, initial value behavior can be supported out-of-the-box using two sub-fields in a subscription, as in @rmosolgo's example. However, the model of an "atomic query/subscribe" is probably better supported via batching. As a result, my next edit will remove any mention of initial value. To be clear, the resulting edit is that we will simply not mention server behavior with respect to initial value. If you choose to implement/use it, that does not make your server violate the spec. |
||
|
||
MapSubscriptionEvents(rootField, variableValues): | ||
|
||
* *Application-specific logic to map from root field/variables to events* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be filled in. In particular, nothing is referencing the algorithm in Publish yet, which should probably be referenced here. Or perhaps there's something missing? By name, I would expect this algorithm to map one set of events to another. Maybe this one should be renamed Also - this section on mapping event streams is a great opportunity to mention that while described as an inline algorithm, this step is often implemented across services, with the original event stream belonging to a subscription management service, and the execution happening on an API middleware service. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, you're right, "map" is misleading here. Changed to "CreateSubscriptionEvents" and hooked up the pseudocode from above and below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a reflection of the feedback on graphql/graphql-js#846 - let's break out the top portion of
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once the |
||
|
||
#### Publish | ||
|
||
Once a subscription is created, we listen for events on its event stream. Each | ||
event carries an optional payload, which we combine with the arguments from | ||
Subscribe() to resolve the selection set. | ||
|
||
* For each {event} and {eventData} on {eventStream}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This algo needs a name and arguments There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there value to describing |
||
* Let {data} be the result of running | ||
{ExecuteSelectionSet(selectionSet, subscriptionType, eventData, variableValues)} | ||
*normally* (allowing parallelization). | ||
* Let {errors} be any *field errors* produced while executing the | ||
selection set. | ||
* If {errors} is not empty, optionally, Unsubscribe() the subscription. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this basically just saying the subscription can be terminated at any time by the backend? Or is there something special about terminating in response to a particular kind of error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, from the client's perspective, the subscription could be terminated at any point by the server. Nothing specific here about what sorts of errors result in termination. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems to make this bullet point unnecessary, since it refers to a thing that can happen at this time or any other time, or may not happen at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree w/ @stubailo - since this could also optionally unsubscribe the subscription if I agree this bullet could just be removed here, but somewhere above mention that a subscription's stream of events may end at any moment, perhaps but not always in response to an error on the server |
||
* Yield an unordered map containing {data} and, optionally, {errors} on | ||
{publishStream}. | ||
* Let {publishStream} be the sequence of outputs yielded above. | ||
|
||
#### Unsubscribe | ||
|
||
The unsubscribe operation can be implemented in a number of ways. For example, | ||
by using a dedicated subscription manager, defining it as a method on the | ||
subscription object, or cancelling the iterator. | ||
|
||
Unsubscribe() | ||
|
||
* Terminate {publishStream} (For example, cancel iteration, detach mapping function) | ||
* Terminate and clean up {eventStream} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole section seems like implementation detail and not really relevant to what is or isn't GraphQL, could it be collapsed into a footnote in the top section on subscriptions? |
||
|
||
#### Example | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Elsewhere we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great suggestion! |
||
|
||
As an example, consider a chat application. To subscribe to new messages posted | ||
to the chat room, the client sends a request like so: | ||
|
||
```graphql | ||
subscription NewMessages { | ||
newMessage(roomId: 123) { | ||
sender | ||
text | ||
} | ||
} | ||
``` | ||
|
||
While the client is subscribe, whenever new messages are posted to chat room | ||
with ID "123", the selection for "sender" and "text" will be evaluated and | ||
published to the client, for example: | ||
|
||
```js | ||
{ | ||
"data": { | ||
"newMessage": { | ||
"sender": "Hagrid", | ||
"text": "You're a wizard!" | ||
} | ||
} | ||
} | ||
``` | ||
|
||
#### Recommendations and Considerations for Supporting Subscriptions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a long section title and it's specifically about scale. So perhaps just |
||
|
||
Supporting subscriptions is a large change for any GraphQL server. Query and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "significant" instead of "large"? |
||
mutation operations are stateless, allowing scaling via cloning GraphQL server | ||
instances. Subscriptions, by contrast, are stateful. The pieces of state for a | ||
subscription are: | ||
|
||
* Subscriber/client channel | ||
* Subscription document | ||
* Variables | ||
* Execution context (for example, current logged-in user, locale, etc.) | ||
* Event stream resulting from Subscribe step (above) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this list of things a bit confusing and overly detailed for the point you're trying to make. Perhaps just: Subscriptions, by contrast, are long-lived and stateful and require maintaining the GraphQL document, variables, and other context over the lifetime of the request. |
||
|
||
We recommend thinking about the behavior of your system when this state is lost | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spec shouldn't speak in the third person. Instead: Consider the behavior of your system when state is lost due to the failure of a single machine in a service. Durability and availability may be improved by having a separate dedicated service for managing this state. |
||
due to single-node failures. We can improve durability and availability by using | ||
dedicated sub-systems to manage this state. For example, event streams can be | ||
built using modern pub-sub systems, and payload delivery to clients can be | ||
handled by a dedicated client gateway tier. | ||
|
||
For systems with high capacity, availability, and durability requirements, we | ||
recommend keeping the GraphQL server stateless and delegating all state | ||
persistence to sub-systems that are designed for stateful scaling. Note that | ||
subscription types are still defined in the original schema along with queries | ||
and mutations. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This note is a bit out of place. You can define this stuff wherever the heck you want, that's implementation detail. Also, this paragraph is really just repeating the point made in the previous one. I think you can probably just remove it entirely if not move a few of the more fine-pointed words into the prior paragraph |
||
|
||
## Executing Selection Sets | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robzhu what is the reason for that? why can't i subscribe to more then one subscription with one call?
so far i thought this is implementation limitation..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Rob's text that starting with just one root field is reasonable. A later version of the spec could support multiple fields, but that requires figuring out a lot of edge cases around errors, when subfields are re-executed, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also would like to know the reason for this rule. I find that it is quite hard limitation, and in a way it goes against GraphQL principles where clients have the power to decide what it wants to get.
I think it would be helpful to list and discuss these edge cases. From my experience, there are definitely things to consider when merging different event streams from different GraphQL subscription fields, but I find it quite manageable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I listed most of the points I found as I was implementing it in sangria here:
#282 (comment)
I think that it boils down to points
3.i
,5
and6
. My take on it is here: #282 (comment)IMHO, if we need to make a trade off, I would rather disallow not-null root subscription field types than allow only a single subscription field in a query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stubailo if one wants to experiment that's fine, but i don't think it should be in the official spec unless there is a reason (which is not implementation) behind it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stubailo thanks a lot for describing in on an example!
I don't see it as a problem but rather as a natural behavior. This is inherent property of event-based interaction. This is also the reason why I suggested to disallow not-null root subscription field types.
I also don't see it as a issue either. Can you describe in more detail why this behavior can be disadvantageous? (considering that you still can make 2 separate and isolated subscription queries if it suits better for the use-case at hand)
I feel that either behavior is fine as long as it is defined in the spec. Though in this case I would suggest draw inspiration from streaming libraries: if 2 event streams are joined/merged together in a single result stream, then an error in either of these will also case the result stream to fail. If one of event steams naturally completes (because of the exhaustion), then the result steam still continue to emit events until all of the source streams are exhausted. I think this behavior is quite intuitive and widespread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OlegIlyenko @DxCx To give a little bit of context on where this came from, we discovered early on that it was better to use subscriptions for modeling granular events. For example, consider the three main subscriptions that operate on a facebook post: live comments, live likes, and typing indicators. These are individual subscriptions as opposed to a single "postLiveUpdate" subscription. Keeping these subscriptions granular on the client made natural sense. @laneyk and @dschafer may be able to add more perspective here.
Thinking this through, if we include multiple root fields like so:
So far, everyone seems to assume this subscription should publish data when either "live like" or "live comment" publishes. Is that clearly the intent of this query? What if there were a desire to trigger the publish only when both root fields have a publish payload available? How would we describe that? By limiting the selection to a single root field, we sidestep all that.
I also don't think the single-root-field-rule introduces any practical limitations to the client. In fact, it results in simpler client-side code, like so:
For subscriptions containing more than one root field, if we assume the "or" behavior, as @stubailo points out, you'll never have more than one event trigger at a time, so the code would end up looking like:
Can someone help me understand a compelling use case that is served by multi-root subscription operations that would not be equally served by separate individual subscriptions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add a meta-point to this conversation:
In my view, there's nothing stopping us from working through these details and figuring out the edge and corner cases of allowing multiple subscription fields in a single request, however the choice we do have is to address those concerns now, or allow for more time to do so. In previous conversations @robzhu has had over the last few months about subscriptions, he has convinced many that this is far more complicated than we originally thought and may not have clear answers. This limitation is added mostly in a desire to expedite the addition of subscriptions to the spec, while reserving the ability to continue to work out how or if multi-field subscriptions should be allowed.
Had this limitations been omitted while also not making mention of how to address multi-field subscriptions in the spec, then we would see divergence of behavior and that could tie our hands in the future for deciding how to address these edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @robzhu and @leebyron for the insightful comments! I think now I got a better understanding of the issue. I was also in two minds on this. On one hand, I wanted to get a better understanding about motivation behind inclusion of this rule. But on the other hand, I don't want to delay the progress on subscriptions incision in the spec. I tend to agree that it is a good idea to disallow multiple fields for now and start a separate discussion. I think it is a discussion worth having. I am actually very glad that this point is considered in the spec since I was also quite concerned about the semantics of multiple subscriptions fields.
In general, I found it very valuable to have as much information from client as possible in single query. For example, the fact that a client can express its requirements for a view or particular part of the application in a single query allowed us to make very interesting optimizations which would be quite hard to do otherwise (it is quite hard to correlate seemingly independent requests/queries). So by allowing client to better express it's requirements with several subscription fields in a single query, we open a door for potential server-side optimizations.
Now that I'm equipped with new insights, I will give it another thought. This thread was definitely helpful in this respect.
Before we will introduce this rule though, I think it is important to consider the nullability of the subscription fields, as i mentioned above. It is possible to make a nullable field not-null later on in a backwards-compatible way. If we allow subscription fields to be not-null now, it might become a challenge in future to allow multiple subscription fields, if we decide to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullability may mean two different things in this context - this is a great point we should address.
One thing it may mean is that a subscription may not exist given some inputs in a way that isn't considered an error. I think this interpretation is both not what you were referring to, and also probably confusing to think about. The schema talks about the type of the payload result - so we're talking about the types of responses. We should probably make this point in the spec to clarify.
Secondly the nullability of the responses. This is one of the concerns with multi-field subscriptions to address later. For example, should it be legal to have a subscription field
streamThings: String?
where it is legal for any payload in the event sequence to in fact benull
? I don't see a compelling reason to explicitly disallow this - though it is an edge case.I think handling the payloads of multi-field subscriptions will need to account for this