tannerntannern / ts-mixer

A small TypeScript library that provides tolerable Mixin functionality.
MIT License
384 stars 27 forks source link

Better API for multiple decorators (very low priority, since the API design is elegant enough 😁) #19

Closed kenberkeley closed 4 years ago

kenberkeley commented 4 years ago

First of all, thanks for your precious time and efforts 🙏🙏🙏

Extend https://github.com/tannerntannern/ts-mixer/issues/15#issuecomment-606242103

Status quo:

import 'reflect-metadata'
import { Mixin, decorate } from 'ts-mixer'
import { IsInt, IsNotEmpty, IsUrl, IsEmail, Min, Max } from 'class-validator'

class User {
  @decorate(IsInt())
  id!: number

  @decorate(IsNotEmpty())
  name!: string
}

class Profile {
  // Looks a bit verbose and repetitive 😢
  @decorate(IsInt())
  @decorate(Min(0))
  @decorate(Max(120))
  age!: number

  @decorate(IsUrl())
  avatar!: string
}

class UserProfile extends Mixin(User, Profile) {
  @IsEmail()
  email!: string
}

Proposal:

import 'reflect-metadata'
import { Mixin, decorate } from 'ts-mixer'
import { IsInt, IsNotEmpty, IsUrl, IsEmail, Min, Max } from 'class-validator'

class User {
  @decorate(IsInt())
  id!: number

  @decorate(IsNotEmpty())
  name!: string
}

class Profile {
  // Looks better 😂
  @decorate(
    IsInt(),
    Min(0),
    Max(120)
  )
  age!: number

  @decorate(IsUrl())
  avatar!: string
}

class UserProfile extends Mixin(User, Profile) {
  @IsEmail()
  email!: string
}
tannerntannern commented 4 years ago

I agree that it multiple decorators in a single call may look cleaner, but I'm not so sure it would be a good idea for a few reasons.

The main reason is that I think that the evaluation order could be misleading since decorators are evaluated "bottom-up". Consider the following:

import { decorate } from 'ts-mixer';
import { Foo, Bar, Baz } from 'some-library';

class MyClass {
    @decorate(
        Foo,
        Bar,
        Baz
    )
    public property1;

    @decorate(Foo)
    @decorate(Bar)
    @decorate(Baz)
    public property2;

    @decorate(Baz)
    @decorate(Bar)
    @decorate(Foo)
    public property3;
}

Should property1 be equivalent to property2, or property3? I think what it should be is debatable. I could see someone thinking, "ok so when these get evaluated, the order will be Foo, Bar, then Baz, because that's the order I passed them in." But I could also see someone else thinking, "ok so when these get evaluated, the order will be Baz, Bar, Foo, since that's what the order would be if I didn't have to wrap them in @decorate"

Sure, I could just choose one and clarify it in the documentation, but by requiring that each decorator be wrapped individually removes the ambiguity.

If your goal is simply to reduce characters, you could do something like this:

import { decorate as d } from 'ts-mixer';
import { Foo, Bar, Baz } from 'some-library';

class MyClass {
    @d(Foo)
    @d(Bar)
    @d(Baz)
    public prop;
}
kenberkeley commented 4 years ago

Well, with import alias, it seems that I can close this issue 😂