Defer directive

[rmosolgo]

This is the first step to support @defer and @stream as described in https://www.youtube.com/watch?v=ViXL0YQnioU

TODO:

• Need a migration path for graphql-batch + @defer
• Need a way to run the same tests on both execution strategies
• Make defer and stream opt-in
• Oh, yeah, and implement @stream
• Good API for providing a collector for query resolution I think injection by context: is ok, you'll need to make new instances all the time anyways because they have to contain their patch targets
• Can we be more efficient with Frames and new path arrays? in some cases we don't need new ones? I think it's fine, if you made a new one before finishing a returned value, you'd waste it sometimes but use it verbatim other times, not clear if it would be an improvement
• Handle case when @defer'd field raises an error which is rescued by middleware (stop using query.context) not a problem, middleware actually returns the error
• Deferred fields should be absent from the initial patch, not nil (because they may be non-null types)

[rmosolgo]

I don't actually want to make this default yet, I just need a way to run the whole test suite on it

[metacortex]

I really want this 😄

[rmosolgo]

Pushed demo here: https://github.com/rmosolgo/graphql-ruby-stream-defer-demo

[rmosolgo]

I merged some of the refactors from this branch into master, that way it will be easier to keep this branch up to date.

[rmosolgo]

Some implementations https://github.com/rmosolgo/graphql-streaming

[staugaard]

This is amazing.

At Zendesk we have a fairly large application written on graphql-ruby, and would jump on the @defer directive on day one.

Are you actively working on shipping graphql-streaming?

[rmosolgo]

The big todo before putting this in master is fixing this problem:

Tests which depend on SerialExecution are littered throughout the test suite. For example, "non_null_type_spec.rb" contains tests about how null values are handled during execution. Both SerialExecution and DeferredExecution need to pass this test, so, how can we make that happen? I can think of two approaches, or maybe three:

1. Parameterize the test suite. Accept an argument for which execution strategy to test, eg rake test DEFAULT_EXECUTION=DeferredExecution. This seems easiest to implement but introduces a weird "gotcha" to the test suite.
2. Extract execution-dependent tests into a module, then include that module into both serial_execution_test and deferred_execution_test. This is good -- code sharing is done with Ruby rather than a weird workaround.
3. The real golden ticket would be to extract those tests into a module and put it in lib/. In theory, execution strategies are part of graphql-ruby's public API, and I think it would be nice to provide "compliance tests" for any user-provided executor. As far as I know, graphql-batch is the only "third party" executor, so this is not a high priority. This approach also appeals to me in the parser department, as that would make it easier to keep graphql-libgraphqlparser up-to-date.

I think eventually, DeferredExecution could replace SerialExecution altogether. It provides serial execution, but the "threaded" implementation is much more robust -- it even lays the groundwork for parallelism in multithreaded Rubies (sadly not part of my world). But I don't want to hose graphql-batch or its users, and that depends on SerialExecution. So, I'm thinking about rolling batching right into deferred execution, so that feature won't be lost. That will take some work :)

As for graphql-streaming, I think ActionCable support is experimental, to say the least, but ActionController::Live (which supports @defer and @stream, but not subscription) is ready to roll -- in fact, if you see the implementation, you'll see it's quite boring 😆.

There's one deficiency in the client, and I'm not sure how to beat it yet: when we receive incoming patches, we re-split the entire patch string, instead of just identifying the newly-added part. As the patch string grows, the cost of splitting it grows. I'm not sure how bad the cost is, but you can see the relevant lines here: https://github.com/rmosolgo/graphql-streaming/blob/master/lib/graphql/streaming/clients/graphql-streaming/streaming_graphql_client.js#L39-L41

[theorygeek]

So this is perhaps a bit of a side-issue, but maybe this is a good place to ask anyway. I'm trying to take a look at the various places where outside code can plug functionality into graphql-ruby, and it's principally two places:

• Instrumentation (and the kinda-deprecated middleware)
• Swapping out execution strategies

That second one, different execution strategies, is a bit confusing for me. To be honest, my knowledge of execution strategies doesn't go much further than, "You have to add this line of code for graphql-batch to work," and there's a similar line that makes @defer work.

I'm wondering if execution strategy is not really a good extensibility point? We're already thinking that async should be part of the core library (this would obviate graphql-batch's execution strategy, right?), and it looks like DeferredExecution could replace SerialExecution.

It seems that if graphql-ruby took a stance on execution strategy, we'd gain:

• Simpler, more-powerful ways to hook up instrumentation (maybe, before/after waiting on promise resolution)
• Ability to combine features like batching, @defer, and @stream - and here especially, it seems like library consumers probably don't want to have to mix-and-match plugins to gain this functionality. (And I can't imagine plugin authors want to have to think about how their plugin mixes-and-matches with someone else's)

From what I can see, the casualty would be parallel execution. My thoughts on that, though:

• How popular is this use case?
• In my experience, parallelism is a good choice on the server primarily for I/O. In contrast, especially in Ruby, parallelizing CPU operations is not. But doesn't async already solve the I/O case?

[rmosolgo]

Totally agree! From my perspective, this feature is next on my list, for all the reasons you mentioned (plus because it will fix all the "path" bugs 😖 ),

I think you're right about execution_strategy as an extension point: considering its only consumer is a library that extends the built-in strategy to compensate for shortcoming of this gem, it's not actually too valuable of an API!

[rmosolgo]

Someday 🌥

[skmvasu]

Hi @rmosolgo Using this library for a while now. Really loving it :)

But we are missing the the @defer directive very much. Is there a timeline in which you're planning to push this? I'd love to help if you need something in this.

[rmosolgo]

I'm hoping to rewrite the GraphQL runtime this summer, and supporting custom directives is one of the requirements :) My first tinkering is on #1394 , keep an eye there or watch this repo to follow development.