Object spread with Partial breaks in TS 2.4
evmar opened this issue · 9 comments
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.
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 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.
Thanks for the quick fix! I'll undo my workaround.