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

Safe generation for create / Update InputTypes #163

Open
lvauvillier opened this issue Dec 4, 2021 · 10 comments
Open

Safe generation for create / Update InputTypes #163

lvauvillier opened this issue Dec 4, 2021 · 10 comments
Labels
type/feat Add a new capability or enhance an existing one

Comments

@lvauvillier
Copy link
Contributor

Perceived Problem

Currently nexus-prisma expose low level building blocks (field generation from prisma DMMF) to safely define Model objectTypes.

Unfortunately, we can't reuse it to build safe inputTypes for create / update mutations:

  • For create inputTypes:
    Fields with @default value should be nullable and resolve function should be removed

  • For update inputTypes:
    All fields should be nullable and resolve function should be removed

Ideas / Proposed Solution(s)

We can have a new api design with read, create, update:

model User {
  id         String  @id
  createdAt  DateTime   @default(now())
  name       String
}
const User = objectType({
  name: User.$name,
  description: User.$description,
  definition(t) {
    t.field(User.read.id);
    t.field(User.read.createdAt); // generates a NonNull field
    t.field(User.read.name); // generates a NonNull field
  }
})
const UserCreateInput = inputType({
  name: "UserCreateInput",
  definition(t) {
    t.field(User.create.id);
    t.field(User.create.createdAt); // generates a Nullable field
    t.field(User.create.name); // generates a NonNull field
  }
})
const UserUpdateInput = inputType({
  name: "UserUpdateInput",
  definition(t) {
    t.field(User.update.id);
    t.field(User.update.createdAt); // generates a Nullable field
    t.field(User.update.name); // generates a Nullable field
  }
})

Api Design consideration:

  • $name and $description cannot be generated for input types
  • if we want extend the api without breaking change (keep the Model.<fieldName>) we can extend the generated model with Model.$create.<fieldName> and Model.$update.<fieldName>.

@jasonkuhrt what do you think?

@lvauvillier lvauvillier added the type/feat Add a new capability or enhance an existing one label Dec 4, 2021
@HendrikJan
Copy link

There may be two other ways to solve this that require less repetition:

const UserCreateInput = createInputType({ // <-- a different constructor
  name: "UserCreateInput",
  definition(t) {
    t.field(User.id);
    t.field(User.createdAt); // generates a Nullable field
    t.field(User.name); // generates a NonNull field
  }
})

or

const UserCreateInput = inputType({
  name: "UserCreateInput",
  type: "create", // <-- mark as create
  definition(t) {
    t.field(User.id);
    t.field(User.createdAt); // generates a Nullable field
    t.field(User.name); // generates a NonNull field
  }
})

@lvauvillier
Copy link
Contributor Author

@HendrikJan I dont think this is faisable.
nexus-prisma models are generated when you run prisma generate. All possibilities (read, create and update) should be generated at this time.

@jasonkuhrt
Copy link
Member

jasonkuhrt commented Dec 6, 2021

@lvauvillier Thanks for this proposal. This is quite interesting. So far I'm leaning toward Model.$create.<fieldName> and Model.$update.<fieldName>.

@HendrikJan The first one should be doable somehow since its a new function that can do anything. The second one should be doable with a Prisma plugin. That said, the original proposal here seems to capture the static nature of NP right now well 🤔.

@HendrikJan
Copy link

That said, the original proposal here seems to capture the static nature of NP right now well 🤔.

Then I guess the original proposal will cause the least amount of headaches which sounds good to me.

@jasonkuhrt
Copy link
Member

jasonkuhrt commented Dec 6, 2021

@lvauvillier I don't care about the breaking changes aspect. I'd just like to get with the best API.

The reason I felt that $create and $update were good is that READ seems like a natural default representation because it reflects what the data "at rest" looks like while the others represent operations over that data.

That said I'm willing to discuss why "uniform":

<Model>.name
<Model>.description
<Model>.read.<field>
<Model>.update.<field>
<Model>.create.<field>

Is better than "read-bias":

<Model>.$name
<Model>.$description
<Model>.$update.<field>
<Model>.$create.<field>
<Model>.<field>

Pros of uniform:

  • system symmetry
  • clearer mapping to C R U D
  • No need to mix $ vs no $. Note how in read-bias we need to put $name etc. while here we can just do name

