search

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

Do not report "redefine existing type" errors for standard types #665

Closed BoD closed 11 months ago

BoD commented 1 year ago

When a schema defines standard (aka built-in) type or directive, such as Boolean, __Field, @include, it is reported with an error 'Xyz' type tried to redefine existing directive 'Xyz' type.

However, some client libraries or framework may expect a "full schema" to be present, i.e. a schema that does explicitly contain all these types/directives exhaustively.

In that context, in the Apollo Kotlin library, we recently introduced a change so that these types/directives are no longer filtered out when downloading a schema from introspection and saving it as a SDL (.graphqls) schema.

To Reproduce Have a schema that defines a built-in type, for example:

scalar Boolean

It will appear underlined in red with the mentioned error.

Expected behavior A special case should be made to not report these errors for built-in types.

Version and Environment Details Plugin version: 4.0.0

Additional context I would be interested in trying to open a PR myself for this, if you think it is a good idea.

vepanimas commented 1 year ago

Hi! It would be great to support this. The easiest way is to suppress that kind of error using com.intellij.lang.jsgraphql.ide.validation.GraphQLErrorFilter, e.g. com.intellij.lang.jsgraphql.frameworks.apollo.GraphQLApolloErrorFilter. However, I believe that this is insufficient, and that we should implement a more sophisticated approach to prioritize user-defined types during schema building. The issue here is that the schema build process is currently somewhat hacky, and it would not be simple to adapt it.

martinbonnin commented 11 months ago

I just found out (thanks to @sonatard) that built in scalars must not be present in the SDL so the initial scalar Boolean example doesn't hold.

The problem might happen one day if a new scalar is added to GraphQL but we can certainly leave that aside for now.

That being said, this issue is still valid for other definitions like directives:

directive @deprecated(
   reason: String = "No longer supported"
 ) on FIELD_DEFINITION | ENUM_VALUE
vepanimas commented 11 months ago

I changed (e87fbf9a0080a412b7eb4ce8a9e4782c5b773b4f) the plugin schema building process and now the built-in types will be used only if no user-defined types with the same name are provided.