React-instantsearch: infinite loop when rendering a conditional loading message with children

[blech75]

Do you want to request a feature or report a bug?

Bug.

Bug: What is the current behavior?

(See below for full description.)

Attempting to use a connected "loading message" component (one that renders static content when loading and its children when finished) causes an infinite loop in certain circumstances.

• when children contains the <Stats /> component, searchResults.searching is set as expected.
• when children contains the <Hits /> and <Pagination /> components (possibly others), searchResults.searching flips back and forth ad infinitum.

When the "loading message" component is configured to render static content upon completion, the infinite loop does not occur.

Bug: What is the expected behavior?

searchResults.searching should not flip-flop between true and false in an infinite loop when rendering children.

Bug: What browsers are impacted? Which versions?

Observed in Chrome 59.0.3071.109 & Safari 10.1.1.

What is the version you are using? Always use the latest one before opening a bug issue.

Observed in react-instantsearch 4.0.3 and 4.0.4.

---

Background

(copied/adapted from discussion on community forum.)

Using the example connector provided in your docs:

const connectLoading = createConnector({
  displayName: 'ConditionalError',
  getProvidedProps(props, searchState, searchResults) {
    return {
      loading: searchResults.searching
    };
  }
});

I then create a component that renders a message when loading, and its children when complete:

const LoadingMessage = connectLoading(({ loading, children }) =>
  <div>
    {loading ? <p>Loading...</p> : children }
  </div>
);

Now we make use of it in a minimal UI where we just display the stats:

<InstantSearch ...>
  <SearchBox />

  <div className="SearchResults">
    <LoadingMessage>
      <p>FINISHED!</p>
      <Stats />
    </LoadingMessage>
  </div>

</InstantSearch>

You can see this example here in a codepen: https://codepen.io/wrksprfct/full/jwmmox/. This works as expected. Load the page and you see "Loading..." and then "FINISHED! X results found in Yms". Ensure you have web console open and you'll see the XHRs fly by and searchResults.searching toggle between true and false when things change state. Type "avocado" into the box: the UI updates and the console logs as you'd expect.

Now here's the fun part. Let's replace the <Stats /> component with the <Hits /> component, so it looks like this:

<InstantSearch ...>
  <SearchBox />

  <div className="SearchResults">
    <LoadingMessage>
      <p>FINISHED!</p>
      <Hits />
    </LoadingMessage>
  </div>
</InstantSearch>

You can see this example in another codepen: https://codepen.io/wrksprfct/full/yXbXNQ. !!!BEWARE!!! before loading that page, be prepared to kill it via Chrome's Task Manager or your OS's equivalent because it gets into an infinite loop and consumes CPU & gobs of memory quickly. Again, ensure you have the web console open.

So, upon loading that page, you'll see two XHRs instead of one (identical, AFAICT), and then searchResults.searching flip between false and true ad infinitum.

If, in this problematic example, you hardcode the loading prop to false:

getProvidedProps(props, searchState, searchResults) {
  return {
    loading: false
  };
}

...everything works. Well, except for seeing the loading message... which is the whole point of this exercise!

Again, the only difference is the switch from <Stats /> to <Hits />. I looked at their sources, but could not find anything interesting.

Interestingly, the same problematic behavior occurs with <Pagination />. See https://codepen.io/wrksprfct/full/zzzOWm/. I did not test any others yet.

Now... if we update the LoadingMessage component to not render children:

const LoadingMessage = connectLoading(({ loading }) =>
  <div>
    {loading ? <p>Loading...</p> : <p>DONE!</p> }
  </div>
);

... then everything works fine:

<InstantSearch ...>
  <SearchBox />

  <div className="SearchResults">
    <LoadingMessage />
  </div>
</InstantSearch>

You can see this example in action in this codepen: https://codepen.io/wrksprfct/full/WOOajx.

[mthuret]

Fix in 4.0.6

[mthuret]

The fix was not the good one.

[ghost]

Hello,
I also just stumbled upon the exact same problem.
I get an infinite loop when rendering <Hits> as the child.
@blech75 did you find a workaround?

