Add LoadedFieldExtension for batch loading individual fields

[dylanahsmith]

cc @Shopify/component-patterns

Problem

We have tended to avoid using promises throughout out codebase and have instead relied on preloading associations. However, as we split our codebase into components that aren't coupled to each other models, we are needing to stop relying on these associations for this purpose.

Instead, I would like a simple primitive that avoids the N+1 problem without having to think about promises, so that we can easily use a non-promise based component APIs.

Solution

Add the concept of a batch loaded field which can be specified next to the field declaration like a typical resolver method. However, instead of an instance method for the batch resolve method, a class method is used which is given an array of objects to load the field on. For example,

class Product < GraphQL::Schema::Object
  field :inventory_quantity, Int, null: false do
    extension GraphQL::Batch::LoadedFieldExtension
  end
  def self.inventory_quantity(instances)
    product_ids = instances.map { |instance| instance.object.id }
    quantities = ProductVariant.group(:product_id).where(product_id: product_ids).sum(:inventory_quantity)
    instances.each do |instance|
      instance.inventory_quantity = quantities.fetch(instance.object.id, 0)
    end
  end

The result is just assigned to an attribute writer on the GraphQL::Schema::Object instance, which fulfills the promise. Since this relies on there being a graphql object to assign the result to, this
feature depends on the graphql gem's class-based API.

In addition to not having to think about promises, this feature also simplifies grouping. It will automatically group by the graphql object class, field and arguments. If either the lookahead or irep_node extras are used, it will group by their ast_nodes since that means it depends on the selections on the field.

[eapache]

A few comments, but does what it says on the tin and seems like a step in the right direction. However, I'm not sure what the long game is here? The more complex loads will still want custom loaders (because the field itself will need to transform the loaded data post-load and it doesn't seem possible to share the loading step while splitting the resolve step with this approach) and the simpler loads will still need some sort of sugar which just wraps the natural resolver in a load. I'm not sure we have enough "middle-ground" complexity to justify this?

cc @rmosolgo

[eapache]

It's clearer in the example code, but worth specifying that the instance in all these cases is the schema object and not the backing implementation object.

[eapache]

I'm not sure I follow this bit about the lookahead extra.

[dylanahsmith]

For example, if we used a batch loaded field for image in the following GraphQL query,

        {
          product1: product(id: "1") { image { id } }
          product2: product(id: "2") { image { filename } }
        }

then it might be fine to load the image field for both products if no lookahead or irep_node is used, since it won't depend on the selection set ({ id } or { filename }).

However, if a lookahead were used (e.g. to avoid loading the image if only the id field is selected), then we don't want to load the image field for both products with a lookahead object that only represents { id } and not actually load the filename for the second product.

Does that make sense? If so, do you have any suggestions on how to clarify the documentation?

[eapache]

Ah, ya makes sense. I think I also got confused by the first sentence talking about "field selections" when it doesn't actually mean selection sets, but just sorta the field itself. I would say something like this:

For fields with arguments, instances of the field are only loaded together if they receive the same argument values. If the lookahead extra is used on the field, then instances of the field are only loaded together if they have the same [sub-field? or call it something else?] selection set.

[eapache]

the local iv_name is unused here?

[dylanahsmith]

However, I'm not sure what the long game is here?

Sorry, I should open a PR on shopify that shows the bigger picture.

Basically, this is just a primitive used for calling something in our non-promise based component query layer from something outside of it. The other part of this use case is the component query builder that will take lookahead as an argument (currently built on top of irep_node, since it looks like lookahead doesn't yet handle type conditional fragments or directives for us and seems like the use of selection from find_selections will be slow).

This will be similar to batch loaded fields on our query layer, which is the primitive that we use in reference_field to do high-level associations across parts of our query layer without coupling on the associated objects persistence layer. I was planning on having something similar to reference_field for our GraphQL layer as a convenient replacement for cross-component associations when no transformations are needed, or where they could be done using generic extensions (e.g. to support a first argument).

The more complex loads will still want custom loaders (...) and the simpler loads will still need some sort of sugar which just wraps the natural resolver in a load.

I'm trying to push this complex loading logic into the component query layer, so the GraphQL layer is mostly acting as an adapter. Unlike the last experiment, this will be built directly into the GraphQL objects derived from GraphQL::Schema::Object. For example, a field without a resolve method will just delegate to the component query layer with the method: option handling mis-match on naming. If a resolve method is provided, then it is like a derived field in our component query layer, where an object_deps: field option can specify dependencies on the underlying component query layer object. That object_deps: option could also be used for batch loaded fields with a component query layer as the backend object type, similar to how we have a preload: option when an active record model is used as the backend object type.

(because the field itself will need to transform the loaded data post-load and it doesn't seem possible to share the loading step while splitting the resolve step with this approach)

Are you referring to sharing the loaded data across fields? If so, then I believe we often use our preload: option for that shared data which we can continue to do while it is backed by an active record model and can replace with object_deps: when using a component query object for the backend object type.

If you aren't referring to sharing data across fields, then transformations could be done before setting the value in the value. For example, that could be done in the instances.each do |instance| loop in the example.

I'm not sure we have enough "middle-ground" complexity to justify this?

I was hoping this would be useful even outside of the component query layer. If it doesn't seem like this is the case, then this doesn't need to live in graphql-batch.

[eapache]

OK. I'd definitely like to see a more concrete internal example of what you think all of this will look like, but the explanation is heading in an interesting direction for sure.

[rmosolgo]

Thanks for the ping on this, I'm excited to see how it plays out! We've definitely littered the code with promises and suffered for the complexity of it, so I'm really interested in seeing alternatives.

[swalkinshaw]

Looks interesting! Implementation is fairly simple which is nice. I agree with Evan about some terminology like instances but can't really think of anything much better.