Skip to content

Feature request: Analyze class decorators for modifications of the decorated class's prototype #8250

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

Closed
frederikschubert opened this issue Apr 22, 2016 · 8 comments

Comments

@frederikschubert
Copy link

When decorating a class and adding properties to the class's prototype the compiler currently does not present the added properties in the auto-completion.
It would be nice if this feature would be implemented because right now to use such a decorator in a type-safe way one has to define an interface that the decorated class has to implement.

@frederikschubert frederikschubert changed the title Feature request: Analyze class decorators for modifications of the decorated class Feature request: Analyze class decorators for modifications of the decorated class's prototype Apr 22, 2016
@kitsonk
Copy link
Contributor

kitsonk commented Apr 22, 2016

Essentially covered by #4490 and related #2919, #4038, #4302 and potentially addressed via TypeScript 2.1 Roadmap item #2900.

@frederikschubert
Copy link
Author

Oh, I wasn't aware of these issues. Thanks! Closing as dup.

@felixfbecker
Copy link
Contributor

I read through all these issues and imo this is a seperate issue. Per the documentation, a class decorator can return a new class that replaces the definition:

function myDecorator<T>(constructor: T): T & {staticMethod(): string} {
  constructor.staticMethod = function(): string {
    return 'hello';
  }
  return constructor;
}

But the return type is not used:

@myDecorator
class Hello { }

Hello.staticMethod() // method staticMethod() does not exist on type Hello

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2016

That is an interesting point. Currently, there is no type analysis/mutation done on decorators, even ones that can return a value. I had always assumed design-time/ambient decorators would be how this would be solved, but you are right, the type analysis could be done like that.

@rbuckton / @mhegazy was there a reason why the return types from decorators aren't used in the type analysis?

@felixfbecker
Copy link
Contributor

felixfbecker commented May 12, 2016

This would be super useful for sequelize models:

const attributeOptions = Symbol('attributeOptions')

function Attribute(attribute: string | Seq.ABSTRACT | typeof Seq.ABSTRACT | Seq.DefineAttributeColumnOptions) {
    return Reflect.metadata(attributeOptions, attribute);
}

function Model<T>(sequelize: Sequelize.Connection, options: Sequelize.DefineOptions<T>) {
  return function Model<U extends Function>(Model: U): Sequelize.Model & U {
    const attributes: Seq.DefineAttributes = {};
    if (!options.instanceMethods) {
        options.instanceMethods = {}
    }
    if (!options.classMethods) {
        options.classMethods = {}
    }
    if (!options.getterMethods) {
        options.getterMethods = {}
    }
    if (!options.setterMethods) {
        options.setterMethods = {}
    }
    for (const key in Model.prototype) {
        if (typeof Model.prototype[key] === 'function') {
            // method
            options.instanceMethods[key] = Model.prototype[key]
        } else {
            const descriptor = Object.getOwnPropertyDescriptor(Model.prototype, key)
            if (descriptor.get) {
                options.getterMethods[key] = descriptor.get
            }
            if (descriptor.set) {
                options.setterMethods[key] = descriptor.set
            }
            if (Reflect.hasMetadata(attributeOptions, Model)) {
                // property
                attributes[key] = Reflect.getMetadata(attributeOptions, Model.prototype, key)
            }
        }
    }
    // static
    for (const key in Model) {
        if (typeof Model.prototype[key] === 'function') {
            // method
            options.classMethods[key] = Model.prototype[key]
        }
    }
   const SequelizeModel = sequelize.define(Model.name, attributes, options);
   return <Sequelize.Model<U, U> & U>SequelizeModel;
  }
}

Usage:

const sequelize = new Sequelize(process.env.DB_CONNECTION)

@Model(sequelize, {tableName: 'users'})
class User {
  @Attribute({type: Sequelize.STRING, primaryKey: true})
  public username: string;

  static myCustomStaticMethod() {
    return 'hello'
  }
}

User.findOne() // should work
User instanceof Sequelize.Model // should be true
User.myCustomStaticMethod() // should work

@mhegazy
Copy link
Contributor

mhegazy commented May 12, 2016

@rbuckton / @mhegazy was there a reason why the return types from decorators aren't used in the type analysis?

This is an implementation limitation. the system as it is built today does not allow for mutating the shape of a declaration after it has been analyzed and bound. different parts of the compiler depends on this assumption. changing this would require some significant rejiggering.
Moreover most of these modifications are already possible using declarative forms. so the design we have adopted is to allow for dynamically changing the value at runtime, but limiting the type mutations at design time. the compiler checks that the return type is assignable to the initial type, so you can not remove properties for instance.
This desing seemed like a good place to be that allows a lot of runtime-depenednt scenarios, and does not require rewriting the compiler.

@felixfbecker
Copy link
Contributor

felixfbecker commented May 12, 2016

@mhegazy

Moreover most of these modifications are already possible using declarative forms.

How is it possible to type a decorator that adds a property to the class?

@mhegazy
Copy link
Contributor

mhegazy commented May 12, 2016

I meant declarative forms instead of the decorator (which I have consider imperative). so the declarative form of adding a property to a class would be a property declaration, the imperative form is a decorator that defines adds a property.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants