Replies: 6 comments
-
I have modified the spec PR I have been working on for deduplicated incremental delivery to reflect this discussion. It turns out that what I had been working on actually most closely matches Option B which is significantly easier to specify. I have raised a different PR to implement option A. The first commit in that PR retains defer ordering across multiple levels, but not the same level, corresponding to the status quo of the implementation, and the last commit retains defer ordering even at the same level, i.e. Option A above. Most of the "work" within the PR is done by that first commit, with Option A is only slightly more complicated to specify than the implementation status quo. We shouldn't necessarily do what's easiest to specify! It would be even easier to specify Option (C) where all defers are siblings, i.e. sent as soon as they complete, even if they cannot yet be merged into the final reconcilable object. That's still not the best option, because the simplicity of the specification created somewhat of a processing headache for the graphql client! On the other hand, ease of specification is certainly something to keep in mind. |
Beta Was this translation helpful? Give feedback.
-
My opinion is that we should move forward with Option B (graphql/graphql-js#4003 (i.e. making Inner and Outer siblings for ExampleC just like in ExampleA, but still not for ExampleB). I have two reasons for this:
|
Beta Was this translation helpful? Give feedback.
-
Above, I listed the benefits of option A and option B, but I left out an important point, related to early execution. @benjie in the linked discussion did a great job of explaining why early execution might lead to delayed delivery of the initial result. The example he used had only a single defer: {
... @defer { superExpensive }
cheap
} My general conclusions from that discussion were that: Let's apply the above to the following operation: {
... @defer {
... @defer { superExpensive }
cheap
}
reallyCheap
} According to Option B, this is the same as: {
... @defer { cheap }
... @defer { superExpensive }
reallyCheap
} And so "technically" in some sense, But that very decision causes early execution with respect to developer intent (as well as early delivery)! So I think we have to preserve (B) in this context => allow disabling this by default/provide mechanisms for management. But what about (A), minimizing the difference in terms of the spec? My initial experience was that delayed execution was easier to specify, but with some work I was able to pare that down under Option B. Now Option B (with its own early execution problems) is actually simpler to specify than Option A! Bottom line, I don't think we have a choice though, because of the early execution issues, I think we need to specify Option A, with a normative note that this might be unnecessary if Option B is pursued. Although I will continue iterating! Continued feedback welcome! Looking forward to discussing at the next meeting! |
Beta Was this translation helpful? Give feedback.
-
Example of why the nested Inner must not be delievered until Outer is delivered: fragment PostDetails on Post {
body
... @defer {
bodyHighlights # Uses an LLM
PostDetailsIsReady: __typename
}
} query ($id: ID!) {
currentUser { name }
post(id: $id) {
id
title
icon
... @defer { ...PostDetails }
}
} Here the PostDetails fragment's component knows it can render |
Beta Was this translation helpful? Give feedback.
-
based on feedback from meeting today: |
Beta Was this translation helpful? Give feedback.
-
In today's incremental delivery wg, we decided to proceed with Option A. The example in #80 (comment) demonstrates that is likely what is most expected by the user, or require more complexity in clients to work around it. Marking this discussion as resolved and opened #83 to discuss how the "pending/complete" objects in the response format should behave. |
Beta Was this translation helpful? Give feedback.
-
EDITED BELOW 2023-12-31 and 2024-01-03
Continuation of #43, as updated by the new response format.
TLDR:
In
ExampleB
below, we have assumed thatInner
must always followOuter
. The implementation currently makesInner
andOuter
siblings for belowExampleA
, but not for similarExampleC
. Why?Nested Defers at the Same Level
Example operation,
ExampleA
:Should
Inner
be sent once afterOuter
or shouldOuter
andInner
be considered sibling defers?Benefits of Always Preserving Defer Dependencies
Full conformance to "actionable payloads only" principle
The client has indicated that the component for
Outer
has anInner
portion that is "deferrable"; it also has non-deferrable portions. Those non-deferrable portions are not sent prior to all of the fields forOuter
being ready. Why should the deferrable portions ofOuter
(i.e.,Inner
) be sent when ready before non-deferrable portions are ready? Just as individual fields forOuter
such asfieldForOuter
((or nested fragment spreads!) are not sent until all of the fields forOuter
are ready, neither shouldInner
.More consistent handling of errors.
If
Outer
andInner
are made siblings:If
Inner
completes first but with an error causing the entire fragment to bubble up,Actually,Inner
would sendme
asnull
with errors, andOuter
would be suppressed as non-mergable, even thoughInner
is deferrable =>Outer
can be interactive without it =>Inner
could be useful as an error boundary.Inner
would complete with errors as non-mergable into the final response, and thenOuter
would complete normally. This seems fine, as long as the client knows that a non-mergable fragment as a particular path does not preclude later mergable data at that same path, which seems fine, as that's the basic definition of non-mergable.If
Outer
completes second but with an error causing the entire fragment to bubble up, without this change,Inner
would be successfully sent firstwith non-nulland a notification forme
Outer
is sent as non-mergable. In this situation,Inner
is actually useless to the client.If the defer dependency is preserved:
Inner
completes first but with an error causing the entire fragment to bubble up,Outer
is sent first without errors,Outer
is interactive, after which a notification forInner
is sent as non-mergable.Outer
completes second but with an error causing the entire fragment to bubble up,me
is sent asnull
, uselessInner
is not sent.Benefits of Allowing Sibling Defers
What does "actionable" mean?
Inner
is in a sense actionable as soon as it completes, because the graphql client building the "final reconcilable object" can do something with it, i.e. can insert the data under theme
key within the initial result. Although the client application cannot yet use this data, the graphql client has a place to store it. This is as opposed to the following counter-exampleExampleB
:In this case, until
Outer
completes, the graphql client cannot even storeInner
in the final reconcilable object, it would instead have to saveInner
in some sort of queue and then whenOuter
is sent with its "pending" notification forInner
, merge both. We have already considered that to be undesirable secondary to the "no action-less payloads" principle, but we allowedInner
to be sent prior toOuter
inExampleA
, because even though it's not actionable by the client application, it's actionable by the graphql client.To buttress support for the actionable claim for
ExampleA
, we could argue that it is useful for the client application if not immediately actionable, because it allows a form of pre-loading;Inner
will be instantly available within the final reconcilable object. A distinction must then be drawn between preloading inExampleA
andExample B
.Avoiding Defer Creep
@leebyron has raised the concern of fragments sprawled across a code base leading to unintended unnecessary defers. This would seem to be a broader concern not entirely mitigated by specifically changing the behavior for nested defers at the same level, but it would certainly cut down on the number of unintentional defers (at the expense of ignoring intentional defers!).
Allowing More Siblings Based on Deduplication Effects
Let us assume that in
Example A
we have decided to makeOuter
andInner
siblings.Consider the following operation,
Example C
:Because of deduplication,
me
is sent within the initial result rather than withinOuter
, and so the operation is equivalent to:Similar to
ExampleA
, we might then expectOuter
andInner
to be treated as siblings:Inner
inExampleC
andExampleA
are essentially equivalent. Offline, @robrichard noted to me that this was his expectation.Inner
is not immediately obvious (and in fact may be unknowable until runtime depending in the presence on type dependencies/variables), onlyOuter
andInner
should only be considered siblings inExampleA
and not inExampleC
. In fact, this is the state of the current implementation in graphql-jsConclusion
In
ExampleB
, we have assumed thatInner
must always followOuter
.The status quo on main within graphql-js makes
Inner
andOuter
siblings forExampleA
, but not forExampleC
, whereInner
still followOuter
, i.e.Inner
always followsOuter
, except when they are at the same level.I have raised separate PRs to explore the options of:
(A) making
Inner
always followOuter
, including where they are at the same level like inExampleA
, and(B) sending
Inner
as soon as the graphql client can merge it into the final reconcilable object (i.e. makingInner
andOuter
siblings forExampleC
just like inExampleA
, but still not forExampleB
).I have not raised a PR for the more aggressive option (C) of always sending
Inner
as soon as it completes, i.e. even inExampleB
, although I consider this not so much less reasonable than (B).I lean at this point toward Option A, followed by Option B, with the current status quo my third choice, and lastly option C, but eager for input from others!
Beta Was this translation helpful? Give feedback.
All reactions