Nest: RFC: Replace class-transformer and class-validator

Created on 18 Mar 2019  路  13Comments  路  Source: nestjs/nest

I'm submitting a...

[X] Feature request

Hi guys,

this is rather a RFC than a feature request, so bear with me please. I'm the author of the FOSS library Marshal.ts which I have used now for over a year. At the beginning I was using NestJS with class-transformer but due to many pain points and serious issues with Typescript's strict mode, I had to ditch it.

Current limitations

First, please let me explain current limitations of class-transformer which decreases efficiency, type guarantees, and hurts usability:

1. No strict mode

Technically, you should always activate strict mode in tsconfig.json if you work on something more serious. The reason is simple: It forbids you to implicitly have type any etc. So let's take a look at a very common use-case that is also mentioned in the NestJS Serialization documentation:

export class UserEntity {
  id: number;
  firstName: string;
  lastName: string;

  @Exclude()
  password: string;

  constructor(partial: Partial<UserEntity>) {
    Object.assign(this, partial);
  }
}

With strict mode enabled this won't compile, because all fields are defined as required and none is guaranteed to be set in the constructor.

With strict mode disabled, this will compile fine. However, the usability is unclear and type checking is basically disabled. Why? Let's see how we can legally use that UserEntity:

const user = new UserEntity({});
user.id === undefined; //oops
user.name === undefined; //oops

Our fields id, firstName, lastName, and password have all a type defined. However, the example above returns for every field undefined. This leads not only to weird runtime bugs, but also is bad practice since you don't know what constructor values are necessary during build-time to build a valid UserEntity. Also it's completely unclear how a valid UserEntity looks. Is id required or can it be undefined, how about name, etc? Not clear at all. If you assume all can be undefined, then you get basically no help from the type system in your code when you operate potentially on an entity field with undefined value.

Better is an entity like that:

export class UserEntity {
  @Exclude()
  password?: string;

  constructor(
        public readonly id: number;
        public firstName: string,
        public lastName: string;
) {}
}

Here it's crystal clear how to use UserEntity and the class itself made sure that the internal state is always correct by forcing the user to provide required data via constructor.
However, this is not possible in class-transformer. This leads basically to the conclusion that you can not activate strict mode in NestJS projects, which I find very disappointing considering the fact that NestJS is targeted at professionell users as well.

2. Can't share DB entities and frontend entities.

Currently, if you have a UserEntity like above and want to use it in your frontend and database as well, you have to basically to write it twice. First with TypeORM decorators and second with class-transformer decorators. This makes stuff more complicated than it needs to be. The more complex my entity gets and the more I have, the more difficult it is to manage all those duplicates and to make sure all field names and types are the correct ones.

In a ideal world, you have one Entity definition and use it in your Frontend, HTTP Transport, Backend, and Database. With class-transformer this is not possible.

3. No Patch support

The bigger your model gets, the more optimisation opportunities you get. But with class-transformer you are here out of luck as well. If you want to implement a JSON patch protocol and/or controllers that issue mongo patch commands, you need to define here again an own Entity or manually transform the data to the correct target type.

4. No MongoDB support

Let's face it, it's super simple to user MongoDB with NestJS and any other Node server. That's why I often use it. However, with class-transformer you have no way to decorate the _id field, which is used in MongoDB for every record. Also UUIDs (stored as Binary in MongoDB) is not supported. So you end up having in all your controller class a manual serialization of _id and uuids fields back and forth. This is very ugly and should really not be necessary.

5. No Parent references

I have on product that is basically the SketchApp in the browser, using NestJs and Angular. There I have dozens of models and the main one are pretty big with certain relations. One example simplified:

export class DocumentClass {
    @IDField()
    @MongoIdField()
    _id?: string;

    @Field()
    name?: string;

    constructor(@Field() public readonly name: string) {}
}

@Entity('PageClass')
export class PageClass {
    @UUIDField()
    id: string = uuid();

    @Field([PageClass])
    children: PageClass[] = [];

    @Field()
    @ParentReference()
    @Optional()
    parent?: PageClass; //not available in the database

    constructor(
        @Field(forwardRef(() => DocumentClass))
        @ParentReference()
        document: DocumentClass, //not available in the database
        @Field() public readonly name: string) {
    }
}


const doc: DocumentClass = plainToClass(DocumentClass, {
    name: 'myDoc',
    pages: [
       {
           name: 'Page 1', 
           children: [{name: 'Page 1.1'}]
       },
       {name: 'Page 2'}
    ]
});
doc.pages[0].name === 'Page 1';
doc.pages[0].children[0].name === 'Page 1.1';
doc.pages[0].children[0].parent === doc.pages[0];
doc.pages[0].children[0].document === doc;

This use-case is not possible in class-transformer, yet definitely a common use-case in bigger applications.

5. some other small issues I personally don't like about class-transformer

No binary support, Data types require ugly @Type(() => Date) created: Date everywhere, no out-of-the-box Enum support, need separate package for validation (namely class-validator), and that per default all fields are serialized, which makes it necessary to mark private fields with ugly _ prefix (which breaks _id of Mongo btw).

Solution

Marshal brings basically solutions to all of the points above, while having same test coverage and battle proven code.

One of the main points is basically to have ideally one entity definition for all use-cases, frontend usage, validation, http transport, backend, and database. By decoupling the metadata (field types, indices) from the consumers, you can use the entity with all of its decorators in your frontend without pulling in typeorm as dependency, while having in your backend TypeOrm thatuses a generated EntitySchema from your entity, so you end up having only one entity. In some use-cases you create a child class with additional information to separate private database fields from the frontend, however you still have no duplicates here.

Second main point is allowing to have strict mode enabled, which is for all projects a must-have.

Also worth noting is that Marshal is 4-6x faster than class-transformer.

In addition, I already started supporting NestJS at https://marshal.marcj.dev/modules/_marcj_marshal_nest.html which allows users to validate incoming data and serialize it automatically (namely ValidationPipe). However, I this is not yet feature complete (ClassSerializerInterceptor is missing).

Question

Here comes my question: Are you generally interested? I can prepare Pull-Requests to the main repo + documentation repo in order to have a smooth transition.

Most helpful comment

but you still have all the goodies and huge performance improvements Marshal bringts to the table

I agree here. My complain is only about correct motivation for this decition that can just lead people the wrong way. Anyway Nest says it is unopinionated and replace is just few lines in your project that can be easily shared as an extension.

This I don't understand. You don't have that with class-transformer. Only Marshal bringts you strict-mode compatibility of models. See first point of the issue.

Let me explain. The main cause of your problem with strict mode isn't class-transformer or typescript at all. It is just because you don't use correct types to explain your domain field constraints.

export class UserEntity {
  @Exclude()
  password?: string;

  constructor(
        public readonly id: number;
        public firstName: string,
        public lastName: string;
) {}
}

You made password optional, but why? In database it is required field. As a result if you have const user: User you can not rely on compiler to help you with missed password field.

When you will pass user.password to libsodium function hash(pwd: string) compiler will complain about types and you will need to force bad casting or runtime null checking.

The same with id. It always present if you read entity, but you don't need it in POST request or when you save entity first time.

Some additional examples to help you:

  1. Usually you don't store password or hash in Users table. For example, you may want to store it in Credentials table. In this case your presentation model requires additional required field that isn't present in your persistence model. You will need to perform runtime null-checking in presentation level.
  2. Database field message can store encrypted message as binary type, but you POST messages as plain text.

All 13 comments

Why it was clone closed ? Make sense to me and I have basically the same questions. ..

i'm agree that is need to use something more flexible and ready for high load, Marshal it's a good way to improve DTO layer part

@marcj I don't have any issues.

  1. Use validator, when you create a new object, it's super simple. You don't need to use constructor for entities, use ValidationPipe, it converts objects to your entities and will use validation, so you never get unexpected undefined.
  2. Put typeorm configs for entities in separate files, so you could reuse the same classes
  3. Patch working perfectly for me. Just make a DTO like this:
export class EditUserDto {
  @MinLength(2)
  @IsOptional()
  public name: string

  @MinLength(5)
  @IsOptional()
  public password?: string
}

and then you can use it:

  public async edit(id: number, data: EditUserDto): Promise<User> {
    const user = await this.userRepository.findOne(id)

    if (user === undefined) {
      throw new UserNotFoundException(id)
    }

    Object.assign(user, data) // Patching is here
    this.userRepository.save(user)

    return user
  }

I can't help with mongodb issues unfortunately because I didn't use it much

@marcj I'm curious as to why you closed this issue?
Is marshal.ts still a good drop in replacement for class-transformer and class-validator?

Marshal.ts is still the excellent (meanwhile up to 10x faster) alternative to class-transformer, but it's not a drop-in-replacement. I closed because I don't use NestJS anymore and thus have no priority in replacing class-transformer w/ Marshal in Nest's code.

@xorik,

  1. You have to use the constructor in strict mode, otherwise you play with unhandled undefined types everywhere which is bad practise and unacceptable in professional code.

  2. I don't want to do the job twice nor do I want to separate that in two different files.

  3. That's not a patch what you showed, that's a simple update, which requires to retrieve the full object first, then apply your changes as "patch", and at the end sending it again completely to the database. It's slow and inefficient, for bigger entities unacceptable - and requires you to write yet another DTO class which duplicates the fields of the original entity class, which generates unnecessary code and complexity. Btw, your given class code doesn't compile in strict mode.

But I got your back: All of that is fixed in Marshal.

One of the main points is basically to have ideally one entity definition for all use-cases, frontend usage, validation, http transport, backend, and database.

Just comment that it is non-optimal approach for the most of more-less complex solutions. Your persistent model is just different from your presentation model. And PUT and POST bodies are also different for the most non-trivial use cases. If you want efficient typing and maintenance you need to define different models just because they describe different things.

Of course, if they describe different things, then they are different models. But let's be honest most of the time they do not describe different things. They describe variations. No need to write then everything twice. Especially not if you throw PUT/POST use-cases into the room. If you for example have a regular model on server-side and drop the ID, relations, and timestamps fields in your POST DTO, and in your PUT same but including the ID again, then those do not describe different things. They describe the same model, with slightly variants. Those use-cases are exactly where Marshal shines. You create a base-class and use inheritance to remove code-duplication. In the class-transformer world you essentially write 3 entire classes, so you end up having all field definitions 3 times. A hell to maintain if you rename one or change field type slightly. Plus, if your deal with binary data or Mongo's ObjectID you additionally have to convert manually between those classes.

So, it's actually quite the opposite, Marshal was exactly made for the most-common uses cases you just described and was made out of the demand for very complex apps. I use it for my complex applications where you deal with hundred of models and you definitely don't want to have field definition duplications. For example given model with 500 LOC, you don't want to write that definition twice just to remove the job::ID field in a POST DTO respectively, that would be crazy, but necessary in class-transformer&typescript world.

A hell to maintain if you rename one or change field type slightly.

I understand this point, but here are obvious disadvantages that prevents me to follow this way.

Sometimes i need to re-structure database, but keep my API backward compatible or maintain multiple versions of API and such small renaming on entity class just breaks my system.

I want to auto-generate API documentation based on presentation model definition, but with your approach can't as i have a lot of garbadge in models.

I don't want to share my backend internals with frontends.

I often have presentation entities that a based on multiple internal entities stored in different databases.

I want to have strict compile-time null-checks in my code without null-checking boilerplate that is necessary if i share models.

Sometimes i need to use GraphQL and related tools that can't generate correct code based on entities model.

I decided to go with @marcj/marshal.ts for https://github.com/venobo/blazar aswell so +1

Only thing missing is more built in validators.

I understand this point, but here are obvious disadvantages that prevents me to follow this way.

Fair enough. In worst case scenario you have the same boilerplate as with class-transformer, but you still have all the goodies and huge performance improvements Marshal bringts to the table. So, even in your cases a huge win.

I want to have strict compile-time null-checks in my code without null-checking boilerplate that is necessary if i share models.

This I don't understand. You don't have that with class-transformer. Only Marshal bringts you strict-mode compatibility of models. See first point of the issue.

but you still have all the goodies and huge performance improvements Marshal bringts to the table

I agree here. My complain is only about correct motivation for this decition that can just lead people the wrong way. Anyway Nest says it is unopinionated and replace is just few lines in your project that can be easily shared as an extension.

This I don't understand. You don't have that with class-transformer. Only Marshal bringts you strict-mode compatibility of models. See first point of the issue.

Let me explain. The main cause of your problem with strict mode isn't class-transformer or typescript at all. It is just because you don't use correct types to explain your domain field constraints.

export class UserEntity {
  @Exclude()
  password?: string;

  constructor(
        public readonly id: number;
        public firstName: string,
        public lastName: string;
) {}
}

You made password optional, but why? In database it is required field. As a result if you have const user: User you can not rely on compiler to help you with missed password field.

When you will pass user.password to libsodium function hash(pwd: string) compiler will complain about types and you will need to force bad casting or runtime null checking.

The same with id. It always present if you read entity, but you don't need it in POST request or when you save entity first time.

Some additional examples to help you:

  1. Usually you don't store password or hash in Users table. For example, you may want to store it in Credentials table. In this case your presentation model requires additional required field that isn't present in your persistence model. You will need to perform runtime null-checking in presentation level.
  2. Database field message can store encrypted message as binary type, but you POST messages as plain text.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hackboy picture hackboy  路  3Comments

menme95 picture menme95  路  3Comments

anyx picture anyx  路  3Comments

rlesniak picture rlesniak  路  3Comments

KamGor picture KamGor  路  3Comments