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

GraphQL RFC #21

Closed
berekuk opened this issue Mar 27, 2022 · 3 comments
Closed

GraphQL RFC #21

berekuk opened this issue Mar 27, 2022 · 3 comments

Comments

@berekuk
Copy link
Collaborator

berekuk commented Mar 27, 2022

I'll explain my opinions and decisions I'm planning to make regarding the upcoming GraphQL API.

Some of these should IMO be done upfront, some are more like a future roadmap and braindump of my previous experience.

Short version:

  • let's use urql, nexus and graphql-code-generator
  • mutations, inputs and error handling conventions are important from the start
  • we need to decide on API stability guarantees, but this can be done later

(long version)

We face multiple choices:

  1. which client-side library to use for graphql requests
  2. how to implement graphql server on the backend side of API
  3. how to handle non-trivial parts of API, e.g. mutations and errors
  4. how much of backward compatibility we're going to provide for third-party users

1. Client-side library

There are multiple possible options for how to do graphql queries from the frontend.

Most of my own experience with Apollo Client, and I have some experience with using urql in a minor unfinished project (both are pretty powerful and include a normalized document cache; both seem pretty much equivalent to me). IMO Apollo Client's implementation is bloated and I'd go with urql for any new project.

We could also live on just graphql-request (which is already used in the codebase on the backend) or react-query for a while, but eventually any non-trivial graphql codebase requires a normalized document cache, so I propose we go with urql to avoid later refactorings.

I'd also like to set up graphql-code-generator immediately; it makes the overall dev experience much nicer.

2. GraphQL server

It's possible to write a graphql server from scratch with graphql-js, but that's quite low-level and annoying.

Prisma (which we probably will adopt later as an ORM, but that's a different topic) recommends three solutions: graphql-tools, Nexus and TypeGraphQL.

Graphql-tools is SDL-first (first you write GraphQL schema and then you implement the methods listed in schema); in my experience SDL-first approach can become limiting, e.g. if we list all platforms in code and then want to turn them into graphql enum then we'd have to do it by hand, or deal with string templates to generate an SDL from code.

So I prefer code-first approach (I've tried both). This leaves us with Nexus vs TypeGraphQL. I haven't tried TypeGraphQL, but I remember evaluating this choice in the past (a year ago or so) and deciding that Nexus is a safer choice, though TypeGraphQL APIs might be more convenient, with more decorator magic. I've used Nexus in a few small projects in the last year and had no significant issues.

Three minor issues I had with Nexus:

  1. It's kind of weird how it regenerates the types based on code, so you often type the code which is not valid in typescript's opinion, then you save it, then typescript decides that it's ok; this is not a bug, though, it's how it's supposed to work with Nexus, and overall the experience is nice when you get used to it.
  2. Its integration with Prisma was deprecated and there were no good solution for some time; but looks like they have recently updated the https://nexusjs.org/docs/adoption-guides/prisma-users page so it should be better.
  3. Its source types is hard to wrap your head around first, but they might be doing as good as possible there, considering that source type vs GraphQL type issue is intrinsic with how GraphQL resolvers always work; at least they provide several different ways to configure types matching (see the link if you want to know more), so it's hard to draw yourself into a corner.

I lean towards just going with Nexus, but considering that GraphQL server usually requires a lot of repetitive code and the cost of switching to a different framework later is O(number-of-implemented-fields), maybe I should evaluate TypeGraphQL vs Nexus (vs something else?) for a few hours first before settling on a decision.

Other considerations:

  • current database structure is not really GraphQL-friendly since most data is stored in JSON fields; it might make sense to implement the really basic GraphQL API first, then work on Prisma/ORM refactoring (I think the entire DB layer of metaforecast should be rewritten, btw), and then expand on GraphQL
  • dataloaders are great for avoiding N+1 performance issues in any non-trivial GraphQL server; we'll get them when we get Prisma (if we settle on Prisma)
  • auth can be done by cookie initially and by user-generated tokens later when we get user accounts

3. API design

I'll describe my previous experience with GraphQL API design here. I probably put more details here than necessary, but this can be the draft for the future coding style docs, so I hope it's not a waste.

GraphQL queries are mostly straightforward and easy to evolve.

GraphQL mutations are a bit more tricky: there's a temptation of cutting the corners and doing something like:

type Mutation {
  createForecast(title: String!, description: String) Boolean
}

Issues with this example:

First, there are just two input fields for now, but it might grow up to ten or more.

And then you'd have to query it like this, even for two fields:

const q = gql```
mutation CreateForecast($title: String!, $description: String!) {
  createForecast(title: $title, description: $description)
}
```;

graphqlClient.request(q, { title, description });

It gets annoying really quickly.

The solution for this is to create Input types for every mutation, even if it feels excessive. Having common conventions for everything is great, and backend code can be simplified with helper functions to avoid copy-paste (this is one of the reasons why code-based is better than SDL-based).

input CreateForecastInput {
  title: String!
  description: String!
}

type Mutation {
  createForecast(input: CreateForecastInput!) Boolean
}

I'm currently agnostic on whether the input pattern is needed for non-mutation fields. Probably not by default, since those inputs usually evolve much slower.

Second, return values. You might think initially that your mutation is trivial and you don't need to return anything, but it's hard (read "impossible without breaking prod") to change a graphql field from scalar value to object.

Also, if you return an "obvious" object (e.g., Forecast in case of createForecast example), then you won't be able to return anything else beside it.

The best practice for this is to always create a Result object for every GraphQL mutation.

So:

type CreateForecastResult {
  forecast: Forecast
  error: String
}

type Mutation {
  createForecast(input: CreateForecastInput!) CreateForecastResult!
}

Or, maybe:

union CreateForecastResult = Forecast | GenericError

type Mutation {
  createForecast(input: CreateForecastInput!) CreateForecastResult!
}

Or:

type CreateForecastOkResult {
  forecast: Forecast!
  # room for more fields
}

union CreateForecastResult = CreateForecastOkResult | GenericError

type Mutation {
  createForecast(input: CreateForecastInput!) CreateForecastResult!
}

I'm unsure if the last one is not an overkill, but all of these are better than returning a scalar or something like { ok: true }.

This might feel like too much stuff for every minor mutation, but... well, I tried to cut these corners and didn't like the consequences.

Third, error handling. As can be seen in the examples above, I'm returning a GenericError object instead of relying on GraphQL native errors. That's because native GraphQL errors suck and everyone agrees that you shouldn't rely on them if code didn't fail through an exception. This is a large topic and I won't expand on it here, but basically we should treat error objects as first-class citizens in our API design.

Fourth, on nulls. This is a topic on which I diverge from the mainstream opinions in GraphQL community, or at least from the opinions of the initial GraphQL standard authors. Standards basically say "mark fields as non-nullable only when you know what you're doing".

This helps with graceful degradation when your project is built from multiple backend microservices where each microservice provides its chunk of data and can fail separately; but in my experience it's too much pain to no clear benefit to check every field if it's null on the frontend, and I prefer to just mark everything with !s by default.

Fifth, on pagination. Don't have a custom opinion here, let's just use Connections. Again, in my previous project I tried to cut this corner with custom page: $id inputs and had to refactor later. Relay-style pagination is clunky but other approaches are worse.

4. Stability guarantees

GraphQL is easy to evolve, but when we get third-party clients eventually, we'll have to avoid changing it too frequently.

This means we'll have to go through the "deprecate a field, wait for a few weeks or months, remove the field" dance for all refactorings.

Depending on how large the project is going to be, this might mean setting up a mailing list for devs and sending notifications about backward-incompatible API changes there.

Having some statistics on "how many clients have requested this field over last N days" might be nice too. Apollo Studio does this, but I don't have any experience with it since it's a bit costly and seems too much like a corporate lock-in. This might be done by hand with GraphQL tracing tools.

But all this is just stuff for later discussions and we don't need to worry about it for now, I guess.

@OAGr
Copy link

OAGr commented Mar 28, 2022

This all sounds pretty reasonable to me. It sounds like you've had very relevant experience before.

Right now we're really unsure how much we expect metaforecast to grow / how much funding it should/will absorb. I'd be surprised if we have over 3 reliable API users that we care about (effective altruists), 6 months from now. That said, most of the decisions you're recommending don't seem that high-cost to me, so seem like probably good bets to make now.

@NunoSempere
Copy link
Collaborator

I agree with Ozzie that this looks good, but am more optimistic about number of users.

@berekuk berekuk mentioned this issue Mar 29, 2022
6 tasks
@berekuk
Copy link
Collaborator Author

berekuk commented Mar 29, 2022

Great, I'll continue with this plan in #32.

@berekuk berekuk closed this as completed Mar 29, 2022
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

No branches or pull requests

3 participants