Skip to content
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

also emit type arguments with --emitDecoratorMetadata #3015

Closed
hypno2000 opened this issue May 3, 2015 · 20 comments
Closed

also emit type arguments with --emitDecoratorMetadata #3015

hypno2000 opened this issue May 3, 2015 · 20 comments
Assignees
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@hypno2000
Copy link

At the moment:

class Article {
   @Relation
   author: OneRelation<User>;
}

emits:

__metadata('design:type', OneRelation)

My suggestion is that that instead of just type it would also include the type args.

Maybe something like this:

__metadata('design:type', {type: OneRelation, args: [{type: User, args: []}])

With the latter i don't have to repeat myself in decorator that this is a one relation to user as i have already specified this in type.

Thanks,
Reio

@mhegazy
Copy link
Contributor

mhegazy commented May 3, 2015

How are using the type information later on? Generics do not really have a runtime representation. So other than Array, how would the type parameter be used?

Also what about generic classes, how should OneRelation be represented in class Article.

@hypno2000
Copy link
Author

declare class OneRelation<T> {
   fetch(): T;
}

@Collection('users')
class User {
    email: string;
    username: string;
}

In this case the type parameter would be used by the model framework to hook up the correct model (User) to this relation. For example article.author.fetch() instanceof User would be true. Model framework would need to load the data from correct store and instantiate a correct class. Its a runtime functionality.

At the moment i would have to repeat myself to let the framework know the target model like so:

@Collection('articles')
class Article {
   title: string;
   content: string;

   @Relation(User)
   author: OneRelation<User>;

}

What i would like to do is avoid specifying the User twice and let the framework to be smart and infer it from the type argument:

@Collection('articles')
class Article {
   title: string;
   content: string;

   @Relation
   author: OneRelation<User>;

}

I don't see how the arguments passed to generics have any less runtime representation than any other types in typescript.

I realise that interfaces don't have runtime representation and they are just emitted like this:

__metadata('design:type', Object)

Which does not give much info tbh but thats a different subject.

I think emitting the type arguments would be useful as it makes possible the frameworks to infer more type info from the type itself rather than needing to duplicate it via decorator. I think thats the reason why the --emitDecoratorMetadata option was created.

@mhegazy
Copy link
Contributor

mhegazy commented May 3, 2015

EmitDecoratorMetadata emits values, and not serialized types. So it only works with classes and native types. This is because classes act as both a type at design time and a factory at run time (I.e. Constructor function). And native types have matching boxed types that can be used as constructors at runtime as well. Interfaces as you noted do not have this duality.

Generic types have no parallel at runtime, they are mearly a design time construct that allows for describing type relations. Similar to interfaces. The code sample above will assume something about the type OneRelationship, and in effect treats the generic type argument as an argument to the decorator.

I believe what you are asking for is a full type serialization thst is available at runtime. Got this I do not see why interfaces should be excluded. @rbuckton had a proposal for this; @rbuckton can you share your type serialization proposal here?

@rbuckton
Copy link
Member

rbuckton commented May 4, 2015

@mhegazy This gist contains some of my thoughts on a more comprehensive type serialization format.

@hypno2000
Copy link
Author

Full type serialization would be awesome and would definitely solve my case.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels May 5, 2015
@pavelsavara
Copy link

Full type information on runtime, specifically generic parameters, would probably allow to build smart factory methods. That assumes also decorated method calls. I guess lot could be learned from type erasure mistakes in Java.

@dsebastien
Copy link

+1 for full type information at runtime. There are many cool things that we'll be able to do with these.

@rbuckton
Copy link
Member

rbuckton commented Jul 7, 2015

@pavelsavara, @dsebastien: I've been considering this, but truly capturing full type information would require not only compiler support but a full runtime library. I have an updated gist with a tentative JSON schema for type information. There are still many issues to consider, such as the fact that TypeScript uses structural typing for interfaces and assignability, so its generally not enough to state "Inject an IMyService instance into this constructor parameter", if you have a class with the same shape that is not an IMyService instance.

In general, strings or symbols will likely always be a better choice for use cases like dependency injection as they are easier to work with, are more reliable, and don't have the runtime overhead of a runtime library for interpreting type relationships.

@pavelsavara
Copy link

As I said before, the IoC usecase could be satisfied with only type name emmited by compiler. Structural interface inference at runtime is not really necessary, rather nice to have. Do you plan to add support for interface name emmision for constructor parameters ? Or perhaps add extension points to the compiler service to enable it ?

@mhegazy
Copy link
Contributor

mhegazy commented Jul 10, 2015

I do not think we will emit type names, just because they do not make much sense in a structural type system. emitter extensibilty is something that is on our radar.

@meirgottlieb
Copy link

@rbuckton when considering a format for JSON type information and a library to work with it, take a look at these two projects:

Generates type information in JSON format:
https://www.npmjs.com/package/tsreflect-compiler

Library for working with type information in Node:
https://www.npmjs.com/package/tsreflect

@marinasundstrom
Copy link

I see type metadata emission as useful when wanting to infer things at runtime, for example generating a data model or serialize data. I really hope that the community can come up with a proposal that also works with decorators in cases where there are no type annotations available, like in standard ECMAScript.

@RyanCavanaugh
Copy link
Member

My notes say "Assign to Mohamed"

@mhegazy
Copy link
Contributor

mhegazy commented Aug 5, 2015

The current design leverages JavaScript objects that exist at runtime. this works for classes, since they exist both in the type-space at compile-time and in the value-space at run-time. interfaces do not have these properties, nor do type parameters.

Moreover, the name of an interface, or a type parameter by itself is not sufficient information in a structural type system. You need the shape of the type to be able to make any decisions.

I definitely see the value in a full reflection system, that allows for querying types at runtime, asserting them, detecting compatibility using the same rules as the compiler use at design-time etc.. I would see this implemented through a JSON-like serialization mechanism similar to @rbuckton's proposal, and a runtime library that allows for querying these types and verifying assignability, subtype, and identity relationships, and figuring out type parameters, and constraints.. etc..

This however is a big undertaking that is out of scope for typescript at the current time.

@mhegazy mhegazy closed this as completed Aug 5, 2015
@mhegazy mhegazy added Out of Scope This idea sits outside of the TypeScript language design constraints Too Complex An issue which adding support for may be too complex for the value it adds Declined The issue was declined as something which matches the TypeScript vision and removed In Discussion Not yet reached consensus labels Aug 5, 2015
@pcan
Copy link
Contributor

pcan commented Aug 6, 2015

@mhegazy I understand that this is out of scope for TypeScript compiler itself, but it could expose at least some API that describe the type system used internally for type checking during compilation, so a third party post-compiler could implement a full-blown reflection system along with a runtime library (maybe using the JSON structure proposed by @rbuckton).

@mhegazy
Copy link
Contributor

mhegazy commented Aug 6, 2015

You should be able to use the TypeChecker API from the compiler to get to the type information you need. there is no serialization logic readily available, but should be possible to add one.

Here is the TypeChecker API: https://github.com/Microsoft/TypeScript/blob/085f0df4556801c14a4333d6c1d0bab1375586e4/lib/typescript.d.ts#L1027-L1057

Documentation on using the compiler API: https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API

Here is the part of the emitter that emitts this infromation today using the Checker:
https://github.com/Microsoft/TypeScript/blob/085f0df4556801c14a4333d6c1d0bab1375586e4/src/compiler/emitter.ts#L5065-L5104

@goloveychuk
Copy link

goloveychuk commented Apr 22, 2017

@mhegazy as I understood, it's impossible to override this function
https://github.com/Microsoft/TypeScript/blob/43d16773cc11523e974718e54ad1b1f9cc75f1d3/src/compiler/transformers/ts.ts#L1500
using Compiler Api?
So only option is to write a tool, which will execute separately.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 24, 2017

you could write your transform that transforms the decorator as a whole; but meta-data only is not overridable at the moment.

@goloveychuk
Copy link

goloveychuk commented Apr 24, 2017

@mhegazy
so it's possible to override
https://github.com/Microsoft/TypeScript/blob/43d16773cc11523e974718e54ad1b1f9cc75f1d3/src/compiler/transformers/ts.ts#L1300
Could you please tell how, because I can't find docs about this.
Currently, I wrote webpack loader to emit metadata, but it's very hacky, since it appends some ts code to original one.
https://gist.github.com/goloveychuk/81ff1125d0b079a1faa87aa546bb9dd2

update:
ok, I understood. I can update class with updateClassDeclaration, add there DecoratorEmitHelper and then emit result. Still need to integrate it with awesome-type-loader, which, looks like, uses typescript server api.

@goloveychuk
Copy link

goloveychuk commented Apr 28, 2017

tried to write before transformer, which adding decorator for classes and properties.
https://gist.github.com/goloveychuk/77b924debd948dafd7b53e6d41e48946#file-transformer-ts
if I insert directly to node.decorators - it emits decorator.
But if I creating new decorators array and new classDeclaration - I'm seeing error
https://gist.github.com/goloveychuk/77b924debd948dafd7b53e6d41e48946#file-err-trace

switch (location.kind) {
      case 264 /* SourceFile */:
            if (!ts.isExternalOrCommonJsModule(location))
             break;
            isInExternalModule = true;
      case 232 /* ModuleDeclaration */:
            var moduleExports = getSymbolOfNode(location).exports;
            //getSymbolOfNode(location) is undefined there. 
           // Location - sourcefile
           // getSymbolOfNode(location.original) returns symbol
            if (location.kind === 264 /* SourceFile */ || ts.isAmbientModule(location)) {

Maybe this api didn't supposed to modify nodes (before typescript transformer)?

upd:
fixed with

    (<any>newNode).symbol = (<any>source).symbol;

but this is really hacky, are there any ts function to do this?

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

10 participants