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

InputType classes are needed for both: Create and Update of entity because of nullable. #424

Closed
Brampage opened this issue Sep 18, 2019 · 8 comments
Labels
Community 👨‍👧 Something initiated by a community Duplicate 🔑 This issue or pull request already exists Enhancement 🆕 New feature or request

Comments

@Brampage
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When implementing the creation of an entity inside a resolver I find the need to create an InputType. This is perfectly fine. But I also want to implement the update for this entity, then I find myself duplicating the InputType and make some fields optional.

Describe the solution you'd like
Proposal/discussion: When adding the InputType using the Arg decorator, I add { nullable: true }. This would then make the fields within the InputType optional. This will however introduce a problem regarding nullable within the TypeScript InputType class.

I don't like the duplication just for the nullable fields (DRY).

Perhaps I'm just overlooking something..

Additional context
TaskCreate InputType

@InputType()
export class TaskCreateInput implements Partial<Task> {
  @Field()
  name: string;

  @Field()
  numberOfPersonsNeeded: number;

  @Field({ nullable: true })
  description?: string;

  @Field({ nullable: true })
  location?: string;
}

TaskUpdate InputType

@InputType()
export class TaskUpdateInput implements Partial<Task> {
  @Field({ nullable: true })
  name?: string;

  @Field({ nullable: true })
  numberOfPersonsNeeded?: number;

  @Field({ nullable: true })
  description?: string;

  @Field({ nullable: true })
  location?: string;
}
@Brampage Brampage changed the title InputType classes are needed for both: Create and Update of entity. InputType classes are needed for both: Create and Update of entity because of nullable. Sep 18, 2019
@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request labels Sep 21, 2019
@MichalLytek
Copy link
Owner

When adding the InputType using the Arg decorator, I add { nullable: true }. This would then make the fields within the InputType optional.

How in that case make the argument optional (nullable) but not the fields itself?

For the last few months I have a class operators in mind that works like the TS type operators (Parital, Required, Pick, Omit). But I have to check that and make a proof of concept first:

@InputType()
export class TaskCreateInput implements Partial<Task> {
  @Field()
  name: string;

  @Field()
  numberOfPersonsNeeded: number;

  @Field({ nullable: true })
  description?: string;

  @Field({ nullable: true })
  location?: string;
}

@InputType()
export class TaskUpdateInput extends Partial(TaskCreateInput) {}

@numToStr
Copy link

@InputType()
export class TaskCreateInput implements Partial<Task> {
  @Field()
  name: string;

  @Field()
  numberOfPersonsNeeded: number;

  @Field({ nullable: true })
  description?: string;

  @Field({ nullable: true })
  location?: string;
}

@InputType()
export class TaskUpdateInput extends Partial(TaskCreateInput) {}

I also ran into the same problem several times. It would be welcome feature for many of us.

@amille14
Copy link

For the last few months I have a class operators in mind that works like the TS type operators (Parital, Required, Pick, Omit).

@MichalLytek I would absolutely love this feature. I use type-graphql with typeorm and it would save so much time to be able to generate gql types based on a typeorm entity. I've been trying to implement it myself but keep hitting walls and have to fall back to manually creating types and keeping them up to date when my entities change. Ideally I would love to be able to replicate some of the typegen stuff that prisma does for you, but without being locked into using their "orm"/database interface.

It would also be nice to be able to map the keys of one type to new values in a new type. For example, I want to create an PostsOrderByInput type that allows me to specify "ASC" or "DESC" order enum for any of the fields on the Post type.

@MichalLytek
Copy link
Owner

Closing in favor of #453 🔒

@MichalLytek MichalLytek added Duplicate 🔑 This issue or pull request already exists and removed Discussion 💬 Brainstorm about the idea labels Oct 30, 2019
@nickyhajal
Copy link

What is the best practice for this at the moment? I find myself in this exact situation with copy/pasted InputTypes where the update just adds {nullable: true} and an id field. Is there no better way?

@smolinari
Copy link
Contributor

@nickyhajal - Did you follow/ read the discussion in the issue Michal pointed to above?

Scott

@nickyhajal
Copy link

Hey @smolinari, yep - it seems like @MichalLytek's last word on that thread is:

Be aware that the internal metadata storage might be changed without any notice between releases, so it's not recommended to do that kind of hacks.

But, your comment made me revisit more closely and I do see the main recommendation was using a mixin as shown here: https://github.com/MichalLytek/type-graphql/tree/master/examples/mixin-classes

What I'm not seeing in that example is how to make certain fields nullable in the UpdateInput, while required in the CreateInput. Is there something I'm missing here?

Thanks!

@MichalLytek
Copy link
Owner

@nickyhajal
Class mixins should be preferred instead of omit/pick approach, however for making types partial/required we still need #453 and its type operators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Duplicate 🔑 This issue or pull request already exists Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants