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

Remove DgsNoOpPreparsedDocumentProvider #989

Merged
merged 1 commit into from Apr 18, 2022

Conversation

kilink
Copy link
Member

@kilink kilink commented Apr 16, 2022

Switch to the NoOpPreparsedDocumentProvider from graphql-java.

Switch to the NoOpPreparsedDocumentProvider from graphql-java.
@kilink kilink requested a review from berngp April 18, 2022 17:14
@berngp berngp merged commit bb793c2 into master Apr 18, 2022
@berngp berngp deleted the remove-dgs-no-op-preparsed-document-provider branch April 18, 2022 18:18
@jord1e
Copy link
Contributor

jord1e commented Apr 19, 2022

I implemented this in #599 because the GraphQL-Java one is annotated with @Internal, those classes are not public API and thus should never be used

https://github.com/graphql-java/graphql-java/blob/d783ec7ee30369ca2fdecdfd0b74e8bf6d96baa8/src/main/java/graphql/execution/preparsed/NoOpPreparsedDocumentProvider.java#L9

This PR reverts to using the internal non-public implementation. It should probably be reverted

@kilink
Copy link
Member Author

kilink commented Apr 19, 2022

Thanks for catching that, I didn't notice it was annotated with @Internal. However, I still think it's not necessary to duplicate the NoOpPreparsedDocumentProvider in DGS. All we have to do is not pass in anything to the builder returned by GraphQL.newGraphQL, and it ends up being the default one used anyhow. See #994

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.

None yet

3 participants