Introduce computed fields

[gmac]

As discussed in #1959, this sets up system where MergedTypeConfig fields options may be flagged as computed. When a field is marked as computed, it is isolated to a dedicated subschema for computed fields (if necessary) that will always be targeted via the gateway so dependencies are provided.

{
  Product: {
    selectionSet: '{ id }',
    fields: {
      shippingEstimate: { selectionSet: '{ price weight }', computed: true },
      deliveryService: { selectionSet: '{ weight }', computed: true },
    },
    fieldName: '_products',
    key: ({ id, price, weight }) => ({ id, price, weight }),
    argsFromKeys: (representations) => ({ representations }),
  }
}

All told, this is a no-op unless a merged type specifically enables one or more fields marked as computed: true. In the case of computed fields intermixed with static fields, it performs the schema isolation approach described in #1959 (comment).

Changes

• Adds isolateComputedMergeSchemas that will look through a SubschemaConfig array and split apart subschema configs that contain both static and computed fields. No change is made to types that are already entirely static or entirely computed.

• Schema splitting it setup to shift computed fields and their interface memberships over to a dedicated schema. The static schema will lose any computed interface fields even if they're used by other types, however will get them back by virtue of interface merging (✨).

• Expands filterSchema to be more robust in its capabilities. Now supports filtering object and interface fields, with better naming on the object field filter to reflect that (the old object field filter name is left for backwards compatibility). Using one filter pass seems better than running individual transform passes for each concern, plus it appears that the FilterInterfaceFields transform is untested and presently broken.

TODO:

• If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[yaacovCR]

See discussion in #1959

I think it would be worthwhile to work on this, in the sense that I think that we should export some helpers that allow end-users -- such as yourself -- to easily create custom functions that take in a SubschemaConfig object and return a new object or an array of new such objects, based on directives within the SubschemaConfig objects target schema.

These helpers, if they return an array of subschema config objects, should move the endpoint data into a shared endpoint object using the endpoint property to preserve query batching.

[gmac]

I've updated the shape of this to integrate through a simple "plugins" interface. Using plugins would allow stitching to be preconfigured in interesting ways – like setting up computed fields, setting up a merged description behavior, etc. In the future, I could imagine other hooks being added that could allow a plugin to tap into other areas of stitching.

See https://github.com/ardatan/graphql-tools/pull/1989/files#diff-9714caf463c56574ed31e1c31f89562bR310-R340

[gmac]

Okay, this has been refactored back into a helper method (open to suggestions on the name). I've also got it (optionally) reading required fields directly from deprecations in the SDL, which it handles un-deprecating within the gateway schema. Here's a full-blown example built using the federation entities service pattern: https://github.com/ardatan/graphql-tools/pull/1989/files#diff-358771a00c79c2ee3c35bc7023e05632R143-R202.

Another interesting thought would be to somehow leverage the native @includes and/or @skip directives. As part of the core spec, they've got good universal language support (that's my main complaint about custom directives). They also do mostly what computed fields would want within the subschema, although I can't think of a good way to reliably set the boolean argument. Do you have experience with them, by chance?

[gmac]

As for invoking this helper on subschema config... I still don't think it would be terrible to run as part of stitchSchemas, or at the very least based on a computedFields: true option. If they were interested in targeting this schema via manual delegation, they'd just have to call it manually on the subschema config first. I believe multiple invocations on a subschema config should be idempotent once the first execution normalizes everything.

[yaacovCR]

ok, done with my docs pass-through for now. @gmac, i am ready to merge into master. what do you think?

[yaacovCR]

My planned description for this change when merging:

• Introduces a federate attribute on fields within the type merging configuration. Setting federate to true will cause the gateway's query planner to always to retrieve that field's dependencies from other services, even if the query originates from that subservice. This allows type merging patterns in which a given service may have to be visited twice.

• Introduces the requires directive that allows schemas to be annotated with field-level required selectionSets (or field sets), as well as whether or not the required fields should be federated.

[gmac]

Well, that turned into a humblingly large update. Was the query planner fundamentally changed? It appears to still be using the isolation approach.

Thanks for all your work on this... hope I didn’t coerce you into it.

[gmac]

Merge description sounds good! I’ve got a few tweaks to make in the docs in response to the final field name that I’ll try to add in tonight.

[gmac]

@yaacovCR – would you be offended if I made an editing pass on this new section here? https://github.com/ardatan/graphql-tools/pull/1989/files#diff-112839495c94df82bbbb7843b4e71b31R435-R439

