Erroneous TS2783 when ternary expressions are inlined in a spread operator

[tpict]

Bug Report

🔎 Search Terms

TS2783, spread operator, control flow analysis

🕗 Version & Regression Information

This changed between versions 4.3.5 and 4.4.4

⏯ Playground Link

Playground link with relevant code (thanks webstrand#8856 on the TS community server)

💻 Code

const override = { color: "green" };
const conditional = false as boolean;

const example = {
    color: "red",  // TS2783 here
    ...(conditional ? override : { size: "large"})
}

const more = conditional ? override : { size: "large"};
const example2 = {
    color: "red", // no TS2783 here
    ...more
}

🙁 Actual behavior

When using a ternary expression within a spread operator to conditionally apply properties to an object, a TS2783 error (This spread always overwrites this property) is raised even if the property is only overwritten in one branch of the conditional. The error is not present if the result of the ternary is assigned to a variable which is then applied with the spread operator.

🙂 Expected behavior

TS2783 should not be raised in situations where the property is only conditionally overwritten by a spread.

[sandersn]

Broken by #44696, where I commented "I think it's fine to ship a stricter check; we'll see if people complain in the beta."

I'm not sure the break was intentional, though, so I'm going to make sure.

[sandersn]

The new 4.4 code assumes that conditional ? override : { size: "large"} produces a type { color: string, size?: undefined } | { color?: undefined | size: string } -- non-optional properties on either side of the union will produce the error.

However, the type is still { color: string } | { size: string }. I need to figure out how to produce the correct union type and make sure to call that function for spreads.

[sandersn]

There are 3 ways to fix this, but none of them are satisfactory.

1. call getWidenedType when checking a spread so that { a: A } | { b: B } widens to { a: A, b?: B } | { a?: A, b: B }. But getWidenedType returns {} behaviour when one side of the conditional is {}, which is pretty common in spreads. The larger union type also interacts worse with inference -- a test with Object.entries fails now.
2. Allow getPropertyOfUnionOrIntersectionType to create a union property a?: string | undefined for a in { a: A } | { b: B }. This was explicitly disallowed by Don’t allow an object literal with a spread as a fallback for destructuring a property not present in all constituents #43783 because of bug Immediately destructuring a spread union allows access to properties not present in all union constituents #43174, so I think it's a bad idea.
3. Partly restore the exemption removed in Fix check for overwritten properties in object spreads #44696 so that the error isn't issued on required properties that include undefined in their type. That's because getPropertyOfUnionOrIntersectionType currently returns a: string | undefined. However, that exemption is still wrong for { a: A | undefined } when exactOptionalPropertyTypes is true. But restoring the exemption only when it's off is extra confusing.

I don't think there's a good fix to this bug. The first two fixes are far-reaching, and have bad effects. The third fix still isn't really correct, especially when exactOptionalPropertyTypes is on, and making the error condition different when it's on is confusing.

[sandersn]

I pushed the fix to make-spread-overwrite-error-more-lenient for future reference.