Object spread can lead to unknown properties added [Regression v3.1.6]

[Cellule]

TypeScript Version: 3.6.0-dev.20190719

Search Terms: spread type check

Code

// Error as expected, b2 is not a valid key
const foo: {a?: string; b?: string} = {
    //a: "",
    ...{b2: ""}
}
// I meant to overwrite b, but somehow this is accepted
const bar: {a?: string; b?: string} = {
    a: "",
    ...{b2: ""}
}

Expected behavior: bar should not accept ...{b2: ""}

Actual behavior: bar has type {a?: string; b?: string}, but at runtime actually has extra property b2

Playground Link: No problem v3.0.1 Regression v3.1.6

Related Issues:

30129

[micheleangioni]

I've run into this problem as well. It's still present in 3.7.2

Other example

[hikarino-my]

I've created a PR that will fix this issue. Could someone check for it please?

37392

[manast-apsis]

Not sure but seems to me that this issue should have quite a big impact in angular apps that use ngrx, where spread operator is commonly used to copy state properties from one action to the next in the reducers... basically it allows you to put any data on the state without being aware of it.

[mnn]

Would love to see this fixed, since it can lead to very undesirable outcomes (inadvertently polluting strictly typed store state, values of forms, data for saving to db, request bodies).

[jer-sen]

Would love too ! This code should definitely not be correct: const v: { a: string } = { a: '', ...{ b: '' } };

[sandersn]

We decided to track excess properties only for actual properties, not for spreads, back in 2017. See the design meeting notes on Spread object literal freshness at #14853.

[mnn]

Eh, this bug is "working as intended"? Well, I guess I question your "intentions" if you are allowing values to mismatch types in a statically typed language. But only sometimes, other times excess property check is working as users expect it to.

var c = { a: 0, d: 0 } var o: { a: number } = { a: 0, ...c}

Seems arguable about whether there should be an error.

I don't see how it is arguable. In my statically typed point of view it definitely should be an error, or at the very least a user should be able to enable a compiler option for it being an error.

We probably will have to use properly typed lenses (and forbid object spreads) to overcome this "working as intended" blunder, which already caused a bunch of bugs in our code which could have been caught early if the compiler would have done its job.

[ruizb]

The meeting notes from #14853 were written 3 years ago. Isn't type inference better nowadays, which would allow TypeScript to really know which properties the spread object has?

[jer-sen]

I can't believe it. It would avoid so many bugs...

[manast-apsis]

So I think it is quite important to know why they do not support this, according to the link above:

var c = { d: 0 };
var o: { a: number, b: string } = {
    a: 0,
    b: "",
    ...c,
    d: 100
};

When you spread in c, you don't know what properties it really has. So TypeScript doesn't really know if you have excess properties in some cases."

Which makes sense, and I guess this problem is just surfacing from the fact that typescript type system is not sound, and this is just a manifestation of that limitation... I started to look into hegeljs which has some interesting properties though: https://github.com/JSMonk/hegel

[sandersn]

I question your "intentions" if you are allowing values to mismatch types in a statically typed language

Typescript, along with any language with subtyping (including Hegel), allows this:

class A { a }
class B extends A { b }
const a: A = new B()
const ab = { ...a } // contains b at runtime -- even though the static type of `a: A`.

I don't like subtyping, but it's unavoidably part of Javascript, and nothing to do with whether Typescript is sound or not. If you want to avoid this problem, use a language without subtyping.

[mnn]

I don't like subtyping, but it's unavoidably part of Javascript, and nothing to do with whether Typescript is sound or not. If you want to avoid this problem, use a language without subtyping.

Believe me, I would love to, but I am stuck at work with TypeScript.

Typescript, along with any language with subtyping (including Hegel), allows this:

But what if we don't use subtyping?

interface A { a: null }

const good: A = { a: null, invalid: true }; // correctly reports error

const bad: A = { a: null, ...{ invalid: true } }; // incorrectly doesn't report error

TypeScript here can statically verify it's an error. Why both cases aren't reported as errors or at least cannot be enabled in compiler to report them as errors? This looks to me like pretty bad inconsistency.

[sandersn]

...{} is really rare, so checking spreads of object literals differently from other spreads would complicate the compiler with no real benefit.

I grepped for ...{ in all the .js/.ts files in our user and, besides tests in prettier and flow, found one use, which looks like it was introduced by a mechanical upgrade from Object.assign.

[JSerZANP]

@sandersn I'd say spreading is quite common in JSX when a wrapper component relays props to its children.

this could be very useful in React.

regarding this matter Flow does better https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+IFkolXpUCWewUEAwhCQgRDH8wEH4hMnwROHlsNnw4KHwwSLAAC3wANyo4LFxscWQuHgMNZmwsiRSAWglaY1cq-hIAa2wJXNpG4Vxcdvdu3v7B0RxKUYMhKDBSqmbWwIq3eagoOrKSKgH0wtMMPznY7d2SfcoBiEZ-aG5G3Ix085AF-ZhsRoRehqUEiNMgSQHlSruBZxJrMcJwMhzAC+-EgGiCLWMAAIALK0AAqayxAF4scAADpQLFYjDILHCEQzADclKRlMptnsWKO3DgbBJZMp1NpWIAjLwhVi0HSAEysllQSkAeiV9IgUWwWIkOUiWIABjzcHy9dySGqJDTrogoCVNWF9XjCcY9RzoFzpbiCUTScAaXTxVLZUiFcrVWhQliAO6aqgxtClLUQLWmMBdbkWyNwbVYpiefn3EyRCCUV12C0ix3eslYqy1w18rFImIgfImEhwaBBfIABisMtFAHYrN2QEigA

[fbeaudoincoveo]

Hi @sandersn,

One pattern we use all the time is:

type MyType = {
  a: number;
  b?: number;
}

const obj: MyType = {
  a: state.a,
  ...(state.b !== undefined && { b: state.b }),
}

to avoid setting optional properties at all when their value would be undefined.

I believe this pattern is legitimate because

const obj1: MyType = {
  a: 1,
}

and

const obj2: MyType = {
  a: 1,
  b: undefined,
}

are not strictly equivalent. For instance, 'b' in obj will return false in the former case and true in the latter.

The fact that TypeScript does not check excess properties in spreads of object literals makes it easy to make mistakes such as:

type MyType2 = {
  myProp1: number;
  myProp2?: number;
}

// Notice there's a typo in { myPop2: state.myProp2 }, but this would compile just fine
const obj: MyType2 = {
  myProp1: state.myProp1,
  ...(state.myProp2 !== undefined && { myPop2: state.myProp2 }),
}

when using this pattern.