Merge pull request #468 from MareStare/fix/abort-server-side-suggestions-when-popup-is-hidden

Abort the server-side completions when the autocomplete popup needs to be hidden

assets/js/autocomplete/__tests__/context.ts

@@ -6,7 +6,7 @@ import { fireEvent } from '@testing-library/dom';
 import { assertNotNull } from '../../utils/assert';
 import { TextInputElement } from '../input';
 import store from '../../utils/store';
-import { GetTagSuggestionsResponse } from 'autocomplete/client';
+import { GetTagSuggestionsResponse, TagSuggestion } from 'autocomplete/client';
 
 /**
  * A reusable test environment for autocompletion tests. Note that it does no
@@ -44,27 +44,30 @@ export class TestContext {
       }
 
       const url = new URL(request.url);
-      if (url.searchParams.get('term')?.toLowerCase() !== 'mar') {
-        const suggestions: GetTagSuggestionsResponse = { suggestions: [] };
-        return JSON.stringify(suggestions);
-      }
+      const term = url.searchParams.get('term');
+
+      const termLower = assertNotNull(term).toLowerCase();
+
+      const fakeSuggestions: TagSuggestion[] = [
+        {
+          alias: 'marvelous',
+          canonical: 'beautiful',
+          images: 30,
+        },
+        {
+          canonical: 'mare',
+          images: 20,
+        },
+        {
+          canonical: 'market',
+          images: 10,
+        },
+      ];
 
       const suggestions: GetTagSuggestionsResponse = {
-        suggestions: [
-          {
-            alias: 'marvelous',
-            canonical: 'beautiful',
-            images: 30,
-          },
-          {
-            canonical: 'mare',
-            images: 20,
-          },
-          {
-            canonical: 'market',
-            images: 10,
-          },
-        ],
+        suggestions: fakeSuggestions.filter(suggestion =>
+          (suggestion.alias || suggestion.canonical).startsWith(termLower),
+        ),
       };
 
       return JSON.stringify(suggestions);

assets/js/autocomplete/__tests__/server-side-completions/debouncing.spec.ts

@@ -0,0 +1,71 @@
+import { init } from '../context';
+
+it('ignores the autocompletion results if Escape was pressed', async () => {
+  const ctx = await init();
+
+  // First request for the local autocomplete index was done
+  expect(fetch).toHaveBeenCalledTimes(1);
+
+  await Promise.all([ctx.setInput('mar'), ctx.keyDown('Escape')]);
+
+  // The input must be empty because the user typed `mar` and pressed `Escape` right after that
+  ctx.expectUi().toMatchInlineSnapshot(`
+    {
+      "input": "mar<>",
+      "suggestions": [],
+    }
+  `);
+
+  // No new requests must've been sent because the input was debounced early
+  expect(fetch).toHaveBeenCalledTimes(1);
+
+  await ctx.setInput('mar');
+
+  ctx.expectUi().toMatchInlineSnapshot(`
+    {
+      "input": "mar<>",
+      "suggestions": [
+        "marvelous → beautiful  30",
+        "mare  20",
+        "market  10",
+      ],
+    }
+  `);
+
+  // Second request for the server-side suggestions.
+  expect(fetch).toHaveBeenCalledTimes(2);
+
+  ctx.setInput('mare');
+
+  // After 300 milliseconds the debounce threshold is over, and the server-side
+  // completions request is issued.
+  vi.advanceTimersByTime(300);
+
+  await ctx.keyDown('Escape');
+
+  expect(fetch).toHaveBeenCalledTimes(3);
+
+  ctx.expectUi().toMatchInlineSnapshot(`
+    {
+      "input": "mare<>",
+      "suggestions": [],
+    }
+  `);
+
+  ctx.setInput('mare');
+
+  // Make sure that the user gets the results immediately without any debouncing (0 ms)
+  await vi.advanceTimersByTimeAsync(0);
+
+  ctx.expectUi().toMatchInlineSnapshot(`
+    {
+      "input": "mare<>",
+      "suggestions": [
+        "mare  20",
+      ],
+    }
+  `);
+
+  // The results must come from the cache, so no new fetch calls must have been made
+  expect(fetch).toHaveBeenCalledTimes(3);
+});

assets/js/autocomplete/__tests__/server-side-completions/smoke.spec.ts

@@ -1,4 +1,4 @@
-import { init } from './context';
+import { init } from '../context';
 
 it('requests server-side autocomplete if local autocomplete returns no results', async () => {
   const ctx = await init();

assets/js/autocomplete/index.ts

@@ -229,18 +229,23 @@ class Autocomplete {
       // We use this method instead of the `focusout` event because this way it's
       // easier to work in the developer tools when you want to inspect the element.
       // When you inspect it, a `focusout` happens.
-      this.popup.hide();
+      this.hidePopup('The user clicked away');
       this.input = null;
     }
   }
 
+  hidePopup(reason: string) {
+    this.serverSideTagSuggestions.abortLastSchedule(`[Autocomplete] Popup was hidden. ${reason}`);
+    this.popup.hide();
+  }
+
   onKeyDown(event: KeyboardEvent) {
     if (!this.isActive() || this.input.element !== event.target) {
       return;
     }
     if ((event.key === ',' || event.code === 'Enter') && this.input.type === 'single-tag') {
-      // Coma means the end of input for the current tag in single-tag mode.
-      this.popup.hide();
+      // Comma/Enter mean the end of input for the current tag in single-tag mode.
+      this.hidePopup(`The user accepted the existing input via key: '${event.key}', code: '${event.code}'`);
       return;
     }
 
@@ -265,7 +270,7 @@ class Autocomplete {
         return;
       }
       case 'Escape': {
-        this.popup.hide();
+        this.hidePopup('User pressed "Escape"');
         return;
       }
       case 'ArrowLeft':
@@ -340,7 +345,14 @@ class Autocomplete {
     // brief moment of silence for the user without the popup before they type
     // something else, otherwise we'd show some more completions for the current term.
     this.input.element.focus();
-    this.popup.hide();
+
+    // Technically no server-side suggestion request can be in flight at this point.
+    // If the user managed to accept a suggestion, it means the user already got
+    // presented with the results of auto-completions, so there is nothing in-flight.
+    //
+    // Although, we don't make this a hard assertion just in case, to make sure this
+    // code is tolerant to any bugs in the described assumption.
+    this.hidePopup('The user accepted the existing suggestion');
   }
 
   updateInputWithSelectedValue(this: ActiveAutocomplete, suggestion: Suggestion) {