algolia / react-instantsearch

⚡️ Lightning-fast search for React and React Native applications, by Algolia.
https://www.algolia.com/doc/guides/building-search-ui/what-is-instantsearch/react/
MIT License
1.97k stars 386 forks source link

Type checking of hitComponent props of Hits and InfiniteHits should include elementType #3086

Closed rupertchen closed 3 years ago

rupertchen commented 3 years ago

Describe the bug 🐛

Passing a hitComponent that is not a functional component results in a type check warning. Should the type be elementType (introduced in 15.7.0)?

To Reproduce 🔍

Steps to reproduce the behavior:

  1. Create a non-functional component. For example by using React.memo(...) which returns an object:

    const MyHit = React.memo((props => (<div>{props.hit.name}</div>)));
  2. Use the non-functional component in Hits or InfiniteHits:

    <InfiniteHits hitComponent={MyHit} />
  3. Perform search

  4. Open console in dev toolbar to see warning. For example:

    Warning: Failed prop type: Invalid prop `hitComponent` supplied to `InfiniteHits`.
       in InfiniteHits (created by Translatable(InfiniteHits))
       in Translatable(InfiniteHits) (created by AlgoliaInfiniteHits(Translatable(InfiniteHits)))
       in AlgoliaInfiniteHits(Translatable(InfiniteHits)) (created by Context.Consumer)
       in ConnectorWrapper (at Demo.js:62)
       in InstantSearch (at Demo.js:51)
       in section (at Demo.js:41)
       in Demo (at App.js:164)

A live example helps a lot! We have a simple online template for you to use for your explanations:

https://codesandbox.io/s/github/algolia/create-instantsearch-app/tree/templates/react-instantsearch

https://codesandbox.io/s/vigilant-hofstadter-kzsib?file=/src/App.js

Warning: Failed prop type: Invalid prop `hitComponent` of type `object` supplied to `Hits`, expected `function`.
    in Hits (created by AlgoliaHits(Hits))
    in AlgoliaHits(Hits) (created by Context.Consumer)
    in ConnectorWrapper (at App.js:48)
    in div (at App.js:41)
    in div (at App.js:36)
    in InstantSearch (at App.js:35)
    in div (at App.js:34)
    in div (at App.js:21)
    in App (at src/index.js:6)

Expected behavior 💭

I expect there to be no warnings.

Environment:

All environments where warnings in console are visible.

Additional context

The current implementation in InfiniteHits also includes PropTypes.string which seems incorrect.

Haroenv commented 3 years ago

FYI, the typescript types aren't maintained here, but by the community on DefinitelyTyped, so it's possible the types aren't correct

EDIT: I thought this was the typescript type, sorry for replying too quick

Haroenv commented 3 years ago

A string actually isn't a wrong type for the PropTypes, as you could do <InfiniteHits hitComponent="my-custom-element" /> that would accept the hit prop.