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

Too many queries when connectors are nested #2795

Closed StarpTech closed 5 years ago

StarpTech commented 5 years ago

Describe the bug πŸ›

When I pass a property from type function to the connectSearchBox searchbox the browser gets frozen and the memory will overflow.

To Reproduce πŸ”

const SearchBox = ({ currentRefinement, refine, translations, onKeyUp }) => {
  return (
    <div className="search__box">
      <input
        className="search__input"
        onKeyUp={onKeyUp}
        type="search"
        value={currentRefinement}
        placeholder={translations.placeholder}
        onChange={event => refine(event.currentTarget.value)}
      />
    </div>
  );
};

const CustomSearchBox = connectSearchBox(SearchBox);

const Autocomplete = ({hits, translations }) => {
  function handleKeyUp(event) {}

  return (
    <div className="search">
      <CustomSearchBox translations={translations} hitsCount={hits.length} onKeyUp={handleKeyUp} />
    </div>
  );
};

const CustomAutoComplete = connectAutoComplete(Autocomplete);

const AlgoliaSearch = ({ indexName, searchClient, translations }) => {
  return (
    <InstantSearch searchClient={searchClient} indexName={indexName}>
      <Configure hitsPerPage={6} />
      <CustomAutoComplete translations={translations} />
    </InstantSearch>
  );
};

Example (Be careful and save your workspace before) https://codesandbox.io/s/react-instantsearch-app-niiq2

Environment:

StarpTech commented 5 years ago

That issue results in an uncontrol amount of requests to algolia (~16k in 2min)

Haroenv commented 5 years ago

Is there a reason the example code has AutoComplete assigned twice?

Haroenv commented 5 years ago

I think the example is missing out some things, and this is rather about nested connectors causing the infinite loop, do you have the autocomplete wrapped inside a connectHits or similar @StarpTech ?

Haroenv commented 5 years ago

For finding out where the root issue comes from, I'll need more information, so can you create what you have so far in a sandbox environment? We have a template available for that purpose here. Thanks!

Otherwise, also note that we have a component specifically for autocompletes in a single shot: connectAutoComplete

StarpTech commented 5 years ago

Is there a reason the example code has AutoComplete assigned twice?

No, fixed.

I think the example is missing out some things, and this is rather about nested connectors causing the infinite loop, do you have the autocomplete wrapped inside a connectHits or similar @StarpTech ?

No, I double checked it.

Otherwise, also note that we have a component specifically for autocompletes in a single shot: connectAutoComplete

Yeah, that would be my workaround but I have no access to isSearchStalled right?

StarpTech commented 5 years ago

This example produce the memory leak or at least an unresponsive browser https://codesandbox.io/s/react-instantsearch-app-niiq2 (Save your workspace before :smile: )

StarpTech commented 5 years ago

Yes, when I remove the keyUp handler in Autocomplete component it works :small_airplane:

StarpTech commented 5 years ago

@Haroenv comment in/out the event handler to see the effect. The service worker will run out of memory.

     <CustomSearchBox
        onKeyUp={handleKeyUp}
Haroenv commented 5 years ago

not sure if it was fully clear, but if you use connectAutoComplete, you no longer use connectSearchBox, since it's a superset, and will indeed cause these infinite loops (#137, algolia/instantsearch.js#5266, unfortunately very hard to solve).

I fixed your code sandbox: https://codesandbox.io/s/react-instantsearch-app-jexcd

StarpTech commented 5 years ago

Hi @Haroenv thanks for investigation. This is very confusing and frustrating because I can't find a hint in the documentation and the bug costs me more than 100K of my search contingent. If the bugs can't be fixed for years then the implementation should be overhauled.

Haroenv commented 5 years ago

It's something that we work on, but has a lot of unanswered questions on when requests should be done. This is not something we can just "fix". Please contact support@algolia.com mentioning this ticket about the search requests you didn't expect to be done during development, we can fix that.

tkrugg commented 5 years ago

closing this for now as it's getting pretty old. For anyone stumbling on a similar problem, the autocomplete experience is documented here:

https://www.algolia.com/doc/guides/building-search-ui/resources/ui-and-ux-patterns/in-depth/autocomplete/react/#results-page-with-autocomplete