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 calling fields with arguments #301

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Nov 27, 2023

GraphQL supports fields at arbitrary depths that take arguments. We already support passing args at the base-most level of a query, like with:

await api.widget.findMany({filter: { ... }});

but, GraphQL supports sending args at any depth at all! For example, you might want to fetch a filtered list of widgets, and their child gizmos, and filter the list of gizmos for each widget as well! In GraphQL, this looks like:

query {
 widgets(filter: { inventoryCount: { greaterThan: 10 } }) {
   edges {
     node {
       id
       inventoryCount
       gizmos(first: 10, filter: { published: { equals: true } }) {
         edges {
           node {
             id
             published
           }
        }
      }
    }
  }
}

We don't currently support this in the JS client, even though our GraphQL schema supports it fine. The main use case is adjusting how many child records you get back for each parent on a nested fetch, as well as filtering those child records, and asks in discord have cropped up a bunch for this. In this code snippet, where do you put arguments for gizmos?:

// before, no calls supported
await api.widget.findMany({
  select: {
    id: true,
    name: true,
   // how pass args right here?
    gizmos: {
      edges: {
        node: {
          id: true,
          published: true,
        }
      }
    }
  }
});

This adds a thing to do this just this! There's two parts to it: knowing which fields take arguments, which we currently don't really describe, and then adding a JS-land syntax for passing calls at an arbitrary place in the selection. I chose to do this with a new primitive that looks like this:

import { Call } from "@gadget-client/kitchen-sink";

await api.widget.findMany({
  select: {
    id: true,
    name: true,
    gizmos: Call({
      first: 10,
      filter: { published: { equals: true } },
    }, {
      edges: {
        node: {
          id: true,
          published: true,
        }
      }
    })
  }
});

What I like about this is that it isn't a breaking change, and you only have to deal with it once you start caring about calling fields -- selections remain nice and simple in the general case. I don't love it though, its a bit weird to mix objects into the tree like this, and it sucks that you still need to care about edges and nodes for connections, which I think will be the lion's share of the use case.

Also tricky is making this typesafe. Args like this don't really affect the output type, but ideally, the arguments you did pass are typechecked too. Currently, the Select<> type helper traverses a Schema type that looks like this:

export type Schema = {
  widgets: { 
    edges: {
      node: { 
        id: string;
        name: string;
        gizmos: {
          edges: { 
            node: { 
              id: string;
              name: string;

which doesn't describe the type of the arguments. We need to get them in somehow. We could make a breaking change to this type, but in case anyone is importing it, and to lower the risk we accidentally break something, I would love to keep the structure of this schema type largely the same. Open to other ideas, but I propose we change the generated type Gadget side to look like this:

export type Schema = {
 widgets: { 
   ["$args"]: {
     first: number;
     last: number;
     after: string;
     before: string;
     filter: WidgetFilter;
   };
   edges: {
     node: { 
       id: string;
       name: string;
       gizmos: {
         ["$args"]: {
           first: number;
           last: number;
           after: string;
           before: string;
           filter: GizmosFilter;
          };
         edges: { 
           node: { 
             id: string;
             name: string;

Field Identifiers starting with $ are illegal in GraphQL, so we know there will never be a field named $args, and can thusly stash the types there. Then, the Select<> type gets a bit smarter and starts looking for Call objects, and inspecting for args if there are any.

PR Checklist

  • Important or complicated code is tested
  • Any user facing changes are documented in the Gadget-side changelog
  • Any immediate changes are slated for release in Gadget via a generated package dependency bump
  • Versions within this monorepo are matching and there's a valid upgrade path

GraphQL supports fields at arbitrary depths that take arguments. We already support passing args at the base-most level of a query, like with:

```typescript
await api.widget.findMany({filter: { ... }});
```

but, GraphQL supports sending args at any depth at all! For example, you might want to fetch a filtered list of widgets, and their child gizmos, and filter the list of gizmos for each widget as well! In GraphQL, this looks like:

```graphql
query {
 widgets(filter: { inventoryCount: { greaterThan: 10 } }) {
   edges {
     node {
       id
       inventoryCount
       gizmos(first: 10, filter: { published: { equals: true } }) {
         edges {
           node {
             id
             published
           }
        }
      }
    }
  }
}
```

We don't currently support this in the JS client, even though our GraphQL schema supports it fine. The main use case is adjusting how many child records you get back for each parent on a nested fetch, as well as filtering those child records, and asks in discord have cropped up a bunch for this. In this code snippet, where do you put arguments for gizmos?:

```typescript
// before, no calls supported
await api.widget.findMany({
  select: {
    id: true,
    name: true,
    gizmos: {
      edges: {
        node: {
          id: true,
          published: true,
        }
      }
    }
  }
});
```

This adds a thing to do this just this! There's two parts to it: knowing which fields take arguments, which we currently don't really describe, and then adding a JS-land syntax for passing calls at an arbitrary place in the selection. I chose to do this with a new primitive that looks like this:

```typescript
await api.widget.findMany({
  select: {
    id: true,
    name: true,
    gizmos: Call({
      first: 10,
      filter: { published: { equals: true } },
    }, {
      edges: {
        node: {
          id: true,
          published: true,
        }
      }
    })
  }
});
```

[no-changelog-required]
@airhorns airhorns requested review from thegedge and jasong689 and removed request for thegedge and jasong689 November 27, 2023 16:22
@infiton
Copy link
Contributor

infiton commented Nov 27, 2023

not a huge fan of the import { Call }; do you think there is a way we could put it on the model manager so:

await api.widget.findMany({
  select: {
    id: true,
    name: true,
    gizmos: api.call({
      first: 10,
      filter: { published: { equals: true } },
    }, {
      edges: {
        node: {
          id: true,
          published: true,
        }
      }
    })
  }
});

also find call kind of generic, what about something like SubQuery or IncludeRelated?

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.

2 participants