fmoo / react-typeahead

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

Performance when receiving same props #124

Closed trshafer closed 8 years ago

trshafer commented 9 years ago

Hey @fmoo,

There's a significant performance issue. I have a parent component to the tokenizer which re-renders frequently. In my situation, it is improbable to change this parent component to update less frequently. My re-render is extremely fast and usually doesn't update the dom. I've tested it when I re-render my component every keydown and there is no performance slowdown with the tokenizer removed (separate input field). I believe the issue is that every time I pass the same props to the tokenizer, it calls a fuzzy match on all of my options. It happens because of this line: https://github.com/fmoo/react-typeahead/blob/master/src/typeahead/index.js#L283:L287 .

Some possible solutions:

  1. Short circuit this check when the entryValue is empty.
    • Personally, this will probably work in my use case.
  2. Have some smarter check to see if there is actually any props difference in the rerender.
  3. Make the search faster.
    • I think this is inadvisable at the current time.
  4. Other ideas?

Thanks!

thehuey commented 9 years ago

Do a performance test with option 1 and see if that resolves your issue. If so, I think that makes sense to not attempt a match on empty entry. You could get a bit fancy and make the short circuit check a function defaulting to empty entry.

On Tuesday, September 1, 2015, Thomas Shafer notifications@github.com wrote:

Hey @fmoo https://github.com/fmoo,

There's a significant performance issue. I have a parent component to the tokenizer which re-renders frequently. In my situation, it is improbable to change this parent component to update less frequently. My re-render is extremely fast and usually doesn't update the dom. I've tested it when I re-render my component every keydown and there is no performance slowdown with the tokenizer removed (separate input field). I believe the issue is that every time I pass the same props to the tokenizer, it calls a fuzzy match on all of my options. It happens because of this line: https://github.com/fmoo/react-typeahead/blob/master/src/typeahead/index.js#L285 .

Some possible solutions:

  1. Short circuit this check when the entryValue is empty.
    • Personally, this will probably work in my use case.
  2. Have some smarter check to see if there is actually any props difference in the rerender.
  3. Make the search faster.
    • I think this is inadvisable at the current time.
  4. Other ideas?

Thanks!

— Reply to this email directly or view it on GitHub https://github.com/fmoo/react-typeahead/issues/124.

trshafer commented 9 years ago

This is what I've got so far: https://github.com/amplii/react-typeahead/commit/4bd61ddb766eef26bfd207faa8e865fd2d7f6a06

trshafer commented 8 years ago

Issues resolved in #125.