Fix issue 128 where the autocomplete was one step behind the input

[alexjg]

This fixes #128 where the autocomplete was one step behind the input due to a componentWillReceiveProps handler overwriting the entryValue. The fix has been to take Typeahead.state.visible and make it a method call so it can be calculated based on the state at render time.

[fmoo]

Should be getVisibleOptions() for consistency with other accessors ?

Also, whitespace between () and {

[fmoo]

This looks good overall. Can you run npm run benchmark before and after so we can know how if we're looking at a significant performance regression as a result?

cc @growlsworth

[alexjg]

I've updated the name and fixed the whitespace issue. I can't see any script for benchmarking, either I'm missing something or it needs to be set up. I'm happy to set something up, especially if anyone has an example of a benchmarking setup on a project like this for me to shamelessly copy.

[trshafer]

I thought I still had my benchmark for #125 . I can recreate it and send it here.

[trshafer]

npm install benchmark

And here's a benchmark shell with an initialized typeahead.

var Benchmark = require('benchmark');
var React = require('react');

var ReactTypeahead = require('../lib/react-typeahead').Typeahead;
var TypeaheadElement = React.createElement(ReactTypeahead);
var props = {
  options: [],
};
var typeaheadInstance = new TypeaheadElement.type(props);

console.log("running benchmark");

// example from: http://benchmarkjs.com/
var suite = new Benchmark.Suite();
suite.add('Run 1', function() {
  typeaheadInstance.getInitialState()
})
// .add('Run 2', function() {
// })
// .add('Run 3', function() {
// })
.on('complete', function() {
  console.log('Fastest is ' + this.filter('fastest').pluck('name'));
})
.run();

console.log("done");

[alexjg]

Have added a simple benchmark which renders the component into a jsdom document, enters characters and asserts the typeahead options showing. With both this code and master the mean runtime is 0.005.

The benchmark can be run with npm benchmark. At the moment it just spits out the mean runtime, obviously there are more interesting things which could be done but just in the interest of getting this PR merged I've not done anything more.

[alexjg]

@fmoo @growlsworth Has either of you had a chance to review this?

[trshafer]

Apologies @alexjg. didn't know you were looking for feedback. I'm in family vacation at the moment. Will check back in a week. If you don't hear back ping me again.

[fmoo]

Bump on this all.

I ended up merging in #163 which solved this, but broke backwards compatibility.

I'm reverting that and wondering if this would be a better fix for 1.1.x.? Otherwise, I'll remerge #163 and #167 as 2.0.x

cc @nosilleg , @jeffsaracco

[fmoo]

Hey @thomasjshafer , @alexjg . I recently merged in #163 which did something similar to this, but broke backwards compatibility in 1.1.6.

I'm going to revert it for 1.1.7, and was going to reapply along with #167 for a 2.0.0(-beta1?) release, but wondering if you had any thoughts on how that approach compares to this one.

cc @nosilleg for his thoughts as well.