React-bootstrap-typeahead: Debounced `onSearch` handler being cancelled prematurely in AsyncTypeahead

[ivanjuliant]

I just want to tell you if this not working it because i try install last version, and i check on package,json i see this "react-bootstrap-typeahead": "^5.0.0-rc.3" when i try using this version and run example it's not working, but after i change version "react-bootstrap-typeahead": "4.1.0" its running, so maybe there is an issue on new version for OnSearch and OnPagination options, thank you for making awesome plugin.

[ericgio]

Hi @ivanjuliant, thanks for the report. Can you please provide more specific steps to reproduce the issue you're seeing? When I load the v5.0.0-rc.3 examples locally, both onSearch and onPaginate work correctly.

[jbogdani]

Hello,
I think I'm having the same issue. I jumped from v. 5.0.0-rc.2 to v. 5.0.0 and the component stopped working. I found that my problem was related to the onInputChange attribute, which was set. Removing it, made everything work fine.

[ericgio]

Hi @jbogdani, can you please post a code example or sandbox link with a minimal repro please? Thanks!

[jbogdani]

Digging deeper into this I found it was an error of mine. I was using onInputChange to get the value together with onChange. They were calling the same callback function to set the application state on a higher level of my application. This was working up to 5.0.0-rc.2, but prevented somehow the firing of onSearch in 5.0.0. onInputChange works as expected, the error was on my side. Probably this helps also @ivanjuliant.

[Sarthak-Agrawal]

@ericgio I was able to reproduce the issue in the Async sample application itself on the version 5.0.0 and 5.0.0-rc.3. Link to the codesandbox.
The search box is not working in the code and the diff from the async example mentioned in the documentation, is the version(4,.1.0 => 5.0.0-rc3) and the console log as written in _handleSearch.

[ericgio]

@Sarthak-Agrawal: thanks for the sandbox link, I can repro the issue now. From what I can tell, onInputChange is being called immediately when a user types. The setState call there is triggering a re-render of the typeahead, which short-circuits the debounced search function such that it is never called.

I need to dig into it a little more, but it appears that with v5.0, the typeahead is always being re-rendered when the parent re-renders, which was not the case in v4.x.

As a workaround, I was able to fix the symptom by storing query as a class variable rather than in state. This avoids re-rendering the parent when the input value changes, but still makes the value available.

[ericgio]

This should be fixed in v5.1. Thanks again for reporting the issue!

[jbogdani]

It seems to the that onInputChange still prevents the execution of onChange, even in v. 5.1.0

[ericgio]

🤦

[ericgio]

@jbogdani: updating @Sarthak-Agrawal's sandbox to v5.1 solves the problem for me. Just guessing, but if you're using a functional component instead of a class, try wrapping your onSearch handler function with useCallback and be sure to include the dependency array.

I spent a bunch more time digging into this and I think I understand what's happening. I believe the bug has actually been fixed and the code works as expected, but the behavior is still potentially confusing. The root problem is that when the onSearch handler passed into the component changes, it triggers useEffect's cleanup function, which cancels the debounced search function. This happens on a simple re-render unless the handler is either bound (in a class component) or uses useCallback (in a functional component).

Example

const Example = () => {
  const [isLoading, setIsLoading] = useState(false);
  const [query, setQuery] = useState('');

  // `useCallback` is required here, otherwise it will change on every render
  // and cancel the debounced function.
  const handleSearch = useCallback((q) => {
    console.log('handleSearch');
  }, []);

  const handleInputChange = (q) => {
    console.log('onInputChange');
    setQuery(q);
  };

  return (
    <AsyncTypeahead
      id="example"
      isLoading={isLoading}
      minLength={2}
      onInputChange={handleInputChange}
      onSearch={handleSearch}
      options={[...]}
    />
  );
};

The two approaches I see are:

1. Remove onSearch from the dependency array in useAsync. This works in most cases, but is "wrong" from React's perspective, since onSearch could change. If that happens, it won't be updated and could create an even more subtle behavioral issues:

You can still wrap it in useCallback to fix the issue. Remember that technically it is valid for a function to change, and you can’t ignore this case without risking bugs... Ignoring function dependencies completely leads to worse bugs with function components and Hooks because they would keep seeing stale props and state if you do that. So try not to, when you can.

1. Even though I believe the behavior is now "correct", it's potentially confusing. I should probably warn consumers that onSearch is changing and may be triggering unexpected behavior. The problem there is that in some cases it may be valid to change onSearch.

[jbogdani]

Thank you very much! The use of useCallback really solved my issue.
Great component, wonderfully maintained :)

[ericgio]

I've added a warning to the v5 upgrade guide for now, since the new behavior is a potentially breaking change introduced in the latest major version. I'll see how things go and if a lot of people have problems I may consider adding a warning in the code itself.