Skip to content

Decorator metadata breaking change between TS 2.3.4 -> 2.4.0 #18509

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
flushentitypacket opened this issue Sep 15, 2017 · 10 comments
Closed

Decorator metadata breaking change between TS 2.3.4 -> 2.4.0 #18509

flushentitypacket opened this issue Sep 15, 2017 · 10 comments
Assignees
Labels
Bug A bug in TypeScript Domain: Decorators The issue relates to the decorator syntax Fixed A PR has been merged for this issue

Comments

@flushentitypacket
Copy link

I couldn't find any documentation on this breaking change between TS 2.3.4 and TS 2.4.0.

I'm not sure whether this is a bug or simply an undocumented change. I suspect the latter--in which case my suggestion is to add documentation regarding that change.

TypeScript Version: >= 2.4.0

Ran tsc with flags --experimentalDecorators --emitDecoratorMetadata.

Using reflect-metadata v0.1.10

Code

import 'reflect-metadata'

const dec = (obj: {}, prop: string) => undefined

class Foo {
  @dec public foo: string | null // seems to happen with any complex type
  @dec public bar: string
}

console.log(Reflect.getMetadata('design:type', Foo.prototype, 'foo'))
console.log(Reflect.getMetadata('design:type', Foo.prototype, 'bar'))

Expected behavior:

output in 2.3.4:

[Function: String]
[Function: String]

Actual behavior:

output in 2.4.0:

[Function: Object]
[Function: String]
@awendland
Copy link

awendland commented Sep 15, 2017

Related to #9916 but different because #9916 is a feature request while this is a request to change back to previous behavior, or at least document the breaking change. It looks like this commit is what changed the behavior 0eaa8eb.

@awendland
Copy link

awendland commented Sep 15, 2017

Running git tag --contains 0eaa8eb3ab9ad1db8cf95c51d8e02c9d65ee40cf reports v2.4-rc, v2.4.0, v2.4.1, v2.4.2, v2.5-rc, v2.5.0, v2.5.1, and v2.5.2.

@mhegazy mhegazy added the Bug A bug in TypeScript label Sep 15, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Sep 15, 2017
@mhegazy mhegazy added the Domain: Decorators The issue relates to the decorator syntax label Sep 15, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 12, 2017
@ulrichb
Copy link

ulrichb commented Apr 27, 2018

Hmmm. Using the current TS 2.8.3 and still see this issue. Can someone confirm this?

@weswigham
Copy link
Member

Do you have strictNullChecks on?

@ulrichb
Copy link

ulrichb commented Apr 27, 2018

Yes.

BTW, this works: @myDec() public PropN?: boolean;

The following doesn't work:

@myDec() public PropN: boolean | undefined;

or

@myDec() public PropN: boolean | null = null;

@weswigham
Copy link
Member

weswigham commented Apr 27, 2018

Yeah, if you read the linked fix PR - #19089 - this is the expected behavior under strictNullChecks, as a number | undefined isn't just a Number - it's a Number or undefined (which there's no corresponding way to emit metadata for). We don't emit metadata for nonhomogenous unions, as decorator metadata is syntactic (hence why the ? behaves differently than | undefined), and emit the base Object instead.

@ulrichb
Copy link

ulrichb commented Apr 28, 2018

Okay, this was a misunderstanding. I thought this also concerns strict checking. Thanks for clarification.

@ulrichb
Copy link

ulrichb commented Apr 28, 2018

Regarding the general union "issue" of reflecting to Object: I found a few "closed by design" issues.

Is there really no possibility to get this information? Are there no plans to fully support this scenario?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 30, 2018

We have no plans to implement a full run-time type serialization system akin to reflection C# or Java (more in #3628). The meta-data emit, continues to be an experimental feature tied to decorators, and has a very limited scope (i.e. classes and primitives). we have no plans to expand that scope either.

@ulrichb
Copy link

ulrichb commented May 6, 2018

@mhegazy Thanks for clarification.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: Decorators The issue relates to the decorator syntax Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants