Uh oh!
There was an error while loading. Please reload this page.
Defer types like keyof (T & {}) - #49696
Conversation
Anders Hejlsberg (ahejlsberg)
commented
Jun 27, 2022
TypeScript Bot (@typescript-bot) test this |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based user code test suite on this PR at ab04c57. You can monitor the build here. Update: The results are in! |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at ab04c57. You can monitor the build here. |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at ab04c57. You can monitor the build here. |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the abridged perf test suite on this PR at ab04c57. You can monitor the build here. Update: The results are in! |
TypeScript Bot (typescript-bot)
commented
Jun 27, 2022
Anders Hejlsberg (@ahejlsberg) Here they are:Comparison Report - main..49696
System
Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TypeScript Bot (typescript-bot)
commented
Jun 27, 2022
Heya Anders Hejlsberg (@ahejlsberg), I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
TypeScript Bot (typescript-bot)
commented
Jun 27, 2022
Anders Hejlsberg (@ahejlsberg) |
| type K04 = keyof void; // never | ||
| type K05 = keyof undefined; // never | ||
| type K06 = keyof null; // never | ||
| type K05 = keyof undefined; // string | number | symbol | ||
| type K06 = keyof null; // string | number | symbol |
There was a problem hiding this comment.
For me, as a user - this doesn't quite make any sense. It's super hard for me to reason about what kind of runtime behavior this is supposed to represent. I've also checked it with this PR and this creates an unsoundness and might lead to runtime errors like here:
functiontest<T,KextendskeyofT>(t: T,k: K){returnt[k];// oops, runtime error if we allow the call below}test(null,'foo')// OK with this patch, error with 4.7There was a problem hiding this comment.
Given the new 4.8 behavior, test itself should have an error in its implementation due to not having a { } constraint.
There was a problem hiding this comment.
First experiment wasn't optimal, now deferring types like keyof (T & {}) such that effects of removing undefined and null are properly reflected.
keyof undefined and keyof null same as keyof neverkeyof (T & {})Anders Hejlsberg (ahejlsberg)
commented
Jun 28, 2022
TypeScript Bot (@typescript-bot) test this |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at b457868. You can monitor the build here. |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based user code test suite on this PR at b457868. You can monitor the build here. Update: The results are in! |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the abridged perf test suite on this PR at b457868. You can monitor the build here. Update: The results are in! |
Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at b457868. You can monitor the build here. |
TypeScript Bot (typescript-bot)
commented
Jun 28, 2022
Anders Hejlsberg (@ahejlsberg) |
TypeScript Bot (typescript-bot)
commented
Jun 28, 2022
Heya Anders Hejlsberg (@ahejlsberg), I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
TypeScript Bot (typescript-bot)
commented
Jun 28, 2022
Anders Hejlsberg (@ahejlsberg) Here they are:Comparison Report - main..49696
System
Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Mateusz Burzyński (Andarist)
commented
Jun 29, 2022
I've noticed that this PR didn't introduce the reported issue from #49681 as a test case. Is that alright? Or perhaps just an accidental omission? |
| // We allow an unconstrained object of a generic type `T` to be indexed by a key of type `keyof T` | ||
| // without a check that the object is non-undefined and non-null. This is safe because `keyof T` | ||
| // is `never` (meaning no possible keys) for any `T` that includes `undefined` or `null`. |
There was a problem hiding this comment.
Isn't this sort of an unusual thing to "defer" the error reporting to the call site? I'm not totally sure what I think about this yet but my first thought was that it's a special case in the algorithm that might be hard to explain to people.
There was a problem hiding this comment.
I guess this build on top of the fact that T[never] is always OK and never is always assignable to everything so perhaps this fits the existing rules just OK. I'll take consolation in the fact Ryan Cavanaugh (@RyanCavanaugh)'s first instinct was that those should raise errors too 😅
There was a problem hiding this comment.
I added the test and the comment to capture the behavior for future reference. It's nothing new actually. We have the same behavior in 4.7 with the old NonNullable<T> based on a conditional type. This PR ensures we carry it forward in 4.8 with NonNullable<T> based on T & {}.
There was a problem hiding this comment.
Sure - but I'm not thinking here from the PoV of NonNullable at all. My confusion/concern was solely about unconstrained generics and your changes from #49119, I've assumed that given those changes this indexed access would error nowadays. Since the base constraint of unconstrained T is unknown it feels off that we can use indexed access on it without refining the type somehow first
There was a problem hiding this comment.
It may seem surprising at first, but it is actually quite meaningful because we can permit t[k]without requiring local proof that t is non-undefined and non-null. Such proof would add unnecessary overhead for instantiations where the type argument for T is already known not to be undefined or null.
Ordinarily, types of the form
keyof (A & B)are normalized tokeyof A | keyof B. However, for types likekeyof (T & {}), this becomeskeyof T | keyof {}, which becomes justkeyof T, losing the fact thatundefinedandnullhave been removed fromT. With this PR we defer the normalization for intersection types that include{}.Fixes#49681.