-
Notifications
You must be signed in to change notification settings - Fork 2.3k
build: replace express-graphql with graphql-http and graphql-playgrou… #1521
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
build: replace express-graphql with graphql-http and graphql-playgrou… #1521
Conversation
…nd-middleware-express toward resolution of issue cypress-io#1420 and cypress-io#1508
|
with graphql-playground-middleware-express, rather than serving it from GET /gql-playground. This preserves functionality closer to as it was with express-graphql.
createBankAccount: (obj: any, args: any, ctx: any) => { | ||
const account = createBankAccountForUser(ctx.user.id!, args); | ||
return account; | ||
createBankAccount: (obj: any, args: any, ctx: { user: { id: string } }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the initiative here, but it might be better to define a shared ResolverContext
type, e.g.
type ResolverContext = {
user?: Express.User
}
that can be imported into the resolvers, and also passed as a context template type to grapql-http
's createHandler
function - which will ensure that the context()
property has the correct return type
@timheilman Will you have time to address the feedback from review? |
Co-authored-by: Cacie Prins <[email protected]>
…ess-graphql-to-graphql-http
Hi there! Sorry for the delay. When I originally opened this PR, I had lots of time on my hands, but now I'm back to working. I merged the first suggested change, and resolved the second by reverting to the |
…efox all specs passing on firefox locally.
cypress-io#1521) * build: replace express-graphql with graphql-http and graphql-playground-middleware-express toward resolution of issue cypress-io#1420 and cypress-io#1508 * fix: go ahead and preempt provision of GET at /graphql by graphql-http with graphql-playground-middleware-express, rather than serving it from GET /gql-playground. This preserves functionality closer to as it was with express-graphql. * fix: whoops did not mean to modify database.json * Update backend/app.ts Co-authored-by: Cacie Prins <[email protected]> * style: do not pollute type system with one-off inline types * fix: consume rename commited on github from reviewer suggestion * style: do not pollute Query.ts either with one-off inline types * build: empty commit to clear perhaps spurious circleCI failure on firefox all specs passing on firefox locally. --------- Co-authored-by: Cacie Prins <[email protected]>
…nd-middleware-express
toward resolution of issue #1420 and #1508