Skip to content
New issue

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

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

Already on GitHub? # to your account

Apollo Federation, @extends directive and input types #678

Closed
maurocolella opened this issue Aug 7, 2020 · 9 comments
Closed

Apollo Federation, @extends directive and input types #678

maurocolella opened this issue Aug 7, 2020 · 9 comments
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved

Comments

@maurocolella
Copy link

maurocolella commented Aug 7, 2020

Describe the issue
We are using type-graphql with Apollo Federation. As per Apollo Federation guidelines, we try to keep code that relies on a service contained with that service.

So, say I have type A with a field that resolves to type B, I am going to use a @FieldResolver on type B, and return based on an entity stub with type A.

In order to do this, my stub uses @extends and @external directives. As in:

@Directive('@extends')
@Directive('@key(fields: "id")')
@ObjectType()
export class Member {
  @Directive('@external')
  @Field(type => ID)
  id: string;

This works great for object types.

The graphql spec, as far as I understand, allows basic type extensions for input types. But it's not clear to me whether Apollo Federation supports using @extends and @external to achieve this type of thing with input types.

Is it at all possible? If yes, is it possible using type-graphql? If not, does it make sense and is it on any roadmap?

@MichalLytek
Copy link
Owner

The graphql spec, as far as I understand, allows basic type extensions for input types.

Is it at all possible? If yes, is it possible using type-graphql? If not, does it make sense and is it on any roadmap?

Do you think about #228?

@MichalLytek MichalLytek added the Question ❔ Not future request, proposal or bug issue label Aug 7, 2020
@maurocolella
Copy link
Author

Something like that but with directives. In the example I gave, "locations" have a a field called "member".

The location service/resolvers know a member stub; a simplified object definition that extends member through directives.

Any other service that has a field with "member" type has a similar stub.

If the member service is available, at runtime, Apollo Federation reconciles the schema to a "full" definition of member, and exposes all its fields. Any services that uses members will be able to rely on the full object type.

But if the member service is down, the stubs are used instead. They act as placeholders, and only contain an id.

In this way, each individual logical "service" has no need to know the implementation details of any other service.

Now, I'd love to be able to do this with input types as well.

@MichalLytek
Copy link
Owner

Something like that but with directives.
Now, I'd love to be able to do this with input types as well.

So as TypeGraphQL can emit directives for input types, you can write your own directives and a proper schema directive visitor that will do the job for you.

It's not a job for TypeGraphQL to apply some logic (like connecting to federation service) when some of the type contains a selected directive.

@maurocolella
Copy link
Author

maurocolella commented Aug 7, 2020 via email

@MichalLytek MichalLytek added the Solved ✔️ The issue has been solved label Aug 12, 2020
@maurocolella
Copy link
Author

maurocolella commented Aug 20, 2020

I think I might not have done a great job of explaining the initial issue. I have now traced the root cause to the underlying "graphql" library. I just want to document the steps I have taken and clarify the context.

Intrinsically, apollo-server doesn't support the @extends directive for input types at this time. It only applies to object types and interfaces.
https://github.com/apollographql/apollo-server/blob/570f548b88750a06fbf5f67a4abe78fb0f870ccd/packages/apollo-federation/src/directives.ts#L36

Now, with apollo-federation, the recommended practice is to insulate services, so that a service that needs to use a "foreign" object (example: a Member that needs to use a Location when the implementing services for locations is its own) only refers to a locally-declared stub object.

A fake, minimal object that extends the foreign one.

This is all well. Running through the code for type-graphql, if I override directive declarations, my new declarations get run through:

directives: options.directives,

And that is where it hits a current bottleneck of graphql, where the spec says that this "should" be possible,
the code has recently passed a milestone where it is possible (to extend input types), but directives might not currently reflect this.
graphql/graphql-js#1095

Are you saying that, in theory, I could write a custom directive and visitor (https://www.apollographql.com/docs/apollo-server/schema/creating-directives/) and override this behavior? Ie. translate my directives into a syntax currently supported by graphql?

There seems to be a bit of a blur around using buildSchema vs extendSchema for that.
graphql/graphql-js#922

@MichalLytek
Copy link
Owner

TypeGraphQL support for extends keyword is included in the #228 issue.

@extends directive is a specific for Apollo Federation and looks like it does not support that behavior for input types, based on your reports.

You need to wait for the #228 or ask Apollo team to support @extends directive is the use cases you want.

@maurocolella
Copy link
Author

Interesting, thank you. We are actually using @Directive('@extends') as of now, and it works for ObjectType (with type-graphql).

I did request extended support for this to the Apollo team.

On the side of graphql, spec and reference implementation for node-js, the latest status is:

ATM, buildSchema silently ignore extensions and you need to use extendSchema for that.
This issue is about adding support for extensions in buildSchema.

And this is to extend not only input types, but pretty much everything else where this extension mechanism makes sense.

We'll be happy to wait. But with #228 it seems to make sense, based on the spec, to consider input types as well.

@tim-trisnadhama
Copy link

Hi, i m facing the same issue in regards of extending the input types from other subgraphs, is there a solution to this?

@abdurhasan
Copy link

hmmm facing some problem 😆 here ...
I just renaming the input type accros the services🤣

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved
Projects
None yet
Development

No branches or pull requests

4 participants