From 939a73b762968c13425fd550411378049b8049d3 Mon Sep 17 00:00:00 2001
From: Konstantin Vyatkin <tino@vtkn.io>
Date: Sat, 2 Nov 2019 08:58:18 -0400
Subject: [PATCH] fix setChoices flow (#730)

* fix setChoices flow

* ignore Promise as it gate checked

* re-run cypress
---
 public/assets/scripts/.eslintrc.js | 23 +++++++++++++----------
 src/scripts/choices.js             | 25 ++++++++++++++-----------
 src/scripts/choices.test.js        |  9 ++-------
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/public/assets/scripts/.eslintrc.js b/public/assets/scripts/.eslintrc.js
index 00f2747..d761728 100644
--- a/public/assets/scripts/.eslintrc.js
+++ b/public/assets/scripts/.eslintrc.js
@@ -1,4 +1,3 @@
-
 // get polyfill settings from top level config
 // @ts-ignore
 const { settings } = require('../../../.eslintrc.json');
@@ -6,19 +5,23 @@ const { settings } = require('../../../.eslintrc.json');
 // Adding non-polyfilable Symbol-related functions as they are most probably
 // behind the flag
 
-settings.polyfills.push('Symbol.toStringTag', 'Symbol.for', 'Object.getOwnPropertySymbols', 'Object.getOwnPropertyDescriptors')
+settings.polyfills.push(
+  'Symbol.toStringTag',
+  'Symbol.for',
+  'Object.getOwnPropertySymbols',
+  'Object.getOwnPropertyDescriptors',
+  'Promise', // Promise is gate checked
+);
 
-module.exports = /** @type {import('eslint').Linter.Config} */({
+module.exports = /** @type {import('eslint').Linter.Config} */ ({
   root: true,
-  extends: [
-    "plugin:compat/recommended"
-  ],
+  extends: ['plugin:compat/recommended'],
   parserOptions: {
     // ensure that it's compatible with ES5 browsers, so, no `const`, etc
-    ecmaVersion: 5
+    ecmaVersion: 5,
   },
   env: {
-    browser: true
+    browser: true,
   },
-  settings
-})
+  settings,
+});
diff --git a/src/scripts/choices.js b/src/scripts/choices.js
index 158aef0..1c16a01 100644
--- a/src/scripts/choices.js
+++ b/src/scripts/choices.js
@@ -550,19 +550,15 @@ class Choices {
       this.clearChoices();
     }
 
-    if (!Array.isArray(choicesArrayOrFetcher)) {
-      if (typeof choicesArrayOrFetcher !== 'function') {
-        throw new TypeError(
-          `.setChoices must be called either with array of choices with a function resulting into Promise of array of choices`,
-        );
-      }
-
-      // it's a choices fetcher
-      requestAnimationFrame(() => this._handleLoadingState(true));
+    if (typeof choicesArrayOrFetcher === 'function') {
+      // it's a choices fetcher function
       const fetcher = choicesArrayOrFetcher(this);
-      if (typeof fetcher === 'object' && typeof fetcher.then === 'function') {
+      if (typeof Promise === 'function' && fetcher instanceof Promise) {
         // that's a promise
-        return fetcher
+        // eslint-disable-next-line compat/compat
+        return new Promise(resolve => requestAnimationFrame(resolve))
+          .then(() => this._handleLoadingState(true))
+          .then(() => fetcher)
           .then(data => this.setChoices(data, value, label, replaceChoices))
           .catch(err => {
             if (!this.config.silent) {
@@ -572,6 +568,7 @@ class Choices {
           .then(() => this._handleLoadingState(false))
           .then(() => this);
       }
+
       // function returned something else than promise, let's check if it's an array of choices
       if (!Array.isArray(fetcher)) {
         throw new TypeError(
@@ -583,6 +580,12 @@ class Choices {
       return this.setChoices(fetcher, value, label, false);
     }
 
+    if (!Array.isArray(choicesArrayOrFetcher)) {
+      throw new TypeError(
+        `.setChoices must be called either with array of choices with a function resulting into Promise of array of choices`,
+      );
+    }
+
     this.containerOuter.removeLoadingState();
 
     const addGroupsAndChoices = groupOrChoice => {
diff --git a/src/scripts/choices.test.js b/src/scripts/choices.test.js
index 473dac7..cdf9522 100644
--- a/src/scripts/choices.test.js
+++ b/src/scripts/choices.test.js
@@ -1133,7 +1133,7 @@ describe('choices', () => {
           const fetcher = async inst => {
             expect(inst).to.eq(choice);
             fetcherCalled = true;
-            await new Promise(resolve => setTimeout(resolve, 1000));
+            await new Promise(resolve => setTimeout(resolve, 800));
 
             return [
               { label: 'l1', value: 'v1', customProperties: 'prop1' },
@@ -1142,15 +1142,10 @@ describe('choices', () => {
           };
           expect(choice._store.choices.length).to.equal(0);
           const promise = choice.setChoices(fetcher);
-          await new Promise(resolve =>
-            requestAnimationFrame(() => {
-              expect(handleLoadingStateSpy.callCount).to.equal(1);
-              resolve();
-            }),
-          );
           expect(fetcherCalled).to.be.true;
           const res = await promise;
           expect(res).to.equal(choice);
+          expect(handleLoadingStateSpy.callCount).to.equal(2);
           expect(choice._store.choices[1].value).to.equal('v2');
           expect(choice._store.choices[1].label).to.equal('l2');
           expect(choice._store.choices[1].customProperties).to.equal('prop2');