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

Deprecating class-validator and class-transformer usage #8390

Closed
1 task done
tak1n opened this issue Oct 20, 2021 · 3 comments
Closed
1 task done

Deprecating class-validator and class-transformer usage #8390

tak1n opened this issue Oct 20, 2021 · 3 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@tak1n
Copy link

tak1n commented Oct 20, 2021

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

Hi 👋🏽

First thing, thx for making NestJS which is a great Framework in Node.js land.

Due to the recent security issues which arised in class-validator I had a short conversation with @jmcdo29 in regards to future plans with the built in ValidationPipe and other framework parts which heavily rely on class-validator and class-transformer (both seem to be somewhat in a limbo state in regards to maintenance).

After those conversations it seemed that we want to phase out c-v/c-t and replace it by something else which is better maintained (@jmcdo29 mentioned @deepkit/type and he aims to publish a package which contains a ValidationPipe leveraging mentioned lib).

Most likely this will lead to breaking changes and therefore should be released in the next major version of NestJS. I'm opening this issue to have the plans documented in a transparent way (or also discuss alternative ways).

Describe the solution you'd like

No clear solution from my side. Whether we go with @deepkit/type or try to fork c-v/c-t should be up for debate by the core team, also not sure if we should first try to reach out to the core devs of c-v/c-t.

Teachability, documentation, adoption, migration strategy

As already mentioned this probably needs to go into NestJS v9.

What is the motivation / use case for changing the behavior?

Recommending unmaintained libs leads to frustration of Nest users.

@tak1n tak1n added needs triage This issue has not been looked into type: enhancement 🐺 labels Oct 20, 2021
@micalevisk
Copy link
Member

This is related to these others issues:

#7814

#4553

(just linking them)

@jmcdo29
Copy link
Member

jmcdo29 commented Oct 20, 2021

To weigh in on this myself, I do think we (Nest) should move away from class-transformer and class-validator. What I'd love to see would be something like @nestjs/validation or @nestjs/marshaller that has the ValidationPipe and the ClassSerializataionInterceptor as two exported classes, similar to how Nest v8 separated out the HttpModule to be in @nestjs/axios. This would help break up some of the @nestjs/common library to not include so many extras as well, which is always a bonus.

As for what to do, I think generally the approach we've taken so far is the one to follow: let Nest focus on being about architecture and providing the basis for the framework; the dependency injection, the module resolution, the route mapping, etc. Then make use of existing libraries for providing these integrations, specifically in this case I'm thinking @deepkit/type like @tak1n mentioned (and for what it's worth, I'm going to be releasing a "third party" integration probably by the end of the week that's a drop in replacement (save for the decorators that are already in place).

I'm all for doing this as a part of v9. Possibly keep the built in ValidationPipe and ClassSerializationInterceptor around as deprecated for v9 and removed in v10 when we get to that and have new functionalities and the new pipe and interceptor in the separate package.

@kamilmysliwiec
Copy link
Member

I've already replied to similar issues:

We might need to fork these packages one day and just keep pushing security fixes by ourselves (potentially minor improvements as well). However, we will definitely not start using different packages risking that we can run into the same issue again in the future. Based on my experience with class-validator and class-transformer, I conclude that relying on external packages for such key features as validation makes no sense.

@nestjs nestjs locked and limited conversation to collaborators Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

4 participants