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

refactor (graphql.js): Upgrade to the v2 of apollo #18

Merged
merged 1 commit into from Aug 26, 2018
Merged

refactor (graphql.js): Upgrade to the v2 of apollo #18

merged 1 commit into from Aug 26, 2018

Conversation

ghost
Copy link

@ghost ghost commented Aug 25, 2018

We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the middle-wares, but now they have been either removed or hidden from the public. And since v2 of apollo we've faced with a BREAKING CHANGE (See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800).

In order to be compatible with the older codes users are referring,
we have to do some tricks:

  1. Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo' instead
    of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

  2. 'apollo-server-module-graphiql' is removed, so in order to compatible with
    what it was, we have to add this in our package and reuse that. And then we should
    rewrite 'graphiqlKoa' function as the middle-ware copied from: https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to apollo's
version that is less than 2.x. And there're many ambigious problems about this
design of v2 for apollo (See: apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions would be re-added
in the future (See: apollographql/apollo-server#1282 (comment)).

  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Ref: eggjs/egg#2862

@codecov
Copy link

codecov bot commented Aug 25, 2018

Codecov Report

Merging #18 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #18   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines          94    100    +6     
=====================================
+ Hits           94    100    +6
Impacted Files Coverage Δ
app/middleware/graphql.js 100% <100%> (ø) ⬆️

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 0248cb2...3cb249b. Read the comment docs.

We used to get the 'graphqlKoa' and 'graphiqlKoa' before as the
middle-wares,
but now they have been either removed or hidden from the public. And
since v2
of apollo we've faced with a BREAKING CHANGE
(See:apollographql/apollo-server@dbaa465#diff-64af8fdf76996fa3ed4e498d44124800
).
In order to be compatible with the older codes users are referring,
we have to do some tricks:

1) Directly refer 'graphqlKoa' from 'apollo-server-koa/dist/koaApollo'
instead
of 'apollo-server-koa', because it's NOT exposed through 'index.js'.

2) 'apollo-server-module-graphiql' is removed, so in order to compatible
with
what it was, we have to add this in our package and reuse that. And then
we should
rewrite 'graphiqlKoa' function as the middle-ware copied from:
https://github.com/apollographql/apollo-server/blob/300c0cd12b56be439b206d55131e1b93a9e6dade/packages/apollo-server-koa/src/koaApollo.ts#L51

Notice: This change is also a BREAKING one, so please DO NOT apply to
apollo's
version that is less than 2.x. And there're many ambigious problems
about this
design of v2 for apollo (See:
apollographql/apollo-server#1308).
So according to chentsulin's ideas, maybe the middle-ware functions
would be re-added
in the future (See:
apollographql/apollo-server#1282 (comment)).
@ghost
Copy link
Author

ghost commented Aug 25, 2018

/cc:@atian25

@jtyjty99999 jtyjty99999 merged commit 461edf0 into eggjs:master Aug 26, 2018
@ghost ghost deleted the UpToApollo_V2 branch August 27, 2018 00:33
@ghost
Copy link
Author

ghost commented Aug 27, 2018

Thanks!

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.

1 participant