JetBrains / js-graphql-intellij-plugin

GraphQL language support for WebStorm, IntelliJ IDEA and other IDEs based on the IntelliJ Platform.
https://jimkyndemeyer.github.io/js-graphql-intellij-plugin/
MIT License
879 stars 97 forks source link

Add support for Apollo Federation schema specification. #499

Closed srinivasankavitha closed 2 years ago

srinivasankavitha commented 2 years ago

At Netflix, we have many federated graphs built using the Domain Graph Service Framework (DGS) to build GraphQL services. This is implemented according to Apollo's Federation Spec(https://www.apollographql.com/docs/federation/federation-spec/#federation-schema-specification). This PR adds support for the plugin to recognize federation specific schema types and directives.

This is part of a larger effort to improve the developer experience for GraphQL, similar to this PR(https://github.com/jimkyndemeyer/js-graphql-intellij-plugin/pull/498)

paulbakker commented 2 years ago

Note that we did discuss the option of just shipping these types/directives in JAR file using the new META-INF/schema feature. Because federation is used widely and isn't Java specific, we think it's better to have it built-in to the plugin.

vepanimas commented 2 years ago

Hi! LGTM. Could you also please add a new configuration option under Languages > GraphQL > Frameworks called Apollo Federation like it's done for com.intellij.lang.jsgraphql.GraphQLSettings#isRelaySupportEnabled? I think we have a lot of users who don't need it.

srinivasankavitha commented 2 years ago

Thanks for the review. Yes, will add that.

On Mon, Oct 25, 2021 at 7:16 PM Vladimir Panimaskin < @.***> wrote:

Hi! LGTM. Could you also please add a new configuration option under Languages

GraphQL > Frameworks called Apollo Federation like it's done for com.intellij.lang.jsgraphql.GraphQLSettings#isRelaySupportEnabled? I think we have a lot of users who don't need it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jimkyndemeyer/js-graphql-intellij-plugin/pull/499#issuecomment-951492196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ5JPXKZ4DBHDR2LM2WQMBLUIYFQTANCNFSM5GMRPL7A .

srinivasankavitha commented 2 years ago

Added the config option as suggested Screen Shot 2021-10-26 at 4 34 35 PM .

srinivasankavitha commented 2 years ago

I did not face any issues with it. Was trying to follow the same model as the relay types and added it there as a result.

On Fri, Nov 5, 2021 at 4:26 PM Vladimir Panimaskin @.***> wrote:

@.**** commented on this pull request.

In src/main/com/intellij/lang/jsgraphql/ide/references/GraphQLReferenceService.java https://github.com/jimkyndemeyer/js-graphql-intellij-plugin/pull/499#discussion_r744023644 :

@@ -230,6 +230,19 @@ public void visitElement(final @NotNull PsiElement schemaElement) { } }); }

  • if (name.startsWith("_")) {

One more thing, why did you add it here? AFAIU, those fields are added only on the root level, so it should work with the default reference resolution mechanism. Did you face any issues with it? Those fields are declared in the Query type extension, it should work out of the box:

extend type Query { _entities(representations: [_Any!]!): [_Entity]! _service: _Service! }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jimkyndemeyer/js-graphql-intellij-plugin/pull/499#pullrequestreview-799383337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ5JPXN27KPDXZHBEPW7RILUKRRZPANCNFSM5GMRPL7A .