Skip to content

Generics in conditionals have differing behavior in 4.8 #49490

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

Open
DanielRosenwasser opened this issue Jun 11, 2022 · 10 comments
Open

Generics in conditionals have differing behavior in 4.8 #49490

DanielRosenwasser opened this issue Jun 11, 2022 · 10 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@DanielRosenwasser
Copy link
Member

I found a break in zustand from looking at the logs of a failed NewErrors run. It might be more correct, but figured it was worth noting.

export type Store<T extends object> = {
    setState: (x: T) => void
    getState: () => T;
}

type ExtractState<S> = S extends { getState: () => infer T } ? T : never

type Extractable<S extends Store<object>> = {
    getServerState?: () => ExtractState<S>
}

export function f<TState extends object>(api: Extractable<Store<TState>>) {
//                                                        ~~~~~~~~~~~~~
// Type 'Store<TState>' does not satisfy the constraint 'Store<object>'.
//   Types of property 'setState' are incompatible.
//     Type '(x: TState) => void' is not assignable to type '(x: object) => void'.
//       Types of parameters 'x' and 'x' are incompatible.
//         Type 'object' is not assignable to type 'TState'.
//           'object' is assignable to the constraint of type 'TState', but 'TState' could be instantiated with a different subtype of constraint 'object'.
}

https://github.com/pmndrs/zustand/blob/197e5d41993b0e3ad6ebef9a7e6a9161753072f1/src/react.ts#L30

export function useStore<TState extends State, StateSlice>(
  api: WithReact<StoreApi<TState>>,
//               ~~~~~~~~~~~~~~~~
// error TS2344: Type 'StoreApi<TState>' does not satisfy the constraint 'StoreApi<object>'.

There's a few of these.

New errors for non-composite project https://github.com/pmndrs/zustand/blob/197e5d41993b0e3ad6ebef9a7e6a9161753072f1/tsconfig.json

Originally posted by @DanielRosenwasser in #49488 (comment)

@yume-chan
Copy link
Contributor

@Andarist
Copy link
Contributor

cc @devanshj - you might find this interesting

@devanshj
Copy link

devanshj commented Jun 13, 2022

Thanks for the ping @Andarist!

Will take a look into it soon but for the time being I want to point out that @DanielRosenwasser's reproduction is incorrect because zustands's Store has setState's T bivaraint, making T in Store<T> covariant...

type Store<T extends object> = {
    getState: GetState<T>
    setState: SetState<T>
}
type GetState<T> = () => T
type SetState<T> = { _(x: T): void }["_"]

type ExtractState<S> = S extends { getState: () => infer T } ? T : never

type Extractable<S extends Store<object>> = {
    getServerState?: () => ExtractState<S>
}

function f<TState extends object>(api: Extractable<Store<TState>>) {}

And this compiles in both 4.7 and 4.8. So the errors being reported (which I haven't looked into yet) seem to be caused by some other reason.

@devanshj
Copy link

devanshj commented Jun 13, 2022

From pmndrs/zustand#1004

I'm not sure exactly what the issue is but somehow TS 4.8 doesn't understand that StoreApi is covariant on T but with interface instead of type alias and with explicit out annotation TS now understands and accepts that T is indeed covariant which fixes the issue.

@DanielRosenwasser
Copy link
Member Author

Hm, not sure how I made that slip-up, and I'm honestly a little confused about the interface/type alias difference. If you have some time to get a minimal repro that'd be at the very least useful for us to investigate.

@Andarist
Copy link
Contributor

This is quite complex and beyond my understanding. We can already observe the issue with type here. However, the same behavior can be observed on 4.7 with this and if we change type to an interface then we might see an error on the variance annotation, see here.

In Zustand repo I couldn't see the same annotation error though so I kept digging... and I've somewhat narrowed it down to this. In here we won't observe the variance annotation error but it's quite easy to get it - by removing some seamingly unrelated lines of code.

The code here has some cycles and stuff and I think that this just makes variance measurement to give up and thus it ends up too forgiving. For some reason though the type-based variant was accepted in the past and now it isn't. It looks a little bit like as if variance computation became slightly smarter now and errors sooner rather than reporting unmeasurable (or similar)

Note that I'm just guessing a lot here.

@devanshj
Copy link

Hm, not sure how I made that slip-up, and I'm honestly a little confused about the interface/type alias difference. If you have some time to get a minimal repro that'd be at the very least useful for us to investigate.

Yeah I too am not sure what's up with the interface vs type alias thing. Will try to get a minimal repro if I find time for it.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 20, 2022
@jcalz
Copy link
Contributor

jcalz commented Jun 16, 2024

This SO question is running into this.

@FelixZY
Copy link

FelixZY commented Jun 16, 2024

I see there has been some requests for a minimal reproduction. I think the code used in my SO question (linked by @jcalz ) might be able to provide that:

interface Vehicle {}
interface Workshop<TVehicle extends Vehicle = Vehicle> {
  vehicleType: TVehicle;
  repair: (vehicle: TVehicle) => WorkOrder<this>;
}
// Unexpected `Type 'TWorkshop["vehicleType"]' does not satisfy the constraint 'Vehicle'.ts(2344)`
interface WorkOrder<TWorkshop extends Workshop<TWorkshop["vehicleType"]>> {
  vehicle: TWorkshop["vehicleType"];
}

Here's a playground link which includes objects proving that the WorkOrder type is working as expected despite the error: https://www.typescriptlang.org/play/?ts=5.4.5#code/JYOwLgpgTgZghgYwgAgGoQBbAQGxQbwF8AoUSWRFAQWCgAcc4QUIAPSEAEwGc1Ns8yfMWTIQAVwC2AI2gAuMVNlQA3MRJlo8JMgBCAezhhkbDjz5ZcBEcgT6c+qAu5gooAOZqN4LZWQB1RwBrbgx9OgAeABV0S0FTCC5eWIEUAF4LVIA+IRsAN34rKIBPOggFGMK8NVEoCDo4WgUACgK48uRK9oBKZDScwKgggHkoTmgIsCxuLK9SHwodQZGxiajl0PCTdkTzDbDI9eDNugBtACI21JKy84BdLJzhUSurCv3wi9e8G4h7ueIdhALmQjnGUCoCmWo3BEQ+kRo9EYzEefVyLyqHWeokUMnkyAAjAAmADMABobIQKYQ1ID9MDjGDoLoocEYRN4REDEZURlsd8sTZRHYHE5kOdpDhxH8KaIqepaUCQUyoABhVlDdlQOHHA4RREMJgQXno5AChTY4X2RwKCVSmXIAD0juQAFEAEru4bugCEyGG0gAVhAEMYcMByHAcMhJHBiqCQDh49wyghgDB40EQPoAO4gZB0KDhaBgYAQbhk5BMTjIADkIscteQnH05bE+mMbGAINAyDApRQtYNyIgtcp1JUQA

Indeed, with TS 4.7 there are no (unexpected) errors but in 4.8 there is.

@Andarist
Copy link
Contributor

The one above bisects to 4.8.0-dev.20220528 and more specifically to removing those lines:
https://github.com/microsoft/TypeScript/pull/49119/files#diff-d9ab6589e714c71e657f601cf30ff51dfc607fc98419bf72e04f6b0fa92cc4b8L19578-L19584

@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

7 participants