search only uses event.target so you should be able to avoid using persist if you extract it.

aha! I struggled to figure this out, I thought because of React's synthetic events I had to persist when debouncing... So if I extract search hereI won't need the event later. I'll change that.

I wonder if all these debounce's are really needed, the search code is already pretty efficient and tends to exit early when it can.

yes, when typing a search term with an asynchronous source, debouncing is appropriate. before adding the debounce I was getting a search fired for every character I typed. When I quickly type 'cat' I would expect only one search for 'cat' not three searches for 'c', 'ca', and 'cat'

Yes, but in that case the debounce should be in user search code in autocompleters, not in the general code in autocomplete. The general code may still be used to search things that have a fixed list (ie the forward-slash block completer).

Yea, good point. I'll move it there.

To make this clearer to read in the multiple places it is used I would extract this.props.completers[ open.idx ] into a constant, ie:

const completer = this.props.completers[ open.idx ];

As this code is asynchronous I would not reuse the filteredOptions variable (call it asyncFilteredOptions or something) because the code that comes after this async block on the page will not get access to the updated value.

Right, good point, I'll rework this part.

I know my code probably made a similar bad assumption but just because the lengths are the same the item at that position may not be the same - something that will probably be more noticeable with the new search code. It might be desirable to search the filtered options?

yea, good point, the slug should be unique. i'll work on this some more.

I removed my copy of this code block, the length check seems to work, I guess in some cases it might show the incorrect selection?

The original intent of adding the key here was that each unique option would have a unique key that was not effected by the filtering - this effects the ARIA the most because it used the key to determine if the selection had changed and needed to be re-announced. Unfortunately the dynamic server-side search breaks this assumption and I am not sure how you will fix it.

Hmm, not sure i understand. the issue is the tool will not re announce the selection, or that it will over announce when the selection hasn't changed?

could we use the item slug here which should be unique?

I reworked the keying to use slugs if available, seems to work but needs testing

Ok, make sure it works in the general case (ie for the slash completer and the proposed hash-tag completer).

If you rename the asynchronously calculated output further down this doesn't need to be a let.

Why is getOptions a parameter? Why not just pass the search?

I am calling getOptions in the inner function, I couldn't seem to call it without passing, I'll double check this

removed setSearch entirely so this no longer applies

As before, why are you passing getOptions?

You can use ES6 shorthand for search field:

{ data: { search } }

Good point, changed

This is never used? Cause of your lint error.

Ah, that was lost in a merge, fixed and verified its working again.

What is slug here? Are we assuming a particularly-shaped REST API entity where the autocomplete options are otherwise generic?

Hmmm, I'm not sure we need this maybe a remnant of a previous approach, I'll revert this change

This line is excessively long. Implicit object arrow returns are hard to distinguish from arrow function body. Dense single lines ought be undesirable.

With some breathing room, and clear labeling of what we're constructing via variable assignment of the suffix.

const keyedOptions = map( options, ( option, i ) => {
	const keySuffix = option.value.slug || i;

	return {
		...option,
		key: [ index, keySuffix ].join( '-' ),
	};
} );

Aside: Existed previously, but as a reader, I'm not sure what the difference between index and i are here at a glance. Maybe we ought to name each autocompleter instead of using its index as an identifier.

I renamed this variable autocompleterIndex to make it clearer.

also, unfurled that long line as you suggested to make it easier to read

Does this need to be encoded?

I assume you mean just the search part of the payload, for security? otherwise I wouldn’t think its needed, as we are searching usernames?

adjusted

Not necessarily for security. If someone entered @foo&bar, I assume it would (have previously) caused issue for the payload request.

ah, right '&' could cause unexpected results, good catch