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.

Sign up for GitHub

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

Jump to bottom

Possible Bug: Investigate if Weaviate is at fault for fields wrongly marked as deprecated in GraphiQL #1412

Closed
etiennedi opened this issue Jan 13, 2021 · 0 comments · Fixed by #1413
Closed

Possible Bug: Investigate if Weaviate is at fault for fields wrongly marked as deprecated in GraphiQL #1412

etiennedi opened this issue Jan 13, 2021 · 0 comments · Fixed by #1413

Comments

@etiennedi
Copy link
Member

The latest GraphiQL version used in the upcoming console shows all fields as deprecated, there is a chance that this is caused by Weaviate itself. Investigate and potentially fix.
etiennedi added a commit that referenced this issue Jan 13, 2021
@etiennedi
gh-1412 update graphql package
6004dcc 
So, this seems to boil down to the following:

- There is an old bug in graphql-js (fixed in 2017) where the package
  ignores the isDeprecated field, but instead uses
  deprecationReason!=null to check whether a field is deprecated. This
  bug has been fixed a long time ago: graphql/graphql-js#1035
- However, it seems various other dependencies have not updated to
  include the fix, see graphql-go/graphql#504 (comment)
- The graphql-go package has been (correctly) using an empty string (as
  is the default value for string) in Go if no deprecation reason is
  set. As "isDeprecated" this should not ever have mattered
- Since the bug from bullet one still seems to be present in many
  packages - as we have seen with the latest GraphiQL reason - the
  graphql-go team have now instead added a new resolver which explicitly
  sets the "deprecationReason" to null (rather than "") if not set. This
  was released in v0.7.9 which this commit udpates to
- This fixes our issue, the fields are no longer shown as deprecated in
  GraphiQL :)
@etiennedi etiennedi mentioned this issue Jan 13, 2021
Merged
antas-marcin added a commit that referenced this issue Jan 13, 2021
@antas-marcin
Merge pull request #1413 from semi-technologies/gh-1412-fix-deprecati…
b969747 
…on-message

gh-1412 update graphql package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-1412 update graphql package weaviate/weaviate
1 participant
@etiennedi