Skip to content

Fix #13556: enable rest/spread on object#13558

Merged
sandersn merged 2 commits into
microsoft:masterfrom
HerringtonDarkholme:rest-spread-intrinsic
Jan 24, 2017
Merged

Fix #13556: enable rest/spread on object#13558
sandersn merged 2 commits into
microsoft:masterfrom
HerringtonDarkholme:rest-spread-intrinsic

Conversation

@HerringtonDarkholme
Copy link
Copy Markdown
Contributor

@HerringtonDarkholme HerringtonDarkholme commented Jan 18, 2017

Fixes #13556, I wonder if non-primitiveness should pass to spread type?

let a = 12;
let shortCutted: { a: number, b: string } = { ...o, a }
// non primitive
let spreadNonPrimitve = { ...<object>{}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: Primitive
(and missing some semi-colons, not very important in tests though)


// non primitive
let spreadNonPrimitve = { ...<object>{}}
>spreadNonPrimitve : { constructor: Function; toString(): string; toLocaleString(): string; valueOf(): Object; hasOwnProperty(v: string): boolean; isPrototypeOf(v: Object): boolean; propertyIsEnumerable(v: string): boolean; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spreadNonPrimitive should be {}. It looks like getApparentType gets called somewhere, probably in getSpreadType's call to getPropertiesOfType(right). I think you need a special case in getSpreadType that returns emptyObjectType.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, getPropertiesOfType does call getApparentType. Currently spreading Object type also gets all properties on GlobalObjectType, is this desirable?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Just like spreading number in javascript results in {} not { toString(), toFixed() ... }, spreading object should not get the properties of globalObjectType.

(Note that spreading number in typescript is illegal because it's useless, probably a mistake. Spreading object seems more useful.)

@sandersn
Copy link
Copy Markdown
Member

Also, I don't think the non-primitiveness should pass through. Spread types already destroy lots of information, and I think it's fine to convert object to {} by spreading.

@sandersn sandersn merged commit ceb5fac into microsoft:master Jan 24, 2017
@HerringtonDarkholme
Copy link
Copy Markdown
Contributor Author

Thanks! 😄

@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

Successfully merging this pull request may close these issues.

3 participants