appbaseio / reactivesearch

Search UI components for React and Vue
https://opensource.appbase.io/reactivesearch
Apache License 2.0
4.9k stars 466 forks source link

Controlled DataSearch autosuggestions not debounced #1360

Open gauthierm opened 4 years ago

gauthierm commented 4 years ago

Affected Projects React

Library Version: 3.x

Describe the bug When you use a <DataSearch> as a controlled component with value and onValueChange props the componentDidUpdate method calls setValue with isDefaultValue=true.

This means de-bouncing is ignored for the autosuggest query and causes the interface to be slow and places extra load on Elasticsearch.

Changing the isDefaultValue value to false in the componentDidUpdate method fixes the issue for me. I'm not sure how this parameter is used as it is explicitly set to true everywhere it is called inside DataSearch. The debounced key handler is only ever used if it is passed to setValue as false.

Expected behavior When using <DataSearch> as a controlled component, autosuggest queries should be debounced according to the value set on the debounce prop.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dangreaves commented 4 years ago

This is an ongoing issue for us and would like for this not to become stale.

We need the input to be controlled for various reasons that we need the state to be external.

Adding a debounce should work in this scenario.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dangreaves commented 3 years ago

Again, this is an ongoing issue for us and would like for this not to become stale.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dmoskovtsov commented 2 years ago

+1 to this issue! Is there anything we can do to help move it forward?

siddharthlatest commented 2 years ago

@dmoskovtsov If you can share a reproducible example with the issue, that would help a lot. We will look at this on priority.

tdr2d commented 2 years ago

Hello, I also noticed a problem on the debounce prop. It does not debounce at all, even when commenting the onChange callback.

See this demo :

click on codesanbox demo

        <DataSearch
          title="DataSearch"
          dataField={["original_title", "original_title.search"]}
          componentId="BookSensor"
          URLParams
          size={5}
          debounce={1000}
          // onChange={(value, triggerQuery, event) => {
          //   this.setState(
          //     {
          //       value
          //     },
          //     () => triggerQuery()
          //   );
          // }}
        />

The code sets 1 second of debounce, but still it post 1 query per character when I type fast on the search bar.

tdr2d commented 2 years ago

I guess, this is simply because the parent component re-renders every time the value changes. Thus re-creating the DataSearch component and not applying the debounce.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

WGriffing commented 2 years ago

I don't believe this is completed.

If there's a recommended workaround, I'd love to know.

If this is truly a "won't fix" item, I'd love to know that too. I see it has that label, but it seems the stale bot applied it, so not sure if I should take it to heart.

WGriffing commented 2 years ago

Does anyone know if this issue is resolved by https://github.com/appbaseio/reactivesearch/pull/2063 ? I haven't had a chance to try the new version but the release notes for https://github.com/appbaseio/reactivesearch/releases/tag/v3.39.0 looked promising.

WGriffing commented 2 years ago

Based on my testing (I tested 3.40.1), this was resolved by #2064 and can be closed. Thank you very much for addressing this issue.