I don't disagree with the knowledge, though the explanation is extremely dense and I have a hard time following it (knowing how it works). There may be some ways to distill it down into smaller nuggets. Doesn't have to be a blocker if you wouldn't mind a followup PR.

[gmac]

Also, a final mention about the SDL design: personally, I still find the @requires(..., federate: true/false) design to be pretty confusing. It's technical rather than expressive. I'd reiterate the suggestion of making them two directives:

• @requires(selectionSet: ...): the field requires this selection as a hard dependency, therefore the gateway must always fulfill it (federate true).
• @sends(selectionSet: ...): says the gateway sends additional selections when requesting this field directly. These selections may not be present when resolved locally within the subservice (federate false)

[gmac]

Thinking more on the SDL design, I guess another thing that gets me is how it intersects with conventional behavior. @requires with federate true is a pretty direct analog of the established Federation spec. Versus, @requires with federate false has no real analog. I think it’d be a lot more obvious if @requires was left as a direct analog to the established spec, and the unique behavior was a separate directive that fell outside of the spec.

Also for learning, getting rolling with a clear understanding of what @requires does based on convention seems more productive to a general audience than to try to get up and running with a foggy understanding of how @requires may or may not achieve their goals. I’d again reiterate that federation’s standard hard dependency pattern seems like the majority use case here. The soft dependency pattern is really subtle and therefore a fairly advanced use case... this sort of mirrors how I am with the federation @provides directive right now: I generally get what it does though I can’t really picture how I’d use it. However, it’s the kind of thing that I’d put on the mental back burner until I had a situation arise where I say, “OH! THAT is what @provides is for...”. I think separate directives would really help with that separation of the immediate versus the discovered need.

[gmac]

Another name that would make sense here is @requests(selectionSet:...), as in a field may request additional data be sent from the gateway (when possible) versus a field that requires data to be sent from the gateway. The “requests” name is nice in that the directionality of a field requesting gateway data matches that of a field requiring gateway data.

[yaacovCR]

I recognize that this is a point you have made before, but I also cannot say I have ever fully understood it...

from the gateway perspective, all selection sets are required in order to properly merge fields from a given subschema -- whether they are type selection sets or field selection sets, and if they are field selection sets, whether or not the isolation approaches is used.

So I'm not quite sure I understand the distinction between required/requested selection sets.

What federate = true means is that you can construct a schema that won't work on its own, but the gateway will cleave it into two using the isolation method on the federated fields.

This means that resolvers that without isolation might be required to return metadata about a type (price and weight for the Product within the tests), no longer need to do so, moreover, even if they do so, the data will be ignored, as the fields will only be resolved later and the gateway will fetch price and weight from the other service.

I don't believe this change in behavior is fully captured by the word required or requested. It's not an implementation detail to say that subschema when queried on its own will no longer be the same as when queried with the gateway, or at the very least, in my mind, it's not a tiny implementation detail...

I am ok with using the directive name federate instead of required, but I think you seem to want to match the syntax of Apollo Federation, so this is the best I could come up with...

One weakness I think is when a type only has federated fields, you actually don't need to use the isolation method, so federate can be set to false or true and you would get the same behavior. but that doesn't bother me so much as using required only for federated fields, when they are required for merging even non-federated fields...

As I said, I think you have been making this point for a while and I have not been understanding it for a while, so if there's something I'm still not understanding please feel free to correct.

My guess is you are talking not from the gateway perspective but from the subschema perspective. When viewed from the perspective of a subschema, I could see how only federated fields are required, as they supplement metadata that the resolver is not supplying. But from the subschema perspective, the gateway doesn't exist -- so it's difficult to describe required fields when federate is set to false... Or required selection set for a type, or the rest of the merge configuration.

I think our directive syntax should be as close as possible to our merge config syntax. I haven't had a chance to read through your whole proposal for merge conflict syntax, but one small point here is that the required selection set for a type probably should also use the required directive with similar syntax as possible to fields...

[gmac]

We come at this problem from fascinatingly different approaches. You speak from the standpoint of the technical implementation. I speak from the standpoint of the developer experience. Your methodologies speak to configuring what’s happening under the hood. Mine speak to configuring what the developer is trying to achieve as they would say it, regardless of how it works. Which is correct? I think at the end of the day — some of both. That said, I’ve made my case. Not a lot more to say on my end. The question still stands though: would you be offended if I take an editing pass on the new docs? The information is all there, but it’s extremely dense. If that can be distilled down a bit, I think the feature could be a lot more approachable.

