microsoft/TypeScript

Object spread with Partial breaks in TS 2.4

evmar opened this issue · 9 comments

evmar commented

Test program works in TS2.3 and fails in TS2.4:

interface Fields {
  foo: number;
  bar: string;
}

declare let y: Fields;
declare let z: Partial<Fields>;

let x: Fields = {...y, ...z};
sigint:~/t$ tsc --version
Version 2.3.4
sigint:~/t$ tsc
sigint:~/t$ ./node_modules/.bin/tsc --version
Version 2.4.0
sigint:~/t$ ./node_modules/.bin/tsc 
f.ts(9,5): error TS2322: Type '{ foo: number | undefined; bar: string | undefined; }' is not assignable to type 'Fields'.
  Types of property 'foo' are incompatible.
    Type 'number | undefined' is not assignable to type 'number'.
      Type 'undefined' is not assignable to type 'number'.

That might actually be catching a bug, but I think it's running into typescript limitations.

If z is missing the fields (or they're all not undefined), then the result x will still be a valid object; but if z has one of the Fields' fields and it is set to undefined, then x will be an invalid object with an invalid value. TypeScript doesn't distinguish an unset field from a field set to undefined (other than in excess property checks?).

Object.assign({a: 'a'}, {}) // => {a: 'a'} (matches {a: string})
Object.assign({a: 'a'}, {a: undefined}) // => {a: undefined} (does not match {a: string})

@Kovensky is right. Typescript does not currently distinguish between missing and undefined in most cases [1]. We discussed adding missing but came to the conclusion that the additional complexity of yet-another-null-type is not currently worth it, even if we could find a way for users not to need to annotate it.

So 2.4 errs on the side of safety and prevents @Kovensky's second example from compiling at the cost of preventing the first, correct example from compiling.

[1] The excess property check is an ad-hoc check carried out in a completely different way from other assignability checking.

evmar commented

That's interesting. The language syntactically makes a distinction between

interface Foo { bar?: string }
interface Foo2 { bar: string|undefined }

but you're saying they are typed the same?


The best workaround I could find is to cast away from partial when you use it. Kinda disappointing though. Do you have any better suggestion?

function update(f: Foo, newFields: Partial<Foo>) {
  // no longer works:
  // return {...f, ...newFields };
  // workaround:
  return {...f, ...(newFields as Foo) };
}

This issue has been raised erlier in #6613 and #12793.

This looks like an unintended consequence of #15938. untill we have a way to distinguish between missing and undefined, we should error on the side of usability, and treat optional properties with undefined in their type as missing

@evmar basically yes. In this case we're going to make an exception for spread since {...T, ...Partial<T>} should mean that T is a bag of defaults and Partial<T> is a bag of overrides, which nobody has been foolish enough to stick undefined in. This is technically a hole, but we're stuck with holes or over-strictness without missing.

I'll open an issue to track distinguishing between missing and undefined.

Here's a stub for distinguishing missing and undefined: #16524

It still needs a lot more detail.

Fix is up at #16525

evmar commented

Thanks for the quick fix! I'll undo my workaround.