-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Components: Refine When Autocompleter Stops Matching #30969
Description
The autocompleter can cause performance issues when trying to match values in long strings. In #30649 the algorithm was updated to not attempt after a mismatch happened + providing ways to escape from that condition (to continue matching). This already covered most of the scenarios that caused a slow-down due to over-matching. It also will stop matching if a character was 50 characters away from the trigger as a least resort (though this scenarios is only reached in a couple of cases, see the PR/comments).
There are three parts to this issue:
Make the completer function testable
Let's isolate the completer function to be unit testable. (Note that unit tests are much more useful and less fragile than a component test for this example case). Unit tests will allow us to document supported use cases and give us more confidence if we'd like to try a different matching algorithm.
gutenberg/packages/components/src/autocomplete/index.js
Lines 449 to 535 in fe37913
| const completer = find( | |
| completers, | |
| ( { triggerPrefix, allowContext } ) => { | |
| const index = text.lastIndexOf( triggerPrefix ); | |
| if ( index === -1 ) { | |
| return false; | |
| } | |
| const textWithoutTrigger = text.slice( | |
| index + triggerPrefix.length | |
| ); | |
| const tooDistantFromTrigger = textWithoutTrigger.length > 50; // 50 chars seems to be a good limit. | |
| // This is a final barrier to prevent the effect from completing with | |
| // an extremely long string, which causes the editor to slow-down | |
| // significantly. This could happen, for example, if `matchingWhileBackspacing` | |
| // is true and one of the "words" end up being too long. If that's the case, | |
| // it will be caught by this guard. | |
| if ( tooDistantFromTrigger ) return false; | |
| const mismatch = filteredOptions.length === 0; | |
| const wordsFromTrigger = textWithoutTrigger.split( /\s/ ); | |
| // We need to allow the effect to run when not backspacing and if there | |
| // was a mismatch. i.e when typing a trigger + the match string or when | |
| // clicking in an existing trigger word on the page. We do that if we | |
| // detect that we have one word from trigger in the current textual context. | |
| // | |
| // Ex.: "Some text @a" <-- "@a" will be detected as the trigger word and | |
| // allow the effect to run. It will run until there's a mismatch. | |
| const hasOneTriggerWord = wordsFromTrigger.length === 1; | |
| // This is used to allow the effect to run when backspacing and if | |
| // "touching" a word that "belongs" to a trigger. We consider a "trigger | |
| // word" any word up to the limit of 3 from the trigger character. | |
| // Anything beyond that is ignored if there's a mismatch. This allows | |
| // us to "escape" a mismatch when backspacing, but still imposing some | |
| // sane limits. | |
| // | |
| // Ex: "Some text @marcelo sekkkk" <--- "kkkk" caused a mismatch, but | |
| // if the user presses backspace here, it will show the completion popup again. | |
| const matchingWhileBackspacing = | |
| backspacing && textWithoutTrigger.split( /\s/ ).length <= 3; | |
| if ( | |
| mismatch && | |
| ! ( matchingWhileBackspacing || hasOneTriggerWord ) | |
| ) { | |
| return false; | |
| } | |
| if ( | |
| allowContext && | |
| ! allowContext( text.slice( 0, index ), textAfterSelection ) | |
| ) { | |
| return false; | |
| } | |
| if ( | |
| /^\s/.test( textWithoutTrigger ) || | |
| /\s\s+$/.test( textWithoutTrigger ) | |
| ) { | |
| return false; | |
| } | |
| return /[\u0000-\uFFFF]*$/.test( textWithoutTrigger ); | |
| } | |
| ); | |
| if ( ! completer ) { | |
| reset(); | |
| return; | |
| } | |
| const safeTrigger = escapeRegExp( completer.triggerPrefix ); | |
| const match = text | |
| .slice( text.lastIndexOf( completer.triggerPrefix ) ) | |
| .match( new RegExp( `${ safeTrigger }([\u0000-\uFFFF]*)$` ) ); | |
| const query = match && match[ 1 ]; | |
| setAutocompleter( completer ); | |
| setAutocompleterUI( () => | |
| completer !== autocompleter | |
| ? getAutoCompleterUI( completer ) | |
| : AutocompleterUI | |
| ); | |
| setFilterValue( query ); | |
| }, [ textContent ] ); |
Benchmark matching performance
When we are interested in improving performance, we often need to trade some additional code complexity. It's important that we test our changes to make sure that a new approach actually is faster. In some cases, we can accidentally make things slower.
With the completer function isolated, let's benchmark how the quickly a proposed approach behaves against a variety of test strings.
Try to improve the matching algorithm
We might find that our current approach works well enough, but let's try a few others to see if there's an approach we like better. Note that any algorithm here is going to have some tradeoffs between correctness, speed, and complexity.
Another thing we can try is calling this expensive function less often:
- We're currently running this on every keypress. When a user is typing rapidly we probably want to get out of the way. I suspect folks will slow down on typing speed when trying to autocomplete. Tuning a debounce would likely make things feel more lively.
- Should we store more state to avoid running this whenever content changes? (Memo, detect a trigger, stop when a substring has no matches etc). The tradeoff here is that the more complex our approach is, the more likely we'll be wrong.