Pros of read-bias

  • <Model>.<field> is really succinct for the primary use-case?

@jasonkuhrt
Copy link
Member

jasonkuhrt commented Dec 6, 2021

Food for thought:

<Model>.<field>.<create | read | update>
const User = objectType({
  name: User.$name,
  description: User.$description,
  definition(t) {
    t.field(User.id.read);
    t.field(User.createdAt.read); // generates a NonNull field
    t.field(User.name.read); // generates a NonNull field
  }
})
const UserCreateInput = inputType({
  name: "UserCreateInput",
  definition(t) {
    t.field(User.id.create);
    t.field(User.createdAt.create); // generates a Nullable field
    t.field(User.name.create); // generates a NonNull field
  }
})
const UserUpdateInput = inputType({
  name: "UserUpdateInput",
  definition(t) {
    t.field(User.id.update);
    t.field(User.createdAt.update); // generates a Nullable field
    t.field(User.name.update); // generates a Nullable field
  }
})

I think my problem with this is how it reads. <Model> create <field> means "field for when the model is created". When the order is <Model> <field> create the user has to read the operation as relating to <Model> but it is visually most close to <field>.

Also, it aligns less well from a column perspective. In a given type def the C/R/U will never be mixed, so its ideal that they would be in prefix position.

@jasonkuhrt
Copy link
Member

$name and $description cannot be generated for input types

Hm, I think I disagree. We can have default values for both that users can opt into. For the description there can be a generic description about the operation followed by a copy of the model description. There can be a nice "title"/separation to indicate the split between those two sections. This could be a gentime settings toggle. Maybe opt-in to include the model info. It might get exhausting for API users to see that model doc over and over for every operation.

const User = objectType({
  name: User.$name,
  description: User.$description,
  definition(t) {
    t.field(User.read.id);
    t.field(User.read.createdAt); // generates a NonNull field
    t.field(User.read.name); // generates a NonNull field
  }
})
const UserCreateInput = inputType({
  name: User.create.$name,
  description: User.update.$description,
  definition(t) {
    t.field(User.create.id);
    t.field(User.create.createdAt); // generates a Nullable field
    t.field(User.create.name); // generates a NonNull field
  }
})
const UserUpdateInput = inputType({
  name: User.update.$name,
  description: User.update.$description,
  definition(t) {
    t.field(User.update.id);
    t.field(User.update.createdAt); // generates a Nullable field
    t.field(User.update.name); // generates a Nullable field
  }
})

@lvauvillier
Copy link
Contributor Author

@jasonkuhrt Interesting.

We can mix the "uniform" and "read-bias" using function/args. The default create case can be expressed without any arg:

const User = objectType({
  name: User.$name(),
  description: User.$description(),
  definition(t) {
    t.field(User.id());
    t.field(User.createdAt()); // generates a NonNull field
    t.field(User.name()); // generates a NonNull field
  }
})
const UserCreateInput = inputType({
  name: User.$name("create"),
  description: User.$description("create"),
  definition(t) {
    t.field(User.id("create"));
    t.field(User.createdAt("create")); // generates a Nullable field
    t.field(User.name("create")); // generates a NonNull field
  }
})
const UserUpdateInput = inputType({
  name: User.$name("update"),
  description: User.$description("update"),
  definition(t) {
    t.field(User.id("update"));
    t.field(User.createdAt("update")); // generates a Nullable field
    t.field(User.name("update")); // generates a Nullable field
  }
})

@jasonkuhrt
Copy link
Member

Oh that's interesting too. I tend toward favouring static where appropriate, since more data oriented that way. I also wonder if we'd find another use-case for field parameters later (e.g. customize rejectOnNotFound for a relation field).

@lvauvillier
Copy link
Contributor Author

Relations will also need an entry point to inject arguments (for filtering, ordering, pagination, etc.). These are more high level abilities but we need to keep it in mind. This design can be a tradeoff between DX and extendibility.

@jasonkuhrt jasonkuhrt pinned this issue Dec 15, 2021
@andrewicarlson andrewicarlson unpinned this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feat Add a new capability or enhance an existing one
Projects
None yet
Development

No branches or pull requests

3 participants