Typescript: Object spread and enumerable properties

[Conaclos]

Hi :)

EDIT:
As noted by @aluanhaddad, the bug is related to the combination of the shorthand syntax in a literal object and the object spread operator on this object.

TypeScript Version: 2.1.4

Code

First code (compilation success):

interface I <T> {
    f (): void
    g (): void
}
const a = {
    f <T> (this: I<T>): void {}
}
const b: I<any> = Object.assign({
    g (): void {}
}, a)

Replacement for last 3 lines (compilation failing):

const b: I<any> = {
    ...a,
    g (): void {}
}

Expected behavior:

Replace Object.assign with the object spread operator should not be harmful.

Actual behavior:

The first code makes happy the compiler.
However if I I replace Object.assign with the object spread operator (See replacement code), the compiler raises the next error:

error TS2322: Type '{ g(): void; }' is not assignable to type 'I<any>'.
  Property 'f' is missing in type '{ g(): void; }'.

[aluanhaddad]

This is unrelated to generics. It has to do with the difference between properties and methods.
It is also a very subtle behavior.
The issue here is that the value

{
  f() {}
}

is transpiled into the value

{
  f: function() {}
}

which correctly, as per the ECMAScript specification, has an _enumerable own property_ f whose value is a function.

However, consider this expression which is created via a class

new class {
  f() {}
}()

this doesn't have an _enumerable own property_ f. Because f is placed on the prototype and not the object itself, it will be ignored by the spread operator. The ... operator correctly ignores what it sees as methods but the problem is that it is not recognizing that the object literal has a property defined using method syntax.
That said, I think this _is_ a bug because the type of the object literal does have an _enumerable own property_ f but the compiler is behaving as if the concise method notation translates to a method definition when it in fact translates to a property definition.
Changing this would probably break a lot of builds...

[tomdale]

I'm hitting this when trying to use object spread to initialize default values.

This works:

interface RunOptions {
  log: (str: string) => void;
}

const DEFAULT_OPTIONS: RunOptions = {
  log: function(str: string): void {
    console.log(str);
  }
};

function run(command: string, opts: Partial<RunOptions> = {}) {
  let options: RunOptions = { ...DEFAULT_OPTIONS, ...opts }
}

This doesn't, even though it's semantically identical (the only difference is changing function properties to use concise method syntax):

interface RunOptions {
  log(str: string): void;
}

const DEFAULT_OPTIONS: RunOptions = {
  log(str: string): void {
    debug(str);
    logOnFailure(str);
  }
}

function run(command: string, opts: Partial<RunOptions> = {}) {
  let options: RunOptions = { ...DEFAULT_OPTIONS, ...opts }
}

It seems important for POJOs to be treated as a "data bag," and concise methods in this context should be treated as an enumerable property that happens to contain a function.

[wycats]

fwiw, we intentionally made { foo() { } } semantically identical to { foo: function() { } } (enumerable own property in both cases), even though methods are non-enumerable in classes.

As @tomdale says, object literals are intended to be data bags, with concise methods serving as shorthands, while classes have a clear data/behavior split, which is reflected through enumerability.

[aluanhaddad]

@wycats thank you! I remember having read something to this effect at one point and I tried to find it and link it to this issue but I couldn't find it again. It's great to have the intent and reasoning behind the runtime behavior clarified.

[sandersn]

I had a lot of discussions about this when working on the original proposal at #10727 that got condensed to a couple of comments there. I'll try to pull them together in one place here.

First, a summary based on @Conaclos' example:

1. Object.assign's type is wrong right now (T & U). It will change to match the spread expression's type in 2.2 ({ ...T, ...U }).
2. The compiler doesn't track own-ness or enumerability. So we decided to fake it: methods are non-own-enumerable unless they are specified in the object literal that contains the spread.

@Conaclos' example just exposes the boundary where the compiler decides that "this method is not owned". We could extend the fakery and treat methods declared in any object literal as own-enumerable. It would take a little more work but not much.

However, @tomdale's example exposes the place where fakery will never get us anywhere.
The underlying reason that this bug appears is that the compiler tracks neither own nor enumerable. So it can't tell the difference between an own-enumerable property like { f() { } } from @Conaclos' example and a non-own-enumerable new class { f() { } } from @aluanhaddad's example. (I'm ignoring the less common cases where own-ness and enumerability are different for this discussion.)

In fact, as @tomdale's example shows, as soon as you decouple types and values, the compiler doesn't have a good way to guess which properties of a type are owned:

interface I {
  a(): void; // own-enumerable?
  b(): void; // or not?
}
class C implements I {
  a = () => { }; // own-enumerable
  b() { }        // non-own-enumerable
}

Currently we use syntax to guess:

interface I {
  a: () => void; // probably own-enumerable
  b(): void;     // probably not
}

This is not great. The best thing you can say about it is that it reflects the syntax you will probably write to create an instance of the type.

By the way, the reason that this is important is that in 2.2 you will be able to write generic spread types like <T>(t: T) => { ...T, ...I }. Given the current semantics, this return type will have a property a but not a property b (unless of course T happens to have a b).

Unfortunately, the only way I can think of to really fix this is to make own-ness part of the language. For example, we could change interfaces to only have own properties, and require everything else to come from some special extends clause:

interface A {
  // only A's own properties here!
  a: number;
}
interface B extends__proto A {
  // only B's own properties here!
  b: (x: string) => void;
}

Or we could add own annotations:

interface B {
  proto a;
  b;
}

Although those should really be stackable so the compiler could track how many levels up the prototype chain it should expect to see the property. For example: proto proto proto proto fourLevelsUp: number.

I don't really think this is a good solution. I actually think fakery and subtle syntax is preferable. If we do start tracking all methods back to their declaration we could have a complex set of heuristics that differ for object literals, classes and interfaces, something like:

const o =  {
  a: () => { } // own-enumerable
  b() { }      // own-enumerable
}
class C {
  a: () => { } // own-enumerable
  b() { }      // not
}
interface I (
  a: () => { } // own-enumerable
  b() { }      // own-enumerable (Note [1])
}

But any rule you make here is (1) guaranteed to be wrong sometimes (2) trivially easy to circumvent. And you have to be able to explain the rules in a reasonable amount of time.

[1] Probably people who write an interface that they expect to be used in a spread will also expect only object literals to be spread, not instances of classes.

[sandersn]

@tomdale I don't think I called it out explicitly enough, but in 2.1, you can control own-enumerable in interfaces with a syntactic distinction:

interface I {
  a: () => { } // own-enumerable
  b() { } // not
}

I intended it to work that way so in that sense, it is By Design. But it might be better to assume that people who are writing interfaces for use in a spread context want users to spread only object literals:

interface I {
  a: () => { } // own-enumerable
  b() { }      // own-enumerable
}
const o: I = { a() { }; b() { } };
const s = { ...o }; // still has `a` and `b`.

const c = new class { a() { }; b() { } };
const s2 = { ...c }; // incorrectly has `a` and `b`, but why did you spread a class instance?

And it's more honest because the compiler can't possibly track own-enumerable through an interface, so allowing control of its approximation through a "secret" feature is kind of shady.

[mhegazy]

As per the discussion in https://github.com/Microsoft/TypeScript/issues/13331; properties should be filtered iff they originate from method declarations on a class.

[aluanhaddad]

@sandersn Thank you for the detailed explanation. I was only considering the direct use of an inferred object literal type in a spread expression but you point out how quickly that assumption breaks down - as soon as the types and values are are dissociated. @mhegazy Even if the method originates in a class declaration, the class might be extended by an interface and then implemented through an object literal...

declare class A {
  m(): void;
}
interface B extends A {
  n(): void;
}
const x: B = { m() { }, n() { } };
const y = { ...x };

It comes down to how the value is created, which could be at odds with how it is annotated and seems impossible to track in a meaningful way.

Also, given that it is not unheard of for .d.ts files to be updated
from

interface A {
  m(): string;
}
declare const A: AStatic;
declare interface AStatic {
  new (x: string): A;
}

to

declare class A {
  constructor(x: string);
  m(): string;
}

this could get a lot trickier because what was once a straightforward refactoring becomes a major change.

By the way, it is great to hear that spread types are going to be in the next version!