remorses / genql

Type safe TypeScript client for any GraphQL API
https://genql.dev
MIT License
881 stars 37 forks source link

RFC: New Genql major version #161

Closed remorses closed 5 months ago

remorses commented 9 months ago

New syntax for the client, similar to the Linear SDK, easier to use, even for people not familiar with GraphQL

Goals:

Why?

The plan for genql.dev is to create a service similar to Stainless where you can generate SDKs for your GraphQL API together with a docs website (something like this but for GraphQL) and a changelog (automatically generated from the schema changes)

Genql generator will remain open source and also have a cli, only the service and the SDK publishing code will be private

[!IMPORTANT] Leave your opinion on the new changes, do you think it's worth a breaking change or not?

Example queries:

Fetch all scalar fields on a type with no arguments

const res = await client.getCountries()

Fetch all scalar fields on a type (scalar fields that don't have arguments)

const res = await client.getCountries({
    args: { name: 'USA' },
})

console.log(countries.map((x) => x.name))

Fetch subdocuments (non scalar fields)

const res = await client.getCountries({
    args: { name: 'USA' },
    include: {
        regions: true,
    },
})

Fetch subdocuments with arguments

const res = await client.getCountries({
    args: { name: 'USA' },
    include: {
        regions: {
            args: { name: 'California' },
        },
    },
})

Select only specific scalar fields or fetch scalar field with an argument

const res = await client.getCountries({
    args: { name: 'USA' },
    select: {
        name: true,
        code: {
            arg: {
                format: 'ISO_ALPHA_2',
            },
        },
    },
})

Select only specific scalar fields

const res = await client.getCountries({
    args: { name: 'USA' },
   select: {
       name: true
    },
    include: {
        regions: {
            args: { name: 'California' },
        },
    },
})

Fetch an union or interface type

const res = await client.getNodes({
    onCountry: {
        select: {
            name: true,
        },
    },
    onContinent: {}, // fetch all scalar fields
})

If you don't pass onType fields Genql will fetch all scalar fields on all possible types

Fetching many top level fields at the same time

Genql will batch queries done in the same tick. An option will allow you to set a manual batch time in milliseconds

This means that doing the following will batch the queries into a single request:

const [a, b] = await Promise.all(client.getNodes(), client.getCountries())

[!IMPORTANT] Should batch create a single query or use the Apollo batch protocol?

Mutations

Mutations will be exposed as client top level methods the same way as queries. If a top level mutation has the same name as a query field it will be prefixed with mutation, for example getCountries will become mutationGetCountries

[!WARNING] The same name for both query and mutation should be a very rare case, if you know about any instance of this please add it in the comments below

Aliases

Aliases are commonly used for

These cases are already possible using batching

Directives

At first directives won't be supported, support will be added later with the following syntax

const res = await client.getCountries({
    directives: {
        specifiedBy: {
            url: 'https://example.com',
        },
    },
    include: {
        regions: {
            args: { name: 'California' },
        },
    },
})

Using this syntax means that directives on arguments won't be supported, does anyone uses them? The only example i know of is @uppercase or stuff that modifies the argument, which in an SDK can be done in code

Get query and variables without executing the request


import { operationClient } from './generated'

const [{ query, variables }] = operationClient.getCountries()

[!NOTE] operationClient is not final name, give better ideas

jasonkuhrt commented 9 months ago

Exciting! Generally LGTM, some feedback:

Fetch all scalar fields on a type with no arguments

👍

Fetch all scalar fields on a type (scalar fields that don't have arguments)

👍

Fetch subdocuments (non scalar fields)

👍

Fetch subdocuments with arguments

👍

Select only specific scalar fields or fetch scalar field with an argument

👍

Fetch an union or interface type

👍

Should batch create a single query or use the Apollo batch protocol?

Single query would be a good default I would think, gut feeling. Alternative: Simply expose a client.$batch(client.getY(),client.getX(),...) method to remove the ambiguity.

Aliases are commonly used for

Imagine a case where an analytics filtering expression can be given. Then a user might want to get multiple aspects of data analytics in one query.

These cases are already possible using batching

I don't think this is a great answer to the use-case:

  1. User wants to keep a single query response tree to serve their view
  2. User may haver aliases deep in a query tree
  3. User wants to provide alias names colocated with where that field selection is
  4. User wants to make the alias declarative and colocated in one place, not spread out the problem in an ambiguous way (two queries could be for many reasons, and now it could be for aliasing, which is really a weak reason compared to all other reasons for having multiple queries)
julianbenegas commented 9 months ago

Interesting changes @remorses! The new syntax would definitely be more accessible for devs not that familiar with GraphQL, although I fear that for deeper queries, it'll require many, many includes.

In BaseHub, the GraphQL schema gets defined by the structure the user creates in the Dashboard, which—similar to a filesystem—can be very nested.

Today, we have a query for getting a blog post that's like this:

client.query({
  blog: {
    posts: {
      __args: { first: 1, filter: { _sys_slug: { eq: params.slug } } },
      items: {
        _title: true,
        publishDate: true,
        subtitle: true,
        coverImage: {
          url: true,
          altText: true,
        },
      },
    },
  },
});

which with the new syntax, would look something like:

client.blog({
  include: {
    posts: {
      args: { first: 1, filter: { _sys_slug: { eq: params.slug } } },
      include: {
        items: {
          select: {
            _title: true,
            publishDate: true,
            subtitle: true,
          },
          include: {
            coverImage: true,
          },
        },
      },
    },
  },
});

Comparing them side by side, the current ("old" syntax) looks better IMO, and is easier to understand. "system" properties (prefixed with __) are well differentiated against the objects in your schema.

It feels like requiring an explicit include and select keys to go down one level or to select specific properties is a step backwards from the current syntax, which implicitly selects and includes just by passing true!

Additionally, I love how the current SDK is divided: for queries client.query, and mutations client.mutation. This doesn't require having to rename properties, and helps with IDE autocompletion—you don't get queries and mutations mixed up under the same object.

Fragmenting

I'm interested in understanding how fragmenting would work with the new syntax. Right now, it works pretty well, although I see opportunities for simplification.

Aliases

I very much agree with @jasonkuhrt with his opinions on aliases. This doesn't feel well supported yet.

Some ideas

These are some ideas I have for genql:

Fragmenting

An idea for simplification, which wouldn't require you importing a lot of types to get fragmenting right:

import { Fragment, InferFragment } from './generated-client'

const BlogPostFragment = { title: true } satisfies Fragment<"BlogPost">
type BlogPostFragment = InferFragment<"BlogPost", typeof BlogPostFragment>

Aliases

What if the user could write something like:

client.query({
  "homepage as homepageMeta": {
    meta: {
      title: true,
      description: true,
    },
  },
  homepage: {
    content: true,
  },
});

Would something like this be even possible?


So, this is my braindump. I haven't had the time to go deeper on some of these points, but I'd be happy to do so if it helps move the discussion forward.