[gmac]

To the point of a “federate” directive, it’s not terrible assuming that the “federation” terminology is past the point of being fungible (“computed” fields made more sense to me). Matching Apollo federation isn’t absolutely necessary though seems advantageous. I guess the main thing that is weird to me is the “federate: true/false” behavior, mainly because “federate:false” is such a remote use case. I could even see that being left out of SDL config entirely. BUT — if the final answer is that the SDL needs to match internal operations over common development parlance, then yes, the thing that makes the most sense is @federate(selectionSet:..., isolate: true/false). I don’t think it’s very dev friendly, but it is accurate to boot and they’ll be forced to learn something about library internals which is at least something. It’s just the kind of syntax that devs complain about because it’s written to library internals rather than development goals.

Lol... i imagine would probably hate Ruby on Rails. Every darn thing in that library is named for the development goal rather than the internal implementation. ;-)

[yaacovCR]

Oh, no offense at all... I would love if you too a pass, thank you!

I think it makes sense for me to look more closely at your entire sdl RFC and make the sdl here jell with that...

[yaacovCR]

Decided actually to leave federate / isolate false for the remainder of the SDL RFC and to just include a single directive to cover the federate / isolate true scenario.

Within the merge config, I made a new sub-node for computed fields. We may eventually phase out the field selection sets for non-computed fields. Even though computed fields also have field selection sets, the use case and implementation is different enough to justify just splitting it into two directives and two different nodes in the merge config

I also added the notion of sub schema config transforms that can change the subschema config based on directive or whatever. This is done without modifying the original config object via helper cloneSubschemaConfig function. I added an array of default transformers if none specified which includes just the computed field transformer with the directive name set as computed. The directive name can now be changed to required or federate or whatever.

At some point we may add support for plugins, but the many options we do support should allow for a thin wrapping layer or a declarative wrapper like graphql-mesh, or even just a wrapping layer that adds plugin support.

Let me know what you think!

I am again ready to merge at this point. :)

[yaacovCR]

My new planned description for this change when merging:

• Introduces a computeFields attribute within the type merging configuration. Using computed field selection sets instead of regular field selection sets will cause the gateway's query planner to always to retrieve that field's dependencies from other services, even if the query originates from that subservice. This allows type merging patterns in which a given service may have to be visited twice.

• Introduces subschemaConfigTransforms that can transform configs, including a default transform that uses the computed directive to add computed fields and annotate them with field-level required selectionSets (or field sets).

[gmac]

This looks pretty good! I would be a little sad to lose the basic fields feature entirely in the future (it’s still useful for performance tuning), so I would still make a case for the “fields: ... { computed: true }” pattern. Just having the one @computed SDL directive makes sense though. Removing the term “federate” simplifies the rest of the implementation a lot.

That’s a relatively minor note that my feels are only a 3/10 on. Thanks for all the hard work here.

[gmac]

FWIW — something I may never have contextualized for you on this point involves that “tolerance” note. We actually never use subschemas directly. Our apps are on an internal network, and the gateway service signs auth headers so that requests that didn’t come through the gateway are rejected outright. We do this because the gateway is where all caching, rate-limiting, and query budgeting is performed. The only time subservices are used directly are for devs running the apps locally in development mode. I believe our setup is pretty common, and follows a lot of the thinking in the Apollo data graph mindset.

[gmac]

Yeah, this makes sense. It’s not super glamorous, but it’s appropriately unopinionated. Using the term “federate” felt a bit loaded.

[gmac]

This feels okay, although it would be a little sad if the advanced use case were to be eliminated entirely in the future by pulling out “fields”. It can still be useful for performance tuning. I still like the idea of “computed: true” on the base “fields” namespace so that there’s only one “fields” feature to document, and fields can have documented modifiers. With the separate namespace, fields becomes an undocumented feature. That said, I don’t think the SDL needs anything but the computed directive at this time.

[yaacovCR]

We don't have to deprecate the plain old field selection set hints, and we can add more docs and more directives. This seems like the minimal feature set that would provide best foundation forward. :)

[theguild-bot]

The latest changes of this PR are available as alpha in npm: 6.2.3-alpha-c20d6866.0

Quickly update your package.json by running:

npx match-version @graphql-tools 6.2.3-alpha-c20d6866.0

[gmac]

🎉