fmoo / react-typeahead

Pure react-based typeahead and typeahead-tokenizer
ISC License
677 stars 232 forks source link

Version 1.1.6 breaks the typeahead when fetching options via API #165

Closed jeffsaracco closed 8 years ago

jeffsaracco commented 8 years ago

It seems that ed49656 (and subsequently version 1.1.6) breaks this for me when doing an ajax search. The results seem to flash there when key up is fired then disappear again. Hard coding 1.1.5 fixes it for us just thought you should be aware.

This is a react app with flux that, on key up, fetches the next set of options from an API and re-renders the react component containing the typeahead using the new options.

fmoo commented 8 years ago

@jeffsaracco - Ack; I think if we had better test cases, we could catch these types of issues when people propose new functionality.

Any idea on how to fix the race condition without breaking support for remote fetches?

cc @nosilleg

nosilleg commented 8 years ago

@jeffsaracco Do you have some code that you can show us?

So I understand. You're setting the onKeyUp prop to a function that does an ajax request which then sets a new value for the options prop?

I'm happy to try and debug and create some test cases if I can be sure that they will match your usage.

jeffsaracco commented 8 years ago

@nosilleg here is the part of our code... it's pretty straightforward, but I'll also try creating a jsbin or something so that you can see what I mean...

  getOptions(value) {
    if(!_.isEmpty(_.trim(value))) {
      // ajax call here to fetch new options
    }
  }

  handleChange() {
    let that = this;

    return (event) => {
      let value = event.target.value;
      that.setState({inputValue: value});
      that.getOptions(value);
    }
  }

  render() {
    let options = Store.getOptions();

    return (
        <Typeahead
          ref='typeahead'
          onKeyUp={this.handleChange()}
          displayOption='label'
          options={options}
        />
    );
  }
nosilleg commented 8 years ago

@jeffsaracco I've tried modifying my setup to approximate the above, but I can't get it to fail. Are you passing inputValue back in as well? (I can understand a flash, but I would expect the correct result at the end of it.)

eladlevy commented 8 years ago

I'm experiencing the same issue with almost identical use case and implementation as @jeffsaracco described.

nosilleg commented 8 years ago

Are either of you using onBlur? What happens if you change onKeyUp to onChange?

nosilleg commented 8 years ago

I expect that you'll see better behaviour with https://github.com/fmoo/react-typeahead/pull/167 as proper tracking is done with the value now, instead of the change in 1.1.6 which effectively dropped the value between renders.

jeromebridge commented 8 years ago

I'm also seeing this issue using async load of options and redux (version 1.1.6 of typeahead) I downgraded to 1.1.5 to get around the issue.

fmoo commented 8 years ago

Ok, I've reverted and published as 1.1.7 (available on both branch master, and 1.1.x)

eladlevy commented 8 years ago

Thanks :+1: