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

Fix directives building #2188

Closed
wants to merge 1 commit into from
Closed

Conversation

RoMiRoSSaN
Copy link
Contributor

See #2154

In wundergraph cosmo @link is optional.
Apollo requires it. Quarkus does not pass a complete index containing all directives, resulting in the @linkand some others being ignored.

@RoMiRoSSaN
Copy link
Contributor Author

Fell due to the fact that the Deprecated and OneOf directives were added. You can remove them, or simply remove 2 checks in SchemaBuilderTest testConcurrentSchemaBuilding

@jmartisk
Copy link
Member

jmartisk commented Sep 16, 2024

@RoMiRoSSaN, this change should be done on the Quarkus side, see https://github.com/quarkusio/quarkus/blob/3.15.0.CR1/extensions/smallrye-graphql/deployment/src/main/java/io/quarkus/smallrye/graphql/deployment/SmallRyeGraphQLProcessor.java#L286 - here we add any additional classes that need to be indexed.

The Quarkus integration handles the indexing, we don't do any indexing on the SmallRye side when using Quarkus.
When using WildFly, we do this in https://github.com/smallrye/smallrye-graphql/blob/main/server/implementation-servlet/src/main/java/io/smallrye/graphql/entry/http/IndexInitializer.java#L98

so I think the best solution would be to

@RoMiRoSSaN
Copy link
Contributor Author

OK.
All directives have been added for WildFly.
I will do PR for quarkus.
Just a question. Is this really how it should be? After all, this is necessary for the correct construction of a graphql schema. And it turns out that this code is duplicated in two different places. Although partially the schema builder should focus on its own api (annotations, directives, etc.), and not wait for information about it from the outside. Or did I understand something wrong?

@jmartisk
Copy link
Member

Quarkus works in the way that it automatically computes an index and makes it available to extensions, so SmallRye just takes the index produced by Quarkus. As for the duplication, yes we could potentially unify it into a single place (the Quarkus processor would call something inside SmallRye GraphQL) but it hasn't been considered enough of a priority :)

@RoMiRoSSaN
Copy link
Contributor Author

Okay, I get it. Then I'm closing this PR.
And I’ll open it in Quarkus

@RoMiRoSSaN RoMiRoSSaN closed this Sep 16, 2024
@RoMiRoSSaN RoMiRoSSaN deleted the apollo-router branch September 25, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants