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

Add support for passing gql.tada fragments in context.query #9490

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

emmatown
Copy link
Member

@emmatown emmatown commented Feb 21, 2025

The short version is, assuming you have gql.tada setup and you have an import to @keystone-6/core/gql.tada somewhere (this includes some types).

From the example updated in this PR:

const data = await context.query.Post.findMany({
  where: { author: { id: { equals: item.id } } },
  orderBy: { publishDate: 'desc' },
  query: graphql(`
    fragment _ on Post {
      title
    }
  `),
  take: 1,
})

this makes the data variable type:

readonly {
    title: string | null;
}[]

The types to make this work are only added when @keystone-6/core/gql.tada is imported so that Keystone itself doesn't have to take on a hard dependency on gql.tada since then we would either have a normal dependency on gql.tada and create the same problems that Keystone has with requiring a specific version of Next or make it a required peer dependency and force every user to install gql.tada even if they're not using it. This way it's optional and people don't have to use an exact version of gql.tada that matches what Keystone specified. This is what the weird declaration merging stuff is for, it's weird but it works fine and doesn't really affect users (beyond being required to import @keystone-6/core/gql.tada before this works).

While users have to specify a bit more when selecting fields (especially the type condition, on Post/etc.), this is setup so that it enforces that the type condition matches the list you're querying on so if you mess it up, you get a TypeScript error (the runtime error you see getting thrown in the code is just a backstop, you will get a TypeScript error for it, not just the runtime error).

Technically this also makes the following code work:

// data is typed as `readonly { title: string | null; }[]`
const data = await context.query.Post.findMany<{ title: string | null }>({
  where: { author: { id: { equals: item.id } } },
  orderBy: { publishDate: 'desc' },
  query: 'title',
  take: 1,
})

That's not really by design and you probably shouldn't do it but just due to the way this is written, that also works which seems fine imo. (like we could do this in a way in which that code wouldn't work but I don't really think it's a problem)


The making context.query.*.findOne return Promise<T | null> instead of Promise<T> (which was wrong) is unrelated the other type stuff but I just noticed it was wrong and fixed it here.

@emmatown emmatown force-pushed the context-query-gql-tada-support branch 5 times, most recently from 7e11406 to 99c63f8 Compare February 25, 2025 23:46
@emmatown emmatown force-pushed the context-query-gql-tada-support branch from 99c63f8 to 5261ef3 Compare February 26, 2025 03:54
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

Successfully merging this pull request may close these issues.

1 participant