Skip to content
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

Fix exported types so that they work for react-redux #61

Closed
marqu3z opened this issue Feb 6, 2020 · 8 comments
Closed

Fix exported types so that they work for react-redux #61

marqu3z opened this issue Feb 6, 2020 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@marqu3z
Copy link

marqu3z commented Feb 6, 2020

It seems the typing for react redux hook useSelector relies on the types of the equality function to infer the return type:

// @types/react-redux/index.d.ts
export function useSelector<TState, TSelected>(
    selector: (state: TState) => TSelected,
    equalityFn?: (left: TSelected, right: TSelected) => boolean
): TSelected;

Using isEqual as equality function the compiler will infer any as TSelected type because:

declare module 'react-fast-compare' {
  const isEqual: (a: any, b: any) => boolean
  export default isEqual
}

So

const selector = (state: State): DeepObject => {a: { b: [1,2,3] } }
const value = useSelector(selector) // typed correctly as DeepObject
const value = useSelector(selector, isEqual) // typed as any

Declaring isEqual with generic types should solve the issue

declare module 'react-fast-compare' {
  const isEqual: <A, B>(a: A, b: B) => boolean
  export default isEqual
}
@chrisbolin
Copy link
Contributor

thanks @marqu3z! I'll look into this as soon as I can. I might cut a beta release—would you be willing to test that?

@kitten
Copy link

kitten commented Feb 6, 2020

@chrisbolin We may be able to retain support as it was before but satisfy typings like these by using: <A = any, B = any>(a: A, b: B) => boolean

@marqu3z
Copy link
Author

marqu3z commented Feb 7, 2020

thanks @marqu3z! I'll look into this as soon as I can. I might cut a beta release—would you be willing to test that?

Sure, no problem

@chrisbolin
Copy link
Contributor

see #62 as well. We may be able to use kitten's <A = any, B = any>(a: A, b: B) => boolean, but we may have to use (a: any, b: any): boolean. Some testing will be needed, and I think we should release a beta and have folks try it.

@chrisbolin chrisbolin changed the title The exported types are not compatible with React redux hooks useSelector Fix exported types so that they work for everyone Apr 30, 2020
@chrisbolin
Copy link
Contributor

Important note: in order to fix the types, we need to give up the naive hope that we could ever types that statically do a comparison. e.g.

<Thing = any>(<a: Thing, b: Thing>) => boolean

We've dealt with this earlier, so I want to make sure it doesn't come back to bite us.

@chrisbolin chrisbolin added the bug Something isn't working label Apr 30, 2020
kale-stew added a commit that referenced this issue May 11, 2020
kale-stew added a commit that referenced this issue May 12, 2020
kale-stew added a commit that referenced this issue May 12, 2020
kale-stew added a commit that referenced this issue May 13, 2020
kale-stew added a commit that referenced this issue May 13, 2020
@kale-stew
Copy link
Contributor

@chrisbolin the more I'm reading about namespace declaration, the more it seems our types are definitely broken as-is. With the namespace declaration being dead code, I'm not confident it's working the way we think it is?

More info about ambient namespaces / namespacing in general

kale-stew added a commit that referenced this issue May 26, 2020
@chrisbolin chrisbolin changed the title Fix exported types so that they work for everyone Fix exported types so that they work for react-redux May 27, 2020
@cdierkens
Copy link
Contributor

cdierkens commented May 18, 2021

kale-stew moved this from In progress to Done in May 2020 on May 28, 2020

@ryan-roemer this might be closable as well. We may not have had the automation turned on to set this ticket to closed when moving the card on the project board.

@ryan-roemer
Copy link
Member

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants