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

Decorator Metdata: expose array element type #7169

Open
thomas-mindruptive opened this issue Feb 20, 2016 · 45 comments
Open

Decorator Metdata: expose array element type #7169

thomas-mindruptive opened this issue Feb 20, 2016 · 45 comments
Labels
Domain: Decorators The issue relates to the decorator syntax Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@thomas-mindruptive
Copy link

TSC 175, target ES6, Node 5.6

Sample class with decorated properties:

@sg.DMClass
export class User implements IUser {
    @sg.DMProperty
    firstName: string;
    @sg.DMProperty
    lastName: string;
    @sg.DMProperty
    friends: User[];
}

Property decorator:

export function DMProperty(target: any, key: string) {
    var type = Reflect.getMetadata("design:type", target, key);
}

Current behaviour:
When the decorator gets called for property "friends", "type" is "Array" and the captured type ("User") is lost.

Expected behaviour
"type" should contain the captured type ("User"), too

Following very dirty work-around does the job: I modified tsc's method "emitSerializedTypeNode":

function emitSerializedTypeNode(node) {
    if (node) {
        console.log("*** emitSerializedTypeNode: node.kind: ", node.kind);
        switch (node.kind) {
            ...
            case 157:
                // Begin dirty ---------------------------------------------------------------------------
                var _elemType = node.elementType.typeName.text;
                write("{name: 'Array<" + _elemType + ">', type:'Array', elemType:'" + _elemType + "'}");
                console.log("*** emitSerializedTypeNode: Array, type: ", node.elementType.typeName.text);
                // End dirty -----------------------------------------------------------------------------
                return;
            case 150:
            ...

Best
Mind

@mhegazy
Copy link
Contributor

mhegazy commented Feb 24, 2016

looks like a duplicate of #4521

@mhegazy mhegazy closed this as completed Feb 24, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Feb 24, 2016
@thomas-mindruptive
Copy link
Author

Hi Mohamed,
sorry to bother you again. I don't think it's a duplicate of 4521. 4521 deals with the order of loading the modules, respectively circular refernces. That's not a problem in my case. All the modules are loaded correctly but as I can see from the compiler code (emitSerializedTypeNode), the captured type of a generic array is not emitted. Only "Array" gets emitted. So this behaviour is not (yet) in the compiler's code.
Best
Mind

@mhegazy mhegazy added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed Duplicate An existing issue was already created labels Feb 24, 2016
@mhegazy mhegazy reopened this Feb 24, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 24, 2016

thanks. changing the title to reflect the request.

@mhegazy mhegazy changed the title Metdata/Reflection on generics: TSC should emit the captured type Decorator Metdata: expose array element type Feb 24, 2016
@thomas-mindruptive
Copy link
Author

Thanks a lot!
Mind

@abierbaum
Copy link

Running into the same issue here. Would be very helpful if we could get to the full nested type information.

@DanielTWalther
Copy link

This problem apparently still persists. And it really is a dealbreaker for one of my current projects. In an attempt to implement a workaround of my own, much like and inspired by @mindruptive , I created a fork of the TS Compiler, which currently achieves JS outputs along the lines of e.g.:

__metadata('design:type', {name: 'Array<Array<Number>>', type: Array, elemType: {name: 'Array<Number>', type: Array, elemType: Number}})

or

__metadata('design:type', {name: 'Array<InnermostTestClass>', type: Array, elemType: TestClass_1.InnermostTestClass})

for (nested) Arrays as well as lessen the condensing of UnionTypes like so:

__metadata('design:type', Boolean | Number | String)

I realize that this workaround, which is sufficient for said project, creates a breaking change to the behavior of metadata, as (e.g.) the return parameter of Reflect.getMetadata('design:type', target, key?) is no longer guaranteed to be a constructor.
My question (and request) now is: can/will this be resolved?

@DanielTWalther
Copy link

It seems as though there is actually some progress regarding this issue. From what I can tell after looking at the current TSC, there seems to be a modified implementation of the metadata decorators (and two separate instances of said decorator emitters (function addOldTypeMetadata and function addNewTypeMetadata); the new one (when enabled) currently emits javascript code like this:

__metadata("design:typeinfo", {
  type: () => Array
})

I'd suggest modifying the output to emit:
(e.g.: number[][])

__metadata("design:typeinfo", {
  type: () => Array,
  elementType: () => {
    type: () => Array,
    elementType: () => Number
    }
})

This approach is obviously more elegant than building custom constructor functions (with or without a class or object mantle) to inject the elementType into.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 30, 2016

I should note that Decorators have a new proposal in TC39 that should facilitate adding additional meta-data fields such as this one. I would expect TypeScript to implement the new proposal in the coming releases.

@DominicBoettger
Copy link

Are there any informations when design:typeinfo will be available?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 5, 2016

Are there any informations when design:typeinfo will be available?

No updates since #7169 (comment).

@abierbaum
Copy link

@mhegazy Do you know of any workaround that can be used in the meantime? This is a major pain for a couple of our projects.

@DominicBoettger
Copy link

I think a workaround is only possible by defining the type in the decorator manually.

@mhegazy mhegazy added the Domain: Decorators The issue relates to the decorator syntax label Dec 28, 2016
@thomas-mindruptive
Copy link
Author

@DominicBoettger @mhegazy @abierbaum
Since I still need this feature (I create a query-schema from TypeScript metadata) and I don't want to add unnecessary type info to my decorator, I patched the current version of tsc again:

function serializeTypeNode(node) {
	...
	case 163:
	case 164:
		// MR patch -------------------------------------
			// Patch comment out: return ts.createIdentifier("Array");
			console.log(">>> serializeTypeNode: " + node.kind + ", elementType.typeName.text: " + node.elementType.typeName.text );
			// Pass the name of the captured type to "createIdentifier()"
			return ts.createIdentifier("Array", node.elementType.typeName.text);
		// MR end patch --------------------------------   
}
    
// MR patch: add paramter "elemType"
function createIdentifier(text, elemType) {
	var node = createSynthesizedNode(70);
	node.text = ts.escapeIdentifier(text);
	// MR patch -----------------------
		if (elemType) {
			node.text = "{type:" + node.text + ", elemType:'" + elemType + "'}";
		}
	// MR end patch ------------------
	node.originalKeywordKind = text ? ts.stringToToken(text) : 0;
	node.autoGenerateKind = 0;
	node.autoGenerateId = 0;
	return node;
}

Finally to use the generated metadata in my property decorator:

function DMProperty(target: any, key: string)
	var type = Reflect.getMetadata("design:type", target, key);
	if (type.type && (type.type === "Array" || type.type.prototype instanceof Array || type.type === Array)) {
		// Do something with the type info, in our case: Save it to a schema property of the class.
		target[Schema].arrayTypes[key] = type.elemType;
	}
	...
}

From a compiler designer's perspective this might be dirty. TypeScript is great and the static type info makes any kind of schema-generation much easier. This slight enhancement even more so. :-)

@thomas-mindruptive
Copy link
Author

   function serializeTypeNode(node) {
        ...
             case 164:
                    // MR Patch (case was empty before) *****************************************************
                    let typeNameText = undefined;
                    let typeName = node.elementType.typeName;
                    if (typeName) {
                        console.log(">>> serializeTypeNode: " + node.kind + ", elementType.typeName.text: " + typeName.text );
                        typeNameText = typeName.text;
                    } else {
                        let typeNameText = ts.tokenToString(node.elementType.kind);
                        console.log(">>> serializeTypeNode: " + node.kind + ", typeName is undefined => primitive type, using ts.tokenToString(): array elementType: " + typeNameText );
                    }
                    // Pass the name of the captured type to "createIdentifier()"
                    return ts.createIdentifier("Array", typeNameText);
                    // MR Patch end *************************************************************************

@danielweck
Copy link

+1 to avoid workarounds like https://github.com/edcarroll/ta-json#jsonelementtypetypefunction

@thomas-mindruptive
Copy link
Author

and time goes by so slowly, ...

@yoxigen-zz
Copy link

+1

@listepo-alterpost
Copy link

Hi, any news?

@inad9300
Copy link

Any progress after more than one year? Thanks :D

@thomas-mindruptive
Copy link
Author

Excellent question! :-)

@IonelLupu
Copy link

@mhegazy Can you give us some updates on this? What's the next step to add this on the master branch(beside the actual merge request)? Maybe @thomas-mindruptive cand help with this.

@thomas-mindruptive
Copy link
Author

@jcwolf I can provide the "patch-code" I have posted above. But I think it's more of a general discussion within the TS team if and how they want to do this. It would be easy (technically) for the transpiler to decorate additional type-info. Maybe they are waiting for the EcmaScript-specs for these features. (Which would be boring because TS was always ahead! :-))

@thomas-mindruptive
Copy link
Author

And the years go by: > 3

@bemon
Copy link

bemon commented Jul 12, 2019

+1

@BlueFrog130
Copy link

This would be really awesome... Been waiting on this feature awhile.

@maurei
Copy link

maurei commented Nov 26, 2019

I feel like emitting generic type information is the next step towards having a strongly reflective language. At the same time I'm sure there must be technical/design dilemmas that haven't been covered in this thread. Either way, a comment from the typescript team here would be very welcome

@dolsem
Copy link

dolsem commented Apr 27, 2020

Any update on this? Would be nice to emit something like the following structure:

interface TypeMetadata {
  name: string;
  value: any;
  parameters: Array<TypeMetadata>;
}

So, for example, the result for Map<string, MyInterface> becomes:

{
  name: 'Map',
  value: Map,
  parameters: [
    { name: 'String', value: String, parameters: null },
    { name: 'MyInterface', value: Object, parameters: null },
  ],
}

To make this even more unambiguous, perhaps it can be useful to also include a path parameter with the value of the absolute path to the file where a custom type like MyInterface is defined. This way people can use this information at runtime to accurately get types from metadata.

@cuongphamcxa
Copy link

Any progress after more than one year?

@sshhsh
Copy link

sshhsh commented Jun 30, 2020

Looking forward to updates. XD

@comtaler
Copy link

I wish I am the last one to post this question: Any update?

@SpaceNinjaApe
Copy link

SpaceNinjaApe commented Aug 12, 2020

I've wrote a small example showing the use of decorators for request validations. Now i came across this issue.
I could do a dirty workaround like this:

example attr declaration

@prop()
foo: bar;

check

if (Array.isArray(obj.foo)) { 
     obj.foo.forEach((elm) => {
            if (elm instanceof bar  === false) {
                return false
            }
     }
}

But then again i would rather prefer this:

example attr declaration

@prop()
foo: bar[];

decorator-factory

function prop(): any {
    return function (target : Object, key : string) {
        let type = Reflect.getMetadata("design:type", target, key);
        console.log(type.isArray()) // true
        console.log(type.constructor.name) // "bar"
    }
}

@NSExceptional
Copy link

For everyone else who is, like me, wondering why this isn't getting any attention, a Redditor had this explanation for me:

TypeScript's decorators are based on an older proposal, and the JS specs around decorators have gone in a different incompatible direction. I believe they're avoiding changing how decorators work until the final goal is specced. Another way of saying this is that you should avoid using decorators, because they are unlikely to see improvements any time soon.

With the disclaimer that this is just him reading the tea leaves.

@IonelLupu
Copy link

@NSExceptional That's a bit sad but I know what the TS team feels and they are right. It's so hard to do something that doesn't have a spec.

I hope decorators will get specced soon because they are super awesome and save you from writing a lot of code and makes it look really expressive.

@bemon
Copy link

bemon commented Oct 6, 2020

Sometimes its better to ship implementation and made spec out of it based on real world needs. Decorators are super usefull and saves tons of code.

@thomas-mindruptive
Copy link
Author

It's a pity. TypeScript used to lead the way and provide new functionality, which was not available in JS.
Decorators have been around in TS for about 4 years. They are easy to use.

@ties-s
Copy link

ties-s commented May 3, 2021

Would love to see this happen.

@MaverikMinett
Copy link

Please give us this feature.

@Deathrage
Copy link

+1

@Limuyang1013
Copy link

Are their update on this?

@ionutnespus
Copy link

So, it's been seven years since initial request... Anyone found a solution for this?

@Yangff
Copy link

Yangff commented Mar 30, 2024

Implemented a transformer using ts-patch that can provide some workaround..
https://github.com/Yangff/extra-reflect-type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Decorators The issue relates to the decorator syntax Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests