-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regression in the way optional properties behave when there's an error in the property value #61382
Comments
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 02f9ddf (#56318) (this is my first time using @Andarist you broke my code :P |
I was just reading your issue before you pinged me and I thought that it totally looks like something I could change behavior of π |
Haha please do it! |
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 |
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 π |
It is a bug if you see what the type of 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 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 |
There are two types here though - otherwise you wouldn't get the error in the first place. In reality, the |
π 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
π Actual behavior
x
infoo2
has type"test"
π Expected behavior
x
infoo2
should have type"Error: bar should be boolean"
(ie it should behave same asfoo1
)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.
The text was updated successfully, but these errors were encountered: