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 select and include to findOne and findMany #7204

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MurzNN
Copy link
Contributor

@MurzNN MurzNN commented Jan 19, 2022

Now findOne and findMany functions in DB API misses ability to select specific fields only, and include relationship fields too.

It is my try to add this missing functionality.

Prisma allows query specific fields, here is the documentation https://www.prisma.io/docs/concepts/components/prisma-client/select-fields#select-specific-fields

So seems we simply can pass those variables from Keystone API to Prisma API.

Here is usage example of this feature directly in Prisma, based on examples/blog:

    async onConnect(context) {
      const result = await context.prisma.author.findMany({
        take: 2,
        select: {
          name: true,
          posts: {
            select: {
              id: true,
            },
          },
        },
      });
      console.log(result[0]);

And here is desired behavior in Keystone DB API:

      const result = await context.db.Author.findMany({
        take: 2,
        select: {
          name: true,
          posts: {
            select: {
              id: true,
            },
          },
        },
      });
      const result2 = await context.db.Author.findMany({
        take: 2,
        include: {
          posts: true,
        },
      });

Resolves #7198

@changeset-bot

This comment was marked as resolved.

@vercel
Copy link

vercel bot commented Jan 19, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/keystonejs/keystone-next-docs/Dw9hkkQjRzoVGwFbYr5P2SPfsr2q
✅ Preview: https://keystone-next-docs-git-fork-murznn-db-api-fin-5f95c5-keystonejs.vercel.app

@vercel vercel bot temporarily deployed to Preview January 19, 2022 08:54 Inactive
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 19, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@MurzNN
Copy link
Contributor Author

MurzNN commented Jan 19, 2022

Problems now only are:

  1. Keystone's Prisma somehow misses the Prisma type, so this line:
import { Prisma } from '@prisma/client';

produces a TS error because of missing types. But if I add @prisma/client in separate project, those types are exist.

  1. Those new arguments are stripped in some parent functions, so in actual functions we are receiving object without select and include keys.

@vercel vercel bot temporarily deployed to Preview January 19, 2022 08:59 Inactive
@fkrauthan
Copy link
Contributor

This would be a great feature for some more heavy processing in some custom mutations for example.

@dcousens

This comment was marked as outdated.

@dcousens dcousens self-assigned this Aug 1, 2023
@dcousens
Copy link
Member

dcousens commented Sep 26, 2023

I think we can add this functionality with the caveat that no read hooks could exist for the underlying item or fields...

Alternatively, we can change the type of item passed into read hooks to be Partial<typeof item>, to indicate that hooks could receive any subset of item fields.

Alternatively, maybe this is an opt-in behaviour, with something like db.selectable: true which will change the types of item to Partial<typeof item>, that way it's non-breaking, but additionally not painful in the typical scenario? 🤔

@dcousens dcousens marked this pull request as draft September 26, 2023 23:30
@dcousens dcousens changed the title [WIP] Add select and include to findOne and findMany Add select and include to findOne and findMany Sep 26, 2023
@mariomnts
Copy link

Is there any plan to support this or even only select the fields that are being requested in the graphql query?

@dcousens
Copy link
Member

@mariomnts plans yes, but how we integrate this type of functionality with hooks is a problem that needs a resolution.

@mariomnts
Copy link

@mariomnts plans yes, but how we integrate this type of functionality with hooks is a problem that needs a resolution.

That makes sense.

In my particular use case I don't use hooks at all in any part of the app so something that comes to my mind that if hooks are not defined a possible optimization is just getting the requested fields.

@dcousens
Copy link
Member

dcousens commented Jun 16, 2024

@mariomnts indeed, this is what I suggested previously

I think we can add this functionality with the caveat that no read hooks could exist for the underlying item or fields...
(#7204 (comment))

@mariomnts
Copy link

@mariomnts indeed, this what I suggested previously

I think we can add this functionality with the caveat that no read hooks could exist for the underlying item or fields...
(#7204 (comment))

@dcousens sorry I missed that comment, that sounds great then. @MurzNN do you plan to continue working on this PR?

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.

Missing relationship fields with many in Database API find query results
5 participants