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

Incorrect type inference of object spread, when spreaded object is referenced via interface and created by class instantiation #13409

Closed
OleksandrNechai opened this issue Jan 11, 2017 · 1 comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@OleksandrNechai
Copy link

OleksandrNechai commented Jan 11, 2017

TypeScript Version: 2.1.X (Playground)

Code

interface I { x: number, y: number, z: () => void }

class MyClass implements I {
  constructor(public x: number, public y: number) {
  }
  z() { alert(this.x + this.y) }
}

var x: I = new MyClass(5, 7);
var o = { ...x }
alert(o.z); // undefined

Expected behavior:
Compile error. Either MyClass incorrectly implements I interface requiring to define z as z=()=>alert(this.x + this.y) or at alert(o.z) reporting there is no such a property on o, or report something similar to FlowType.

Actual behavior:
No compiler warnings and runtime error when trying to invoke o.z().

Note:
When I change a definition of I this way: interface I { x: number, y: number, z(): void }, I get an error Property 'z' does not exist on type '{ x: number; y: number; }' as expected.

@sandersn
Copy link
Member

This is almost a duplicate of #13148, although there the desired behaviour was different. You can read that bug for the full resolution, but here are a few points specific to this example:

  1. Basically, don't spread class instances. Spread isn't designed to work well with classes.
  2. Unfortunately, because the compiler doesn't track own or enumerable on properties, it guesses when it comes to spread.
  3. After the fix Omit only class methods from object spreads #13365 for Object spread and enumerable properties #13148, Typescript now assumes that any method that doesn't definitely come from a class is still present after a spread. That's because spread should be used with object literals, not class instances.

Note that if you change the interface to be interface I { x: number, y: number, z(): void }, then flow fails to report the error and hits a runtime error. Flow just chose a different default than Typescript did. It might be easier to add own/enumerable inference to flow than to typescript, but I don't think either compiler does right now.

@sandersn sandersn added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jan 11, 2017
@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
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

2 participants