Repository navigation
Erroneous TS2783 when ternary expressions are inlined in a spread operator #46463
Description
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.
Activity
sandersn commentedon Feb 7, 2022
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 commentedon Feb 8, 2022
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 commentedon Feb 9, 2022
There are 3 ways to fix this, but none of them are satisfactory.
getWidenedTypewhen 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 withObject.entriesfails now.getPropertyOfUnionOrIntersectionTypeto create a union propertya?: string | undefinedforain{ 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.undefinedin their type. That's becausegetPropertyOfUnionOrIntersectionTypecurrently returnsa: string | undefined. However, that exemption is still wrong for{ a: A | undefined }whenexactOptionalPropertyTypesis 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
exactOptionalPropertyTypesis on, and making the error condition different when it's on is confusing.Make spread overwrite error more lenient.
sandersn commentedon Feb 9, 2022
I pushed the fix to make-spread-overwrite-error-more-lenient for future reference.