[ghost]

I got the the exact same problem when trying to add a Loading message. Is there any ETA on when this will be fixed?

[samouss]

@blech75 @roma98 @reinvanimschoot

We are currently looking into the issue, we found where the problem came from. But for the moment we don't have a proper fix. The problem is inherent of how React InstantSearch works. It's related to how the query parameters are computed & the lifecycle of the components.

As a workaround for the example provided by @blech75 you need to avoid to mount / unmount the component on each change of the props searching. You can simply hide the Hits component with the help of styles instead of remove it on each changes. The fact that the component is mounted / unmounted trigger the infinite loop. I made an example showing how to avoid the problem with the workaround proposed above.

const Loading = connectStateResults(({ searching, children }) => (
  <div>
    <p style={{ display: searching ? 'block' : 'none' }}>
      Loading...
    </p>

    <div style={{ display: searching ? 'none' : 'block' }}>
      {children}
    </div>
  </div>
));

const App = () => (
  <InstantSearch>
    <Loading>
      <Hits hitComponent={Product} />
      <Pagination />
    </Loading>
  </InstantSearch>
);

https://codesandbox.io/s/7yzwl0kom0

[blech75]

@samouss interesting, and thanks for the update!

this workaround -- using display to show/hide elements -- though not optimal or intuitive, makes sense. i can't dedicate time to applying this approach to my project right now, but i'll be sure to follow up when i do.

[nicosommi]

Same issue here. As a workaround I took the pagination component off the connectStateResults function and connected it individually with connectPagination, but is also not optimal.

[wittydeveloper]

I had the same kind of issue with :

export const SearchResults = connectStateResults(connectHits(SearchResultsComponent));

The workaround was to do :

export const SearchResults = connectHits(connectStateResults(SearchResultsComponent));

💯

[wzup]

Same here.
connectStateResults is infinite loop.
Always returns searching=true case.

const Content = connectStateResults(
  ({ searchState, searching, onSearchStateChange, getSelected }) => {
    if(searching) {
      return <Text>
        SEACHING...
      </Text>
    } else {
      return (
        <InfiniteHits onSearchStateChange={ onSearchStateChange }
                      searchState={ searchState }
                      getSelected={ getSelected }/>
      )
    }
  }
);

              <InstantSearch
                appId="..."
                apiKey="..."
                indexName="..."
                onSearchStateChange={ this.props.onSearchStateChange }
                searchState={ this.props.searchState }
                refresh={ true }
                // resultsState={ { query: '...' } }
              >
                { /*<RefinementList attribute="..." headerText={ '...' } />*/ }
                <TouchableOpacity
                  onPress={ () => {
                    this.props.setModalVisible(false);
                  } }
                >
                  <Text style={ { marginTop: 20, height: 50, alignSelf: 'center', fontSize: 14 } }>
                    Close
                  </Text>
                </TouchableOpacity>
                <SearchBox />
                <Content
                  onSearchStateChange={ this.props.onSearchStateChange }
                  searchState={ this.props.searchState }
                  getSelected={ this.props.getSelected }
                />
              </InstantSearch>

[GriffinSauce]

Running into this problem as well. I'm implementing the display:none workaround but just wondering: could React InstantSearch show a warning for this? That might save a lot of frustration, it took me quite some time to find what was causing this issue.

[samouss]

@GriffinSauce Sorry about that. For the question about the warning it's indeed a good idea but it's a bit hard to detect when we want to warn people about a possible infinite loop. Mainly because this is the "normal" lifecycle of the widgets. Once a widget is removed we have to trigger a new search to update the widgets accordingly.

[GriffinSauce]

Yeah I couldn't figure out a quick idea to do that either. Another idea is to look at the outcome, a massive amount of successive searches, and to trigger a warning that there might be an issue.

Warning: a lot of searches were triggered in short succession, you might have an infinite loop because of nested connected components. [link to docs]

Far from a perfect solution but it would be cheap to do and might mitigate the issue for future users.

Thanks for the support either way!

[jwoo0122]

Is there any updates or solution for this issue?