prevent duplicate autocomplete initialization on first interaction - #732
Open
Montana wants to merge 2 commits into
Open
prevent duplicate autocomplete initialization on first interaction#732Montana wants to merge 2 commits into
Montana wants to merge 2 commits into
Conversation
Clicking the input fires both the container click listener and the input focus listener in the same gesture. The previous guard checked a DOM attribute that Algolia only sets after the dynamic import resolves, so both calls passed the guard and created two Autocomplete instances bound to the same input. Replace it with a synchronous boolean set before the first await.
actions/checkout@v3 and actions/setup-node@v3 run on the deprecated Node 16 runtime and emit warnings; bump both to v4. Also bump dependabot/fetch-metadata to v2 and enable npm caching in setup-node to speed up CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi StackExchange,
Two independent fixes:
Autocompleteinstances bound to the same input.Both are small and self-contained; they're split into separate commits.
1. Duplicate autocomplete initialization
Problem
The autocomplete is lazy-loaded on first interaction, with two
{ once: true }listeners:clicklistener on.js-autocomplete-containerfocuslistener on the input inside itA single mouse click on the input fires both: focus fires on mousedown, then the click bubbles up to the container. Both call
initializeAutocomplete().The existing guard was meant to dedupe:
But
type="search"is only set by Algolia after the dynamicimport()of the autocomplete module resolves, which is asynchronous. So when both listeners fire in the same gesture, both calls pass the guard before either finishes importing, and two instances get constructed on the same input (duplicate event listeners, double submits). Keyboard/tab users avoid it (focus only, no click); mouse users hit it on the first click.Fix
Replace the async DOM-attribute check with a synchronous boolean set before the first
await, so a re-entrant call returns immediately:2. Deprecated GitHub Actions
Problem
.github/workflows/main.ymlpinsactions/checkout@v3andactions/setup-node@v3, both of which run on the Node 16 runtime that GitHub has deprecated. They emit deprecation warnings in CI today and are on a removal path. The two dependabot workflows pindependabot/fetch-metadata@v1, likewise superseded by v2.Fix
actions/checkout@v3->@v4actions/setup-node@v3->@v4, pluscache: npm(the repo has a lockfile and usesnpm ci, so this is a free CI speedup)dependabot/fetch-metadata@v1->@v2in bothauto-upgrade-caniuse-lite.ymlanddependabot-auto-merge.ymlThe
fetch-metadatav2 bump is drop-in (same outputs).Testing
npm ci && npm run buildpasses locally.Notes / out of scope
While reviewing I also noticed
helpers/static-data.tstypesimageUrlasstringbutlogoMappingonly covers seven browsers, so any browser not in the map would render<img src="">. I confirmed this does not currently trigger: the@stackoverflow/browserslist-configquery explicitly excludes every unmapped browser, so the resolved set is exactly the seven that are mapped. Left it alone to keep this PR focused; happy to add a guard in a follow-up if you'd prefer the hardening.