Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML Reporter: Faster and fuzzier module dropdown filter #1685

Merged
merged 1 commit into from Apr 10, 2022
Merged

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Apr 10, 2022

Threshold

Before After
Before: No results for "support for pomise" After: Various results even for "suortprose eachwhit"

I love the super fast fuzzysort.js library. Previously we ommitted results with a score lower than -10000. fuzzysort does not return results that don't contain the input characters, so names that don't match the input at excluded no matter what. The previous threshold more or less correlated with allowing some letters to be skipped in the first word, and leaving off letters from the end of later words. For example, suort for promise matched support for promise. But support for pomise already yielded zero results, despite only missing one letter! I've simply removed the threshold. More details in the commit message.

Debounce

Before After
Kapture before Kapture after

Previously there was a debounce of 200ms. The fuzzysearch algo takes ~1ms even when there are hundreds of test modules in the dataset. I considered removing the debounce entirely, but that would risk an ever-growing backlog of old events with both the input field and the results very far behind. Debounce demo on Codepen. More details in the commit message.

**Threshold:**

This was previously set at -10000, which more or less correlated with
allowing one letter to be skipped in the first word, and leaving off
a letter off the end of later words. For example, "suort for promise"
matched "support for promise". But "support for pomise" would already
result in zero results, despite only missing one letter.

There are two key points that lead me to simply remove the threshold:

1. We don't set a limit. It is merely meant as a filter.

   This means, unlike for fuzzysearch use cases such as a dictionary
   or text editor with potentially millions of files, we are okay with
   rendering all available options. This in fact what we do already
   when the input field as empty, as it is by default.

2. fuzzysort only ever returns results for which each of the input
   characters matched something. I don't know if this is true in
   theory, but in practice it appears that way. It seems then that we
   have nothing to lose by displaying all results it gave us.

   Names that don't contain some of the input characters aren't given
   a bad score, they're simply not in the results at all.

I've set `allowTypo: true` explicitly, though this was already the
default. Unlike what this sounds like, it doesn't allow for
mismatching input, eg. "add nostid" isn't given a bad score, it
is never matched. The typo algo allows for characters to be skipped
is all.

**Timeout**

Previously there was a debounce of 200ms. The algo takes ~1ms even
when there are hundreds of modules in the dataset. We could remove
the debounce timeout entirely, I think. But let's hold off on that
for now. I've lowered it to 0ms which should at least make sure
there categorically cannot be an exponentially growing backlog of
unprocessed and blocking input text to render.

It can be confusing why this would matter at all since a ~0ms timeout
is realistically going to fire after every keystroke (which is good,
that keeps it nice and responsive in realtime), but then how does it
help compared to fully synchronous? Well, it helps because if the
thing we're doing during the event handler is so slow that it is still
running while the user has typed other characters, then once our
current handler is finished, all remaining queued up event handlers
for the `input` event will result in 1 single timeout being queued up
for the rest (based on what `search.value` will be at the end, when
the timeout fires).

Without this, we would be replaying every keystroke since
search.value will not reflect the latest value, only the value at the
time the browser event was queued. Demo to see this in action:
<https://codepen.io/Krinkle/pen/wvpXxwM?editors=0010>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant