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

Idea: class-validator -> directive #322

Closed
j opened this issue Apr 25, 2019 · 2 comments
Closed

Idea: class-validator -> directive #322

j opened this issue Apr 25, 2019 · 2 comments
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request Out of scope ✖️ Proposal doesn't fit in project goals

Comments

@j
Copy link
Contributor

j commented Apr 25, 2019

I know directives aren't supported yet, but one sucky thing I've noticed is when you use class-validators and the front-end passes invalid data that fails for graphql input validation + class-validation errors, you'll get the graphql error first, then re-submit and get the class-validation error.

What would be cool is that if/when directives get supported, that the following could happen:

class Post {
  @Length(3, 50)
  @Field()
  title: string;
}

Would generate something like:

type Post {
  name: String! @ValidLength(3, 50)
}

Obviously, this is #goals, but just having directives would be a one up.

Even this would be cool:

class Post {
  @Field({ directives: "@constraint(minLength: 3, maxLength: 50)" })
  title: string;
}

And potentially allowing for:

Even this would be cool:

import { string, setup } from 'yup-type-graphql';

buildSchema({
  ...,
  directives: [setup()]
})

class Post {
  @Field({ directives: [string().min(3).max(50)] })
  title: string;
}
@MichalLytek
Copy link
Owner

the front-end passes invalid data that fails for graphql input validation + class-validation errors, you'll get the graphql error first, then re-submit and get the class-validation error

That's why you should validate your data first on the client side. And treat server validation only as a security feature for those who tries to call your mutations/queries manually.

I know directives aren't supported yet

I'm not sure if directives are evaluated just in time of validating the query against schema. Maybe you think about custom scalars?

yup-type-graphql

Yeah, I will provide integration for common libs and patterns + building blocks for making a custom ones. For now, class-validator support is hardcoded but with field metadata #124 and custom decorators #45 or middlewares, it's easy to provide this kind of integrations without relying on directives.

@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request labels Apr 28, 2019
@MichalLytek MichalLytek added this to the Future release milestone May 3, 2020
@MichalLytek
Copy link
Owner

Even this would be cool:

class Post {
  @Field({ directives: "@constraint(minLength: 3, maxLength: 50)" })
  title: string;
}

It's supported by #369

Even this would be cool:

import { string, setup } from 'yup-type-graphql';

buildSchema({
  // ...,
  directives: [setup()]
})

class Post {
  @Field({ directives: [string().min(3).max(50)] })
  title: string;
}

Agree but can be done by external libs like in this snippet.
You need to create standard GraphQL directives which are translated to class-validator or yup validation during directives execution.

There won't be any special built-in feature for that like for class-validator.

So closing this one 🔒

@MichalLytek MichalLytek added Out of scope ✖️ Proposal doesn't fit in project goals and removed Discussion 💬 Brainstorm about the idea labels Jan 4, 2021
@MichalLytek MichalLytek removed this from the Future release milestone Jan 4, 2021
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 Enhancement 🆕 New feature or request Out of scope ✖️ Proposal doesn't fit in project goals
Projects
None yet
Development

No branches or pull requests

2 participants