Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support alternative to @boundary directive that is introspection-compatible. #79

Open
nmaquet opened this issue Oct 25, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@nmaquet
Copy link
Contributor

nmaquet commented Oct 25, 2021

Bramble relies on a custom directive called @boundary to determine which object types are shared across services and how to look up boundary objects by ID in queries. This was inspired in large part by Apollo Federation's syntax which relies on 4 custom directives for a similar purpose (see https://www.apollographql.com/docs/federation/federation-spec/#federation-schema-specification).

The issues we have faced with this approach are substantial and are due the fact that directives are only part of the GraphQL IDL and not the GraphQL type system (as explained by Lee Byron in graphql/graphql-js#746 (comment)). This means that any tool that doesn't use the GraphQL IDL (e.g. any code-first implementation like graphql-js or Graphene) is de facto incompatible with Bramble.

In addition, even for systems that do support directives and can specify @boundary on objects and fields, we've had to rely on services communicating the IDL as a string through the Service.schema field (Apollo Federation does a similar thing with _Service.sdl). This is a hack and makes life hard both for the services and Bramble itself.

The alternative we should consider for 2.0 is extending Bramble's Service object type to expose fields describing which object types are boundary types and which root fields should be used to look them up.

This change can be made in a backwards-compatible way. We should support both the directive and its alternative going forward.

@nmaquet nmaquet added the enhancement New feature or request label Oct 25, 2021
@nmaquet nmaquet added this to the Bramble 2.0 milestone Oct 25, 2021
@gmac
Copy link
Contributor

gmac commented Oct 29, 2021

I would argue that the identity of boundary types are implicit: if a typename appears in multiple services, then it is a boundary, and all boundaries require an ID field. However, all boundaries DO NOT require an accessor, consider a foreign key type:

type Product {
  id: ID!
}

This type IS a boundary if it intersects with other types in other services with the same name, and may merge with their data. However, it does NOT require an accessor given that the local service originates the id field and it has no other unique data. Thus, we know this type in this service will never require an inbound request to fetch it. Requiring an accessor that simply parrots an ID for the sake of library operations is a bit tedious.

All that said, having service dictate a list of required accessor fields for the service seems okay.

@nmaquet nmaquet removed this from the Bramble 2.0 milestone Nov 7, 2021
@nmaquet nmaquet changed the title Drop @boundary directive in favour of an introspection-compatible alternative Support alternative to @boundary directive that is introspection-compatible. Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants