Class-transformer: feat: no property missing & type check in plain object

Created on 19 Sep 2016  路  10Comments  路  Source: typestack/class-transformer

When you receive from web client a JSON with user credentials and want to use it in login procedure (check in database), you use let user = plainToClass(User, userJson). The problem is that this function doesn't inform you (throw Error or sth) that the class property is missing in json. So you assert like in let user = JSON.parse(userJson) as User and might have undefined login or password.

It would be nice to have an field in function option (interface ClassTransformOptions) like throwErrorIfMissing: true or have decorators @Required or @Optional with different error strategy.
It could throw Error with message "Parsing error: property ${propertyName} is missing." which could be used in response JSON to REST API client.

The other problem is that doesn't check type of object property - you can have filename: true in photo JSON and you will get runtime error when try to use photo.filename as string. And it also copy everything from the plain object, but it's not as dangerous like the the above-mentioned.

You can see how it's all implemented in TypedJSON and when it throws error.

donreleased feature

Most helpful comment

This can be easily achieved by using this library together with class-validator.

Why forcing using validation library when all that user want is to ensure that the runtime object is matching declared class signature? No custom validation logic, only proper transforming and checking if all of the properties exist and have correct type.

We have 3.1 kB (MINIFIED + GZIPPED) library class-transformer and you claim that we have to use also class-validator which is 112.9 kB (MINIFIED + GZIPPED) just for this one case? It's ridiculous.

Also, I know that it's done in class-validator but there you have to annotate all properties with @IsString and other decorators. Here we have a type info that allow for proper transforming properties already.

This is out of scope for this lib.

I disagree with that. We should continue discussion at least. Closing issues in that way will result in forking this lib.

All 10 comments

It could throw Error with message "Parsing error: property ${propertyName} is missing." which could be used in response JSON to REST API client.

It has such exception before, but I removed that for some reasonable reason (or problem). I don't remember what was a reason, but I'll try to.

The other problem is that doesn't check type of object property - you can have filename: true in photo JSON and you will get runtime error when try to use photo.filename as string

Do you think exception should be thrown for this, or force type-casting should be applied?

I think we should have 2 methods for converting plain object to class:

  • neutral (like now), which only add methods for the object
  • strict, which should throw Error when property is missing or the type doesn't match, because we want to be sure that the object is safe to use

I am thinking about usage with routing-controllers (it's integrated, right?):

@Post("/login/")
loginUser(@Session() session: Express.Session, @Body() user: User) {
}

I would like to have valid user object with all properties with correct types, ready to use. And when the plain object parsing error occurs, it should return HTTP Error 400 with plain or json message which property is missing or wrong type.

@pleerock honestly, this makes this lib useless for deserialization purposes :/ This behavior should be noted in readme because basically all your fields in whole class should be marked as optional.

@cojack what do you think about this enhancement? Shall we set this for a future release?

@krzkaczor You can open a PR if you want. We will happily merge it.

honestly, this makes this lib useless for deserialization purposes.

This can be easily achieved by using this library together with class-validator.

@cojack what do you think about this enhancement? Shall we set this for a future release?

This is out of scope for this lib.

@krzkaczor You can open a PR if you want. We will happily merge it.

This is out of scope for this lib.

This can be easily achieved by using this library together with class-validator.

Why forcing using validation library when all that user want is to ensure that the runtime object is matching declared class signature? No custom validation logic, only proper transforming and checking if all of the properties exist and have correct type.

We have 3.1 kB (MINIFIED + GZIPPED) library class-transformer and you claim that we have to use also class-validator which is 112.9 kB (MINIFIED + GZIPPED) just for this one case? It's ridiculous.

Also, I know that it's done in class-validator but there you have to annotate all properties with @IsString and other decorators. Here we have a type info that allow for proper transforming properties already.

This is out of scope for this lib.

I disagree with that. We should continue discussion at least. Closing issues in that way will result in forking this lib.

Why forcing using validation library when all that user want is to ensure that the runtime object is matching declared class signature?

Because we have another library for that. I just wrote the required snippet for you:

import { validate } from 'class-validator';
import { plainToClass } from 'class-transformer';

export function transformAndValidate<T>(targetClass: typeof T, rawValue: any): T {
    return validate(plainToClass(targetClass, rawValue));
}

only proper transforming and checking if all of the properties exist and have correct type.

What I don't like about your proposal is that you want to ensure that a property has the correct type because this is not the job of the transformer this is the job of the validator. The transformer should just blindly transform the received value into the target type. Which means if you expect prop: number and receive a 'xxx' value then the transformer should simply call Number('xxx') and doesn't care about if it is valid or not. That is the job of the data validation layer.

We have 3.1 kB (MINIFIED + GZIPPED) library class-transformer and you claim that we have to use also class-validator which is 112.9 kB (MINIFIED + GZIPPED) just for this one case? It's ridiculous.

The latest release is that big only becuase the decorators are not defind in different files so everything ends up in the bundle and google-libphonenumber is big. The real size of the class-validator is 18kB. Once decorators gets sliced of it will be around 9kB also.

Also, I know that it's done in class-validator but there you have to annotate all properties with @IsString and other decorators. Here we have a type info that allow for proper transforming properties already.

Not true, we have type info only for properties which as decorators assigned as Typescript documentation states it: _TypeScript includes experimental support for emitting certain types of metadata for declarations that have decorators_.

So you still need to assign at least one decorator (ours or something else) for every property.

I just wrote the required snippet for you

Have you copied it from class-transformer-validator? 馃槅

So you still need to assign at least one decorator (ours or something else) for every property.

Yes but it's easier to maintain decorating with @Allow() or @TransformProp than @IsString(), @IsBoolean and @ValidateNested() and syncing with prop type.

What I don't like about your proposal is that you want to ensure that a property has the correct type because this is not the job of the transformer this is the job of the validator.

Why proper deserialization checks need validation lib? For me they have different responsibilities but both should have the "warn when no property/property missmatch" feature not relaying on each other.

So looks like there's a market niche for a library that will take care of proper deserializations with props existence and prop type checking 馃槙 It could use class-transformer and class-validator features together but under a simpler API.

This is possible for some time with excludeExtraneousValues option.

This issue 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

gregsolo-intent picture gregsolo-intent  路  5Comments

SC30SC picture SC30SC  路  4Comments

taemini picture taemini  路  5Comments

ERPedersen picture ERPedersen  路  4Comments

AckerApple picture AckerApple  路  5Comments