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

feat: fields and population selection with Local API / REST #5942

Open
wants to merge 20 commits into
base: beta
Choose a base branch
from

Conversation

r1tsuu
Copy link
Contributor

@r1tsuu r1tsuu commented Apr 21, 2024

Description

The problem: you want to create a link field, which has relation to Pages collection. But your Pages collection contains many many fields and as well relations, fetching all of that just for slug seems too much unnecessary.
This PR perfectly solves this i believe.

Added 2 parameters to operations / endpoints (select, populate) and 1 parameter to a relationship field config (defaultPopulate), more examples in the separated test config:
select - allows to specify which fields to return from a database.
Considerations: the document's id, array / blocks items that are selected id's, blockName, blockType, relationship ids within selected field depth - are automatically selected.

// returns only title and label - doc.title, doc.label
const post = await payload.findByID({
  collection,
  id,
  select: {
    title: true,
    label: true,
  },
})

// returns array items and `title` inside of them, doc.array[0].title
// this could be a group / tab as well, if so - doc.group.title
const post = await payload.findByID({
  collection,
  id,
  select: {
    array: {
      title: true,
    },
  },
})

// returns array items and all fields inside of them. This could be a group / tab as well.
const post = await payload.findByID({
  collection,
  id,
  select: {
    array: true
  },
})

// the same as with array, but with block that has slug "section" - doc.blocks[0].title
const post = await payload.findByID({
  collection,
  id,
  select: {
    blocks: {
      section: {
        title: true,
      },
    },
  },
})

populate - allows you to specify which relationship fields to populate, as well you can specify select for the current population and nested populate. It still respects depth parameter just to not break the current logic, but we can tweak that i think, it won't populate other relations in the same depth, but if your depth value is not enough it won't populate specified fields.
Example:

// Populates only `item` field doc - all fields.
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: true,
  },
});

// Populates only `item` feield doc and selects only title field from population
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: {
      select: {
        title: true,
      },
    },
  },
});

// Populates `item` field doc and `nested` field doc inside, all fields
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 2,
  populate: {
    item: {
      populate: {
        nested: true,
      },
    },
  },
});

// Populates `item` inside nested to array field
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    'array.item': true,
  },
})

// Polymorphic population of the field named `polymorphic`
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  populate: {
    polymorphic: [
      {
        relationTo: "relationships-items-nested",
        value: {
          select: {
            title: true,
          },
        },
      },
    ],
  },
});

defaultPopulate - configuration for relationships / upload fields:

// By default if in query `populate` is not provided, will be used this configuration for that field
{
  name: 'withDefaultPopulate',
  type: 'relationship',
  relationTo: 'relationships-items',
  defaultPopulate: {
    select: {
      title: true,
    },
  },
},

// The same for polymorphic
{
  name: 'polymorphicDefault',
  type: 'relationship',
  defaultPopulate: [
    {
      relationTo: 'relationships-items',
      value: {
        select: {
          title: true,
        },
      },
    },
  ],
  relationTo: ['relationships-items', 'relationships-items-nested'],
},
  • I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • I have added tests that prove my fix is effective or that my feature works
  • Existing test suite passes locally with my changes
  • I have made corresponding changes to the documentation

@r1tsuu r1tsuu changed the title feat: implement fields selection on database level feat: implement fields selection on a database level Apr 21, 2024
@r1tsuu r1tsuu changed the title feat: implement fields selection on a database level feat: implement fields selection on a database level and select which relations to populate May 1, 2024
@r1tsuu
Copy link
Contributor Author

r1tsuu commented May 1, 2024

Hi again!
I've implemented this feature for relationships with a separated populate property, which allows you to specify which relationship fields to populate (and you can select which fields to return for them as well). And also added option defaultPopulate to the relationship field config.
Examples:

// Populates only `item` field doc - all fields.
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: true,
  },
});

// Populates only `item` feield doc and selects only title field from population
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: {
      select: {
        title: true,
      },
    },
  },
});

// Populates `item` field doc and `nested` field doc inside, all fields
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 2,
  populate: {
    item: {
      populate: {
        nested: true,
      },
    },
  },
});

// Populates `item` inside nested to array field
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    'array.item': true,
  },
})

// Polymorphic population of the field named `polymorphic`
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  populate: {
    polymorphic: [
      {
        relationTo: "relationships-items-nested",
        value: {
          select: {
            title: true,
          },
        },
      },
    ],
  },
});

Configuration for Relationships / Upload fields: defaultPopulate:

// By default if in query `populate` is not provided, will be used this configuration for that field
{
  name: 'withDefaultPopulate',
  type: 'relationship',
  relationTo: 'relationships-items',
  defaultPopulate: {
    select: {
      title: true,
    },
  },
},

// The same for polymorphic
{
  name: 'polymorphicDefault',
  type: 'relationship',
  defaultPopulate: [
    {
      relationTo: 'relationships-items',
      value: {
        select: {
          title: true,
        },
      },
    },
  ],
  relationTo: ['relationships-items', 'relationships-items-nested'],
},

It still actually respects depth parameter, so if you are populating relationship that needs depth "3", you need to specify that, but it won't populate other documents in this depth.

@r1tsuu r1tsuu closed this May 1, 2024
@r1tsuu r1tsuu reopened this May 1, 2024
@r1tsuu
Copy link
Contributor Author

r1tsuu commented May 1, 2024

Happy to share that it passes all my 33 integration tests with both, mongodb and postgres adapters.

@r1tsuu r1tsuu marked this pull request as ready for review May 1, 2024 04:17
@AlessioGr
Copy link
Member

Hey @r1tsuu super interesting PR, thank you for your contribution!

One thing I'm worried about are hooks. What if you added a field or collection hook to a field which depends on the value on another field? That hook would break if the other field isn't queried.

@r1tsuu
Copy link
Contributor Author

r1tsuu commented May 1, 2024

Hey @r1tsuu super interesting PR, thank you for your contribution!

One thing I'm worried about are hooks. What if you added a field or collection hook to a field which depends on the value on another field? That hook would break if the other field isn't queried.

Yes i thought about this too and i think that's basically the cost of optimizations. You need to ensure that the non selected data isn't used in the hooks. Also it's only a problem here with beforeRead and afterRead i believe, most of our business logic in the other hooks, in beforeValidate beforeChange afterChange nothing should be changed since this PR doesn't add select for update and create.

@r1tsuu r1tsuu changed the title feat: implement fields selection on a database level and select which relations to populate feat: fields and population selection with Local API / REST May 2, 2024
@r1tsuu
Copy link
Contributor Author

r1tsuu commented May 5, 2024

@AlessioGr Hi, any chance it could be reviewed? Or is it something that you don't want bring to now? While there's no a discussion with roadmap to this, i believe i saw somewhere that field selection feature is planned.

@jmikrut
Copy link
Member

jmikrut commented May 13, 2024

Hey @r1tsuu just a quick update from our side, this definitely needs to make its way into Payload but our team won't likely have bandwidth to give this PR the justice that it needs until at least a week from now or so. But we'll get there.

My big question will be TypeScript. I have not looked at the code yet here, but have you solved typing accordingly as well?

I will report back here shortly!

@r1tsuu
Copy link
Contributor Author

r1tsuu commented May 13, 2024

Hey @r1tsuu just a quick update from our side, this definitely needs to make its way into Payload but our team won't likely have bandwidth to give this PR the justice that it needs until at least a week from now or so. But we'll get there.

My big question will be TypeScript. I have not looked at the code yet here, but have you solved typing accordingly as well?

I will report back here shortly!

Hi, thank you for the message here!
About TypeScript:
Although i typed parameters populate and select

export type Select = {
  [key: string]: Select | boolean
}

export type PopulatePolymorphicValue = {
  relationTo: string
  value:
    | {
        populate?: Populate
        select?: Select
      }
    | boolean
}[]

export type PopulateSingleValue = {
  populate?: Populate
  select?: Select
}

export type PopulateValue = PopulatePolymorphicValue | PopulateSingleValue

export type Populate = {
  [key: string]: PopulateValue | boolean
}

It's far from full-typed system for this feature of course, but it matches our current implementation of type for where.

Do you mean it should work like in for example Prisma or Drizzle?
I don't think i have a good idea on how to implement that for populate, especially for polymorphic relations, these are hard ones here. We also haven't solved that for our current depth parameter.

It maybe would be better if we had some additional type-gen for these params, like i believe we plan with Where.
I can try to experiment with it this week, do you think it's fine for type-gen here or should i avoid it?

Also i have few considerations:

  1. Currently not selected array/blocks fields are returned as [], not undefined. As well for non selected group fields - {}. Although i'm not fetching them from database, it seems like we're applying to them this default value in afterRead,
  2. Non selected fields are still present in the returned data object, but with undefined value. (the reason here is the same as in 1). They could be deleted with JSON.stringify, meaning also that it's only in Local API, in REST API response we won't see them.
  3. Depth parameter is still respected, meaning that:
// This will work
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: true,
  },
});

// This will work. And only `item` will be populed on the depth level 1
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: true,
  },

// This - won't, as we haven't increased `depth` to 2.
const doc = await payload.findByID({
  collection: docWithRelationSlug,
  id: docWithRelationId,
  depth: 1,
  populate: {
    item: {
      populate: {
        nested: true,
      },
    },
  },
});
});

This was done in order to ensure that we can't exceed maxDepth and to keep changes as small as possible.

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.

None yet

3 participants