facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.41k stars 1.83k forks source link

Cannot process a response that ignores @defer directives #3904

Open kirkbyo opened 2 years ago

kirkbyo commented 2 years ago

It has been discovered that the Relay runtime currently comes with the expectation that fragments marked with the @defer directive are expected to be deferred. Even when the network returns its corresponding value. This leaves fragments marked with the directive to have an undefined value. As far as we can tell, it is due to this function which bakes the assumption into the library.

This behavior seems to be against the current specification, and the original RFC:

... GraphQL clients must be able to process a response that ignores the @defer and/or @stream directives.

We had to make some larger changes to our execution engine to support defer. Having the ability to toggle the defer on the backend lets us roll out the changes slowly and fallback to previous codepaths in the event of regression. This is also a blocker for advanced usage cases in the future. For example, one thing we were considering was merging unnecessarily deferred fragments back into the initial response to the client. This would avoid having to do a pass over the store a second time when not necessary.

Is there a possible workaround or a relatively trivial way to support this behavior?

Code Sample

function Screen() {
 const query = useLazyLoadQuery(
   graphql`
     query ScreenQuery {
       viewer {
         name
       }
       ...ScreenContent @defer(label: "ScreenContentFragment")
     }
   `,
   {},
 );
 return (
   <div>
     <div>{query.viewer.name}</div>
     <ScreenContent queryRef={query} />
   </div>
 );
}

function ScreenContent(props) {
 const content = useFragment(
   graphql`
     fragment ScreenContent on Query {
       newsArticles {
         edges {
           node {
             title
           }
         }
       }
     }
   `,
   props.queryRef,
 );
 console.log(content); // <---- content is null
 return <div>News Articles</div>;
}

Response from the server:

{
  "data": {
    "viewer": {
      "name": "Ozzie"
    },
    "newsArticles": {
      "edges": [
        {
          "node": {
            "title": "Hello World"
          }
        }
      ]
    }
  }
}

Example Project: https://github.com/kirkbyo/relay-use-fragment-defer

ernieturner commented 2 years ago

Not necessarily surprising data points, but also worth noting that using the if argument to the @defer directive has the same behavior as mentioned above as well. Some more data from experimenting with this

@defer(label: "..", if: false)

This actually causes Relay to remove the @defer directive entirely from the query that is sent to the server. So something internally in Relay is checking for hardcoded boolean values and modifying the query. Not a bad thing, just thought it was interesting behavior

@defer(label: "...", if: $someVariableThatEvaulatesToTrue)

In this case, Relay expects that he data for the provided fragment to be delayed and will not process the result if it comes back in the original payload. This is consistent with not having the if statement at all, and is expected.

@defer(label: "...", if: $someVariableThatEvaluatesToFalse)

In this case Relay expects the fragments data to be in the original payload, similar to not having the @defer directive at all.

So all in all, Relay is being consistent with its processing of the @defer directive, it's just that the deferred data must not be in the first payload that seems to go against the spec. Also a fun exercise, doing the following will also work and will process the fragment from the initial payload

...DeferredFragment @defer(label: "...")
...DeferredFragment

That's also expected, but figured it was worth pointing out.

ernieturner commented 2 years ago

As an update here, it looks like this issue can be worked around as long as the JSON response has the extensions: {is_final: true}} flag set on it. This (somewhat) follows the defer spec, albeit with an outdated flag name, but not that big of a deal. This behavior might also be because of using Observables in the Relay network configuration too. However, it feels like there's technically still a bug here since I would expect Relay to flush all pending content when the sink.complete() is called and parts of the response haven't been processed. But at least we have a workaround!

Example server response:

{
  "data": {
    "viewer": {
        ....
    },
    "newsArticles": {
     ...
    }
  },
  extensions: { is_final: true }
}