Skip to content

Recreate old decorator metadata behavior#19089

Merged
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:recreate-old-decorator-metadata-behavior
Oct 12, 2017
Merged

Recreate old decorator metadata behavior#19089
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:recreate-old-decorator-metadata-behavior

Conversation

@weswigham
Copy link
Copy Markdown
Member

@weswigham weswigham commented Oct 11, 2017

Fixes #18509.

With strictNullChecks off, now we intentionally elide undefined and null from the list of type nodes we look at when attempting to find a metadata type for a union type node (and always elide never), emulating our pre-2.4 behavior. As stated in the issue, the behavioral change was actually witnessable in 0eaa8eb - you can see this PR reverts the baselines changed in that commit to their original output.

@sheetalkamat
Copy link
Copy Markdown
Member

@mhegazy had mentioned to not treat null | undefined as excpetion in #13540 (comment) I do think the change makes sense yet i would let @mhegazy approve this.

@weswigham weswigham merged commit de0e475 into microsoft:master Oct 12, 2017
@weswigham weswigham deleted the recreate-old-decorator-metadata-behavior branch October 12, 2017 22:05
@typeofweb
Copy link
Copy Markdown

@weswigham What's the reasoning behind not eliding null and undefined from unions for metadata when strictNullChecks is on?

@weswigham
Copy link
Copy Markdown
Member Author

@mmiszy We're using the syntactic data to simulate how the typesystem sees the type. "Complex" types (unions, intersections, etc) are emitted as Object - unions with null and undefined are no exception. The difference is that outside of strict, we elide null and undefined from unions for the purpose of comparison, so we need to emulate that behavior here to be consistent with how the typesystem views the types.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 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

Successfully merging this pull request may close these issues.

4 participants