FormidableLabs / react-fast-compare

fastest deep equal comparison for React
MIT License
1.59k stars 54 forks source link

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

Closed marqu3z closed 3 years ago

marqu3z commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

see https://github.com/FormidableLabs/react-fast-compare/pull/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 commented 4 years ago

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.

kale-stew commented 4 years ago

@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

cdierkens commented 3 years ago

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 commented 3 years ago

Sounds good!