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

Use GraphQL Codegen to generate more accurate resolver types #5216

Merged
merged 8 commits into from Apr 28, 2022

Conversation

orta
Copy link
Contributor

@orta orta commented Apr 17, 2022

Alternative to #4786

This method tweaks GraphQL codegen to get accurate types, instead of bending the type system to fit (e.g. #4786), this work certainly makes me yearn for relay-compiler style generation of resolver types where it creates the exact types you need instead of a multi-thousand line file where we can pick useful bits out.

Anyway, this seems to work pretty well in strict and unstrict environments. It handles the date/string mismatch by making Prisma models be the default expectation of an resolver. This option has its downsides (I have a few resolvers which return dumb js objects in some places, but that's a rarity.)

Here's an example of a file using this setup:

import cuid from "cuid"
import { dnull } from "dnull"

import { db } from "src/lib/db"
import { MutationResolvers, QueryResolvers } from "types/graphql"

export const users = () => {
  return db.user.findMany()
}

export const user: QueryResolvers["user"] = ({ id }) => {
  return db.user.findUnique({
    where: { id },
  })
}

export const createUser: MutationResolvers["createUser"] = ({ input }) => {
  return db.user.create({
    data: dnull({
      id: cuid(),
      ...input
    }),
  })
}

export const updateUser: MutationResolvers["updateUser"] = ({ id, input }) => {
  return db.user.update({
    data: dnull(input),
    where: { id },
  })
}

export const deleteUser: MutationResolvers["updateUser"] = ({ id }) => {
  return db.user.delete({
    where: { id },
  })
}

export const createNewUserOnAccount: MutationResolvers["createNewUserOnAccount"] = async (args) => {
  if (!context.currentUser) return null

  const username = await canUseHandle(args.input.username)
  if (!username) {
    return null
  }

  return db.user.create({
    data: {
      id: cuid() + ":user",
      username,
      roles: context.currentUser.roles,
      account: {
        connect: context.currentUser.account,
      },
    },
  })
}

( dnull is essential to working with prisma/graphql to handle this issue and avoid data loss, it might be worth thinking about integrating it properly in some way )

@netlify
Copy link

netlify bot commented Apr 17, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit c36252e
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/626a5546e714c200088d7eb3

try {
// Extract the models from the prisma client and use those to
// set up internal redirects for the return values in resolvers.
const localPrisma = require('@prisma/client')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ making sure this is highlighted - there might be other alternative methods to grabbing the list of prisma models which doesn't involve require-ing from the user's node_modules.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out - I get your reservation that it feels a little off, but I also think Prisma is designed to do this, with the generated types ending going into node_modules anyway!

@fenos
Copy link

fenos commented Apr 23, 2022

This feels somewhat the right thing to do with the services definitions.
Is there an ETA on when this will be merged?

@jtoar
Copy link
Contributor

jtoar commented Apr 25, 2022

Thanks for your patience @orta! And @fenos there's no timetable yet, but I'll bring it up at the meeting this week to see if we can prioritize it. In general we don't like to keep things waiting but we're still working though v1 backlog.

@Urigo
Copy link

Urigo commented Apr 27, 2022

@orta not really related to the changes here but what would you imagine for the Relay style resolver generator?
Do you mean writing the schema and resolvers next to each other?
We had some interesting experiments in the past that might be related?
dotansimha/graphql-code-generator#6379
Would love to hear your thoughts on that

@orta
Copy link
Contributor Author

orta commented Apr 27, 2022

wow cool - dotansimha/graphql-code-generator#6379 its in the space for what I was thinking, yep - that's a clever TypeScript approach (using the string of the query to be a matcher against a central type mapping)

I think structurally I'd still end up with something a bit simpler. E.g. each set of resolvers in each SDL string would end up with a corresponding .d.ts which only has the types for that set (which would echo how relay handles that client-side)

It's a clever exploration as it looks like there are WIP builds, I will give it a run at some point (that shouldn't need to block this PR) - thanks

@dac09
Copy link
Collaborator

dac09 commented Apr 28, 2022

Nice one @orta - I do prefer this approach a lot more - as it keeps the type generation all in "one place". Let me try a few things out and I'll merge - looks like you've covered all the tests and stuff already! Thank you so much!

@dac09
Copy link
Collaborator

dac09 commented Apr 28, 2022

Needs one small change, fixing and will merge afterwards!

@dac09
Copy link
Collaborator

dac09 commented Apr 28, 2022

Added a few small fixes and tested with:

  • rw g service --crud
  • rw g sdl without relation
  • rw g sdl with relation
  • rw g scaffold

@dac09 dac09 enabled auto-merge (squash) April 28, 2022 09:10
@dac09 dac09 merged commit 7ae7183 into redwoodjs:main Apr 28, 2022
@jtoar jtoar added this to the next-release milestone Apr 28, 2022
@orta
Copy link
Contributor Author

orta commented Apr 28, 2022

Nice

dac09 added a commit to dac09/redwood that referenced this pull request Apr 28, 2022
…ok-smoke-test

* 'main' of github.com:redwoodjs/redwood:
  Use GraphQL Codegen to generate more accurate resolver types (redwoodjs#5216)
  chore(deps): update dependency @clerk/clerk-sdk-node to v3.3.8 (redwoodjs#5345)
  fix(deps): update dependency jest-watch-typeahead to v1.1.0 (redwoodjs#5339)
@jtoar jtoar modified the milestones: next-release, v1.3.0 Apr 30, 2022
dac09 added a commit to dac09/redwood that referenced this pull request May 2, 2022
…-core-tests

* 'main' of github.com:redwoodjs/redwood: (668 commits)
  Fix broken link in authentication.md (redwoodjs#5394)
  Update local-postgres-setup.md (redwoodjs#5353)
  modify typescript example (redwoodjs#5377)
  fix(deps): update dependency core-js to v3.22.3 (redwoodjs#5376)
  fix(deps): update dependency @types/node to v16.11.32 (redwoodjs#5374)
  fix(deps): update prisma monorepo to v3.13.0 (redwoodjs#5375)
  chore(deps): update dependency typescript to v4.6.4 (redwoodjs#5372)
  chore(deps): update dependency nodemon to v2.0.16 (redwoodjs#5364)
  fix(docs): add WindiCSS in setup ui list (redwoodjs#5369)
  chore(deps): update dependency @nhost/nhost-js to v1.1.6 (redwoodjs#5363)
  Add back git fetch step for lerna
  chore(deps): update dependency @clerk/clerk-sdk-node to v3.3.10 (redwoodjs#5358)
  fix(deps): update dependency @types/node to v16.11.28 (redwoodjs#5326)
  update all contributors (redwoodjs#5357)
  Patch all-contributors-cli (redwoodjs#5356)
  Remove unused `SubmitHandler` in example JavaScript code (redwoodjs#5352)
  Use GraphQL Codegen to generate more accurate resolver types (redwoodjs#5216)
  chore(deps): update dependency @clerk/clerk-sdk-node to v3.3.8 (redwoodjs#5345)
  fix(deps): update dependency jest-watch-typeahead to v1.1.0 (redwoodjs#5339)
  Update yarn.lock
  ...
@orta
Copy link
Contributor Author

orta commented May 14, 2022

Forgot to note for folks how extend the GraphQL context - so here's a quick example from my codebase's api/src/functions/graphql.ts:

import { createGraphQLHandler } from "@redwoodjs/graphql-server"

import directives from "src/directives/**/*.{js,ts}"
import sdls from "src/graphql/**/*.sdl.{js,ts}"
import services from "src/services/**/*.{js,ts}"

import { getCurrentUser } from "src/lib/auth"

import { db } from "src/lib/db"
import { logger } from "src/lib/logger"
import { APIGatewayProxyEvent } from "aws-lambda"

// Extends the context API type with a new string
declare module "@redwoodjs/graphql-server/dist/functions/types" {
  interface RedwoodGraphQLContext {
    gamePlayUserID: string
  }
}

// Then this is how I add to the context via extraPlugins
export const handler = (event: APIGatewayProxyEvent, context: any) =>
  createGraphQLHandler({
    getCurrentUser,
    loggerConfig: { logger, options: {} },
    directives,
    sdls,
    services,
    extraPlugins: [
      {
        onExecute: async (e) => {
          e.extendContext({ gamePlayUserID: event.headers["thingy-gameplay-id"] })
        },
      },
    ],
    // ...
  })(event, context)

@dac09
Copy link
Collaborator

dac09 commented May 14, 2022

Thanks @orta - the other way you can do this is, which admittedly is undocumented (but in the types). I'll make sure I add it into the docs shortly

export const handler = (event: APIGatewayProxyEvent, context: GlobalContext) =>
  createGraphQLHandler({
    getCurrentUser,
    loggerConfig: { logger, options: {} },
    directives,
    sdls,
    services,
    context: () => {
      return {
        ...context,
          gamePlayUserID: event.headers["thingy-gameplay-id"] }
       }
    }
    // ...
  })(event, context)

I haven't tried it recently, but I think you can also avoid wrapping the function if you want, because the context already has the event as context.event

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants