class-validator icon indicating copy to clipboard operation
class-validator copied to clipboard

question: Type safe property decorators

Open karlismelderis-mckinsey opened this issue 2 years ago • 6 comments

I spotted that Typescript is not protecting against wrongly applied Property decorators.

export class Dto {
  @IsNumber() // <<<< No error here
  index: boolean;
}

After some googling I came up with quick example how to overcome it.

declare type NumberPropertyDecorator = <T, K extends keyof T>(target: [T[K]] extends [number] ? T : never, propertyKey: K) => void;

export declare function IsNumber(options?: IsNumberOptions, validationOptions?: ValidationOptions): NumberPropertyDecorator;

Do you think it's possible to extend this example and make Property Decorators more type safe?

karlismelderis-mckinsey avatar Jan 27 '23 15:01 karlismelderis-mckinsey

This is quite cool! I can't think of any obvious pitfalls right now, and this seems like a very nice QoL feature. Has this been discussed previously, @NoNameProvided ?

braaar avatar Feb 01 '23 06:02 braaar

This solution doesn't work for decorated private and protected properties, as they are not in keyof T.

export class Dto {
  @IsNumber() // Argument of type 'Dto' is not assignable to parameter of type 'never'.ts(2345)
  private index: number;
}

As a solution, I would propose to solely check public properties:

declare type NumberPropertyDecorator = <
    T,
    K extends string | number | symbol
  >(
    // Private and protected properties are not in `keyof T`, therefore we are unable to type-check them
    target: K extends keyof T
        ? T[K] extends number
            ? T
            : never
        : unknown,
    propertyKey: K
) => void;

Note: I also replaced [T[K]] extends [number] with T[K] extends number as I wasn't able to see the reasoning behind the additional wrapping tuple.

Dassderdie avatar Feb 06 '23 12:02 Dassderdie

I'm happy to pick this up if we can agree that it make sense and @Dassderdie proposal is valid

karlismelderis-mckinsey avatar Feb 10 '23 09:02 karlismelderis-mckinsey

Validating only public properties would be totally acceptable, there's not much sense validating privates anyway imho. This work would greatly help me (related to #1951, I didn't see this issue first). I'm not sure how to make it work with IsOptional but pretty sure it's as much possible as validating the type.

Also it would be totally broken by pending features like #689.

Have to make sure this feature is actually expected and will be accepted because I feel like it's some work though...

nicolaschambrier avatar Mar 03 '23 10:03 nicolaschambrier

TypeScript 5.0 was finally released a couple weeks ago, and one of the biggest new features it brings is a great implementation of the Stage 3 Decorator Proposal. I highly recommend making the switch from the legacy style - assuming it's a realistic scenario for your project.

Along with a bunch of runtime improvements, the new API comes very well typed right out of the box. Check out the lib.decorators.d.ts file to see what I mean.

So far the only downside I've found is there is currently no support for parameter decorators. I'm not sure if that's in the roadmap for either the TypeScript or TC39 teams, but I'm fine with sacrificing that feature as the cost of upgrading.

nberlette avatar Mar 29 '23 13:03 nberlette

So far the only downside I've found is there is currently no support for parameter decorators. I'm not sure if that's in the roadmap for either the TypeScript or TC39 teams, but I'm fine with sacrificing that feature as the cost of upgrading.

Does that affect this project at all? Our decorators are used for class properties only, as far as I understand.

braaar avatar Apr 11 '23 04:04 braaar