mokkabonna / inquirer-autocomplete-prompt

Autocomplete prompt for inquirer
ISC License
350 stars 82 forks source link

bug: Result of previous search can override current results #153

Closed domsleee closed 1 year ago

domsleee commented 1 year ago

Salutations and thanks for this package πŸš€ My API is quite slow, and sometimes previous results from a different input override the latest results.

For example

  1. input: car
  2. input: care
  3. result of care is shown on screen
  4. result of car is shown on screen <-- bug

Reproduction

Consider this code:

import inquirer from "inquirer";
import inquirerPrompt from "inquirer-autocomplete-prompt";

async function main() {
  inquirer.registerPrompt("autocomplete", inquirerPrompt);
  await inquirer.prompt([
    {
      type: "autocomplete",
      name: "from",
      message: "Select a state to travel from",
      source: async (answersSoFar, input) => {
        return new Promise((resolve) => {
          const delay = tryParseInt(input);
          setTimeout(() => resolve(["hi", delay]), delay);
        });
      },
    },
  ]);
}

function tryParseInt(input) {
  try {
    return parseInt(input, 10);
  } catch {
    return 0;
  }
}

main();

I can use this code to go

  1. input: 2000
  2. Input: 200
  3. result: 200 is shown on screen
  4. result 2000 is shown on screen

The expected behaviour is that 200 remains on screen, since it is the latest result matching the search query

Suggested fix

Currently, thisPromise will always equal lastPromise, because it is captured in the closure. It should be using this.lastPromise instead:

diff --git a/node_modules/inquirer-autocomplete-prompt/index.js b/node_modules/inquirer-autocomplete-prompt/index.js
index 4efe1ec..42a856d 100644
--- a/node_modules/inquirer-autocomplete-prompt/index.js
+++ b/node_modules/inquirer-autocomplete-prompt/index.js
@@ -235,11 +235,11 @@ class AutocompletePrompt extends Base {
     }

     // Store this promise for check in the callback
-    const lastPromise = thisPromise;
+    this.lastPromise = thisPromise;

     return thisPromise.then((choices) => {
       // If another search is triggered before the current search finishes, don't set results
-      if (thisPromise !== lastPromise) return;
+      if (thisPromise !== this.lastPromise) return;

       this.currentChoices = new Choices(choices);
mokkabonna commented 1 year ago

Thanks! That definitively is a bug. A regression introduced here https://github.com/mokkabonna/inquirer-autocomplete-prompt/pull/134/files

It was working before. I missed that functional change. And it was not testet apperantly. Will fix and add test if possible.

mokkabonna commented 1 year ago

Released 3.0.1

https://github.com/mokkabonna/inquirer-autocomplete-prompt/releases/tag/v3.0.1

domsleee commented 1 year ago

Ah nice, thanks for the quick fix @mokkabonna