TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Regression in the way optional properties behave when there's an error in the property value

Open devanshj opened this issue 9 months ago • 7 comments

🔎 Search Terms

optional properties, self types

🕗 Version & Regression Information

This changed between versions 5.3.3 and 5.4.5

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.8.2#code/CYUwxgNghgTiAEYD2A7AzgF3gMyUgjAFwBQ88APACLwgAeGIKwa8AYnvlQHxcAUo2AJYpBGQakLxKASngBeLlOLEMATwAOCdgSoAaeACFY8+AAUYSdXvgAiAEawbPeaXgBveA5gkyv9-ABtAGl4YXgAaxBVJGxDWABdHz9koxhg+Jp6RmZPPAgQKBR4AH44tKCMyRsAURgLb09jNAALJABXCGBPBDs8gpQbeFc-AF9XMeJQSFgEZHQsXCQAJiSqTIYmFm0l7j4BYVFxFEkZeUVKZTVNNjwdyn1Uk3NLa3tHZzlXDy9ipL8PYKhIqRaKxVKJYbJMipdLrbIsXpIfKFEplWFVWr1SReeAtdqdbq5JH9QaQshjcmXDQIZ5WAAq+iCik+ZBCdA2ORBMXgdNRdPR8BQIAAbiAYMpFvheN9YJIPAANKoMTCDEbwEbSYgAei1UKhAD1ihLbtLGg0FUqQCr1erNTq9clDcogA

💻 Code

declare const foo1:
  <D extends Foo1<D>>(definition: D) => D

type Foo1<D, Bar = Prop<D, "bar">> =
  { bar:
      { [K in keyof Bar]:
          Bar[K] extends boolean ? Bar[K] : "Error: bar should be boolean" 
      }
  }

declare const foo2:
  <D extends Foo2<D>>(definition: D) => D

type Foo2<D, Bar = Prop<D, "bar">> =
  { bar?:
      { [K in keyof Bar]:
          Bar[K] extends boolean ? Bar[K] : "Error: bar should be boolean" 
      }
  }

type Prop<T, K> =
  K extends keyof T ? T[K] : never

foo1({ bar: { X: "test" } })
//            ^? "Error: bar should be boolean"

foo2({ bar: { X: "test" } })
//            ^? "test"

🙁 Actual behavior

x in foo2 has type "test"

🙂 Expected behavior

x in foo2 should have type "Error: bar should be boolean" (ie it should behave same as foo1)

Additional information about the issue

I have a project that uses twoslash to test "custom errors" like above (see here for an example) and in the later TypeScript version those tests are failing even though the types are correct and have no change in them. The type error in the quickinfo is same but the inferred type changes across versions. And twoslash returns the inferred type not the type error message. It would be really nice if we can revert back to the previous behavior.

devanshj avatar Mar 09 '25 06:03 devanshj

Here's a fourslash test...

/// <reference path='fourslash.ts' />

// @strict: true
////declare const foo:
////  <D extends Foo<D>>(definition: D) => D
////
////type Foo<D, Bar = Prop<D, "bar">> =
////  { bar?:
////      { [K in keyof Bar]:
////          Bar[K] extends boolean ? Bar[K] : "Error: bar should be boolean" 
////      }
////  }
////
////type Prop<T, K> =
////  K extends keyof T ? T[K] : never
////
////foo({ bar: { /**/X: "test" } })

verify.quickInfoAt("", '(property) X: "Error: bar should be boolean"');

Bisected it to 02f9ddf55d27716c0306ec6561baf80d42744fed (#56318) (this is my first time using git bisect so I could be wrong)

@Andarist you broke my code :P

devanshj avatar Mar 09 '25 07:03 devanshj

I was just reading your issue before you pinged me and I thought that it totally looks like something I could change behavior of 😉

Andarist avatar Mar 09 '25 08:03 Andarist

Haha please do it!

devanshj avatar Mar 09 '25 08:03 devanshj

I meant to write "something I could have changed behavior of" - but nevertheless, I'll see why this has changed and if something can be done about it

Andarist avatar Mar 09 '25 09:03 Andarist

Simpler repro (TS playground):

function test1(arg: { prop: "foo" }) {}
test1({ prop: "bar" });
//      ^? (property) prop: "foo"

function test2(arg: { prop: "foo" } | undefined) {}
test2({ prop: "bar" });
//      ^? (property) prop: "bar"

I wouldn't really say this is a bug though. The behavior has changed, sure - but this related to an implementation detail that shouldn't be depended on. When a code has an error there is not always a good answer as to what should be displayed in situations like this. I'll still investigate this further though to understand the nature of this change better, maybe I'll put up some PR changing it, maybe not 😉

Andarist avatar Mar 09 '25 10:03 Andarist

It is a bug if you see what the type of arg gets inferred to...

function test1<T extends { prop: "foo" }>(arg: T): T { return arg; }
let t1 = test1({ prop: "bar" });
//               ^? "foo"
  t1;
//^? { prop: "foo" }

function test2<T extends { prop: "foo" } | undefined>(arg: T): T { return arg }
let t2 = test2({ prop: "bar" });
//               ^? "bar"
  t2;
// ^? { prop: "foo" } | undefined

Here the type of prop in test2 (and in test1) is inferred as "foo" but the quickinfo doesn't reflect that.

Also I'm not really dependent on it, the type error is same in both of those so it's not really a problem... just that when combined with @ts-expect-error the type error vanishes and one is only left with the type which twoslash returns... so it becomes harder to test it.

devanshj avatar Mar 09 '25 10:03 devanshj

There are two types here though - otherwise you wouldn't get the error in the first place. In reality, the prop has the "bar" type here. Even in the case that seems to work as you expect it to. It's just that there is some extra logic there, purely in the services layer, that makes it to prefer - at times - the property symbol of the contextual type. If you'd call checker.getTypeAtLocation on both of those you'd uniformly get "bar" and not "foo". In some sense, the quick info just tells us a sweet little lie here 😉

Andarist avatar Mar 09 '25 22:03 Andarist

We can't make any guarantees like this in the presence of errors. I'm not against improving something but just in general it's not an invariant.

RyanCavanaugh avatar Mar 24 '25 20:03 RyanCavanaugh

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

typescript-bot avatar Mar 27 '25 01:03 typescript-bot