feat: Migrate the backend from GraphQL to tRPC #282
Conversation
alexanderson1993
requested review from
cball,
kgajera,
cullylarson,
mthomps4 and
prestonkelly1
| setToken(cookies().get(LOGIN_TOKEN_KEY)); | ||
| }, []); | ||
|
|
||
| const meQuery = trpc.user.me.useQuery(undefined, { enabled: !!token }); |
There was a problem hiding this comment.
{ enabled: !!token } only fetches the user record if there is a valid token.
| @@ -31,6 +31,7 @@ module.exports = { | |||
| }, | |||
| }, | |||
| }, | |||
| transformIgnorePatterns: ['/node_modules/(?!(@swc|@trpc))/'], | |||
There was a problem hiding this comment.
This is necessary for tRPC to work properly. Normally, Jest doesn't transform node_modules, but one of the tRPC packages has some syntax that Jest doesn't understand natively. This makes it so only the tRPC stuff is transformed by Jest.
| @@ -21,21 +18,20 @@ | |||
| "db:seed": "yarn prisma db seed", | |||
| "db:seed:prod": "cross-env APP_ENV=production prisma db seed", | |||
| "db:setup": "yarn db:reset", | |||
| "dev": "concurrently -n \"WATCHERS,NEXT\" -c \"black.bgYellow.dim,black.bgCyan.dim\" \"yarn watch:all\" \"next dev\"", | |||
| "dev": "next dev", | |||
There was a problem hiding this comment.
We don't need to run TypeScript concurrently with our dev server; IDEs take care of TypeScript for us.
| "@chakra-ui/react": "^2.2.0", | ||
| "@chakra-ui/theme": "^2.1.0", | ||
| "@emotion/react": "^11.0.0", | ||
| "@emotion/styled": "^11.0.0", | ||
| "@prisma/client": "^4.0.0", | ||
| "apollo-server-micro": "^3.6.7", | ||
| "@tanstack/react-query": "^4.8.0", |
There was a problem hiding this comment.
We probably should bump this to the latest version, or even wait to merge this until tRPC 10 is out of beta. Though in my testing it's been very stable.
| "@vercel/node": "^1.7.3", | ||
| "chance": "^1.1.8", | ||
| "commander": "^8.1.0", | ||
| "concurrently": "^7.1.0", |
There was a problem hiding this comment.
Not used anymore
| @@ -110,13 +96,11 @@ | |||
| "jest": "^27.2.1", | |||
| "jest-environment-node": "^27.2.0", | |||
| "lint-staged": "^10.2.11", | |||
| "msw": "^0.20.1", | |||
There was a problem hiding this comment.
Not used anywhere in the codebase
There was a problem hiding this comment.
We may want to use something like this to mock NextAuth later.
But agree -- remove for now and we can determine what's needed.
| "nanoid": "^3.1.10", | ||
| "pg": "^8.3.0", | ||
| "prettier": "^2.6.2", | ||
| "prisma": "^4.0.0", | ||
| "start-server-and-test": "^1.14.0", | ||
| "supertest": "^4.0.2", |
There was a problem hiding this comment.
Not used anymore
| }, | ||
| }); | ||
|
|
||
| export class BisonError extends TRPCError { |
There was a problem hiding this comment.
This special error class makes it possible for errors sent to the client to include metadata. The invalidArgs comes from the GraphQL extension we were using for this same behavior. Other fields could be added if we wanted to extend it further.
| const [queryClient] = useState(() => new QueryClient()); | ||
|
|
||
| const [trpcClient] = useState(() => | ||
| trpc.createClient({ |
There was a problem hiding this comment.
One thing this doesn't do is provide a way to mock requests. We could add that feature here - a special link which takes inputs and returns some static output provided in the body of the test.
There was a problem hiding this comment.
🤔 This may be another place where msw could come into play.
I'll look at the testing pieces with @dennis-campos when reviewing NextAuth
There was a problem hiding this comment.
I think that's a good thing, as it discourages mocking until we absolutely need it.
There was a problem hiding this comment.
What would be really cool is if we had bulletproof Stripe/Twilio mocks and SQLite support in Bison. Then, tests run e2e without needing a test database running, and only our third-party APIs are mocked.
There was a problem hiding this comment.
We could set up tRPC to use a special client-side link (instead of the BatchHTTPLink) that calls the tRPC procedures directly instead of over HTTP
There was a problem hiding this comment.
Looks great! 🎉 for major deletions.
The only thing I noticed was a bit of old GQL cruft in the readme, but otherwise things look great.
Also just a quick question on procedure comment conventions. If we want to adopt that, it might be good to add them to the hygen template.
docs/trpc-examples.md
Outdated
| // User Type | ||
| export const User = objectType({ | ||
| name: 'User', | ||
| description: 'A User', | ||
| definition(t) { | ||
| t.nonNull.id('id'); | ||
| t.nonNull.date('createdAt'); | ||
| t.nonNull.date('updatedAt'); | ||
| t.nonNull.string('email'); | ||
|
|
||
| t.nonNull.list.nonNull.field('skills', { | ||
| type: 'Skill', | ||
| resolve: async (parent, _, context) => { | ||
| return context.prisma.user | ||
| .findUnique({ | ||
| where: { id: parent.id }, | ||
| }) | ||
| .skills(); | ||
| }, | ||
| }); | ||
|
|
||
| t.field('profile', { | ||
| type: 'Profile', | ||
| resolve: (parent, _, context) => { | ||
| return context.prisma.user | ||
| .findUnique({ | ||
| where: { id: parent.id }, | ||
| }) | ||
| .profile(); | ||
| }, | ||
| }); | ||
|
|
||
| t.list.field('posts', { | ||
| type: 'Post', | ||
| resolve: async (parent, _, context) => { |
There was a problem hiding this comment.
Yeah, I must have missed this.
docs/trpc-examples.md
Outdated
| // Auth Payload Type | ||
| export const AuthPayload = objectType({ | ||
| name: 'AuthPayload', | ||
| description: 'Payload returned if login is successful', | ||
| definition(t) { | ||
| t.field('user', { type: 'User', description: 'The logged in user' }); | ||
| t.string('token', { | ||
| description: 'The current JWT token. Use in Authentication header', | ||
| }); | ||
| }, | ||
| }); |
docs/trpc-examples.md
Outdated
| me: protectedProcedure.query(async ({ ctx }) => ctx.user), | ||
| users: protectedProcedure |
There was a problem hiding this comment.
I know this is just documentation, but as a general convention, do you think it's worth putting a comment above each procedure explaining what it is?
Example:
/*
Returns a list of users with profiles. Ordered name desc by default.
*/Most of these are straightforward by reading through or with really descriptive procedure names, but it could help with scanning the file and when autocompleting with TS?
There was a problem hiding this comment.
Yeah, or we could name the procedures better. The entire router is called user so the procedures could be called getAll, getById or getMe, and accessed like
trpc.user.getAll.useQuery()
trpc.user.getById.useQuery()
trpc.user.getMe.useQuery()
We've got options, I'm happy to adjust or add comments.
There was a problem hiding this comment.
We can also do this is a followup. just came to mind as I was reviewing. I feel like the right answer is probably both in some capacity.
| include: { profile: true, posts: true }, | ||
| }); | ||
| }), | ||
| }); |
| }); | ||
| }), | ||
| // ... | ||
| }) |
| @@ -29,8 +28,7 @@ We're always improving on this, and we welcome suggestions from the community! | |||
| ## Conventions | |||
|
|
|||
| - Don't copy/paste files, use generators and Hygen templates. Need to update the generator as your project evolves? they are all in the `_templates` folder. | |||
| - Use a single command to run Next, generate Nexus types, and GraphQL types for the frontend. By doing this you can ensure your types are always up-to-date. | |||
| - Don't manually write types for GraphQL responses... use the generated query hooks from GraphQL Codegen. | |||
| - Don't manually write types for tRPC procedures. Infer the types from the router definition. | |||
| }; | ||
|
|
||
| await unlinkFile("api.graphql"); | ||
| await unlinkFile("types.ts"); |
| import type { AppRouter } from '@/server/routers/_app'; | ||
|
|
||
| // exporting this so that we consistently use the same transformer everywhere. | ||
| export const transformer = superjson; |
Changes
This is a major architectural change that's difficult to summarize. I decided to do the entire migration in a single PR since GraphQL itself is so deeply integrated with the rest of Bison. A good portion of the changes are documentation, though, so hopefully reviewing everything else isn't too bad.
First, this PR removes code, config, and dependencies related to GraphQL. That includes Nexus, GraphQL Codegen, Apollo Server, and Apollo Client. This also includes anything that references the files generated by Nexus and GQL Codegen.
Next, I created a tRPC router and middleware for authenticated procedures. I created the
userRouterand replicated all of the queries and mutations that existed in the GraphQL schema. After wiring it up to an API route, I set up the tRPC client and replaced all the GraphQL queries with tRPC calls, making adjustments as necessary to make everything behave nicely.I also rewrote the test files using a new
trpcRequesthelper, which lets you optionally pass a user record to make the request.Once I confirmed the app was generating correctly, I updated the hygiene templates to use tRPC, updated the ReadMe and other documentation.
Checklist