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

feat: upgrade to graphql@15.0.0 for #1191 #1204

Merged
merged 7 commits into from
Apr 5, 2020
Merged

feat: upgrade to graphql@15.0.0 for #1191 #1204

merged 7 commits into from
Apr 5, 2020

Conversation

acao
Copy link
Member

@acao acao commented Jan 8, 2020

Have graphql@15.0.0 working as a baseline, without necessarily adding support for new language features yet. Consequently, it requires an upgrade to graphql-config-rc.2. This is why we are still in alpha! Further PRs will follow that address things like interfaces implementing interfaces, etc

  • use instrospectionQuery() rather than constant (deprecated)
  • bump resolutions for 15
  • minor validation error formating issues
  • ensure GraphiQL works end to end (view deploy preview, cypress suite)
  • issue with graphql-config and server tests? potentially graphql-config is not working with 15.
  • check server for regressions, use with a vscode extension
  • manually check for more specific regressions in GraphiQL, other LSP implementations that are related to breaking changes.

@acao acao changed the title feat: upgrade to graphql@15.0.0-rc.1 feat: upgrade to graphql@15.0.0-rc.1 for #1191 Jan 8, 2020
@acao acao changed the title feat: upgrade to graphql@15.0.0-rc.1 for #1191 [WIP] feat: upgrade to graphql@15.0.0-rc.1 for #1191 Jan 8, 2020
@acao
Copy link
Member Author

acao commented Jan 8, 2020

@Urigo so we are using graphql-config@2.2.1 still, and it seems theres a bug with the introspection result in this test:

https://github.com/graphql/graphiql/blob/chore/graphql-15-rc/packages/graphql-language-service-server/src/__tests__/GraphQLCache-test.js#L46

when i print out the result of the introspection request, i get:
Error: Must provide Source. Received: undefined.

@ardatan ardatan mentioned this pull request Jan 14, 2020
2 tasks
@acao acao mentioned this pull request Feb 16, 2020
2 tasks
@acao
Copy link
Member Author

acao commented Feb 16, 2020

only bug left is that this type is deprecated in graphql 15 (though we can work around this):
image

@ardatan @Urigo

maybe i can open an issue?

@acao
Copy link
Member Author

acao commented Feb 16, 2020

ardatan/graphql-tools#1 i think this is the right place?

@Urigo
Copy link
Collaborator

Urigo commented Feb 16, 2020

I think this was already fixed on graphql-toolkit's master.

tomorrow we'll make sure to have updated versions of it published

@acao acao changed the title [WIP] feat: upgrade to graphql@15.0.0-rc.1 for #1191 [WIP] feat: upgrade to graphql@15.0.0-rc.2 for #1191 Feb 25, 2020
@acao acao changed the title [WIP] feat: upgrade to graphql@15.0.0-rc.2 for #1191 [WIP] feat: upgrade to graphql@15.0.0 for #1191 Apr 4, 2020
@acao acao changed the title [WIP] feat: upgrade to graphql@15.0.0 for #1191 feat: upgrade to graphql@15.0.0 for #1191 Apr 4, 2020
@acao
Copy link
Member Author

acao commented Apr 4, 2020

@Urigo everything looks good now, thank you!

@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #1204 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1204   +/-   ##
=======================================
  Coverage   58.98%   58.98%           
=======================================
  Files          60       60           
  Lines        3023     3023           
  Branches      806      806           
=======================================
  Hits         1783     1783           
  Misses       1194     1194           
  Partials       46       46           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fdab8c...9f5782d. Read the comment docs.

@acao
Copy link
Member Author

acao commented Apr 4, 2020

@IvanGoncharov it's finally ready for you to review, what do you think?

@acao acao merged commit f13c8e9 into master Apr 5, 2020
@github-actions github-actions bot deleted the chore/graphql-15-rc branch April 5, 2020 23:23
@acao
Copy link
Member Author

acao commented Apr 6, 2020

@IvanGoncharov I think the only thing we missed here as a baseline is removing the peer dependencies on lower versions of graphql. We should only support 14 and 15 now correct?

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

4 participants