Fix mutation APIs setChoiceByValue/setChoices/setValue now thro…

…w an error the Choices instance was not initialized or multiple choices instances where initialized on the same element. Prevents bad internal states from triggering unexpected errors #1129
Choices-js · Aug 10, 2024 · fa9c341 · fa9c341
1 parent ac2a087
commit fa9c341
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * Fix Choices does not accept an element from an iframe [#1057](https://github.com/Choices-js/Choices/issues/1057)
 * Fix Choices was not disable in a `<fieldset disabled>` [#1132](https://github.com/Choices-js/Choices/issues/1132)
 * Fix `silent` option does not silence warnings about unknown options [#1119](https://github.com/Choices-js/Choices/issues/1119)
+* Fix mutation APIs `setChoiceByValue`/`setChoices`/`setValue` now throw an error the Choices instance was not initialized or multiple choices instances where initialized on the same element. Prevents bad internal states from triggering unexpected errors [#1129](https://github.com/Choices-js/Choices/issues/1129)
 
 ### Bug Fixes (from 11.0.0RC1)
 * Fix possible empty `aria-label` generation on remove item button

diff --git a/src/scripts/choices.ts b/src/scripts/choices.ts
@@ -98,6 +98,8 @@ class Choices implements ChoicesInterface {
 
   initialised: boolean;
 
+  initialisedOK?: boolean = undefined;
+
   config: Options;
 
   passedElement: WrappedInput | WrappedSelect;
@@ -341,6 +343,7 @@ class Choices implements ChoicesInterface {
       }
 
       this.initialised = true;
+      this.initialisedOK = false;
 
       return;
     }
@@ -352,7 +355,7 @@ class Choices implements ChoicesInterface {
   }
 
   init(): void {
-    if (this.initialised) {
+    if (this.initialised || this.initialisedOK !== undefined) {
       return;
     }
 
@@ -376,6 +379,7 @@ class Choices implements ChoicesInterface {
     }
 
     this.initialised = true;
+    this.initialisedOK = true;
 
     const { callbackOnInit } = this.config;
     // Run callback if it is a function
@@ -398,6 +402,7 @@ class Choices implements ChoicesInterface {
 
     this._templates = templates;
     this.initialised = false;
+    this.initialisedOK = undefined;
   }
 
   enable(): this {
@@ -574,7 +579,9 @@ class Choices implements ChoicesInterface {
   }
 
   setValue(items: string[] | InputChoice[]): this {
-    if (!this.initialised) {
+    if (!this.initialisedOK) {
+      this._warnChoicesInitFailed('setValue');
+
       return this;
     }
 
@@ -590,7 +597,12 @@ class Choices implements ChoicesInterface {
   }
 
   setChoiceByValue(value: string | string[]): this {
-    if (!this.initialised || this._isTextElement) {
+    if (!this.initialisedOK) {
+      this._warnChoicesInitFailed('setChoiceByValue');
+
+      return this;
+    }
+    if (this._isTextElement) {
       return this;
     }
     this._store.withDeferRendering(() => {
@@ -679,10 +691,10 @@ class Choices implements ChoicesInterface {
     label = 'label',
     replaceChoices = false,
   ): this | Promise<this> {
-    if (!this.initialised) {
-      throw new ReferenceError(
-        `setChoices was called on a non-initialized instance of Choices`,
-      );
+    if (!this.initialisedOK) {
+      this._warnChoicesInitFailed('setChoices');
+
+      return this;
     }
     if (!this._isSelectElement) {
       throw new TypeError(`setChoices can't be used with INPUT based Choices`);
@@ -2658,6 +2670,21 @@ class Choices implements ChoicesInterface {
 
     return null;
   }
+
+  _warnChoicesInitFailed(caller: string) {
+    if (this.config.silent) {
+      return;
+    }
+    if (!this.initialised) {
+      throw new TypeError(
+        `${caller} called on a non-initialised instance of Choices`,
+      );
+    } else if (!this.initialisedOK) {
+      throw new TypeError(
+        `${caller} called for an element which has multiple instances of Choices initialised on it`,
+      );
+    }
+  }
 }
 
 export default Choices;
diff --git a/test/scripts/choices.test.ts b/test/scripts/choices.test.ts
@@ -323,6 +323,7 @@ describe('choices', () => {
           addEventListenersSpy = spy(instance, '_addEventListeners');
 
           instance.initialised = false;
+          instance.initialisedOK = undefined;
           instance.init();
         });
 
@@ -378,6 +379,7 @@ describe('choices', () => {
       describe('not already initialised', () => {
         beforeEach(() => {
           instance.initialised = false;
+          instance.initialisedOK = undefined;
           instance.destroy();
         });
 
@@ -1225,10 +1227,22 @@ describe('choices', () => {
       describe('not initialised', () => {
         beforeEach(() => {
           instance.initialised = false;
+          instance.initialisedOK = undefined;
         });
 
         it('should throw', () => {
-          expect(() => instance.setChoices(null)).Throw(ReferenceError);
+          expect(() => instance.setChoices(null)).Throw(TypeError);
+        });
+      });
+
+      describe('initialised twice', () => {
+        it('throws', () => {
+          instance.initialised = true;
+          instance.initialisedOK = false;
+          expect(() => instance.setChoices(null)).to.throw(
+            TypeError,
+            'setChoices called for an element which has multiple instances of Choices initialised on it',
+          );
         });
       });
 
@@ -1310,17 +1324,24 @@ describe('choices', () => {
       });
 
       describe('not already initialised', () => {
-        beforeEach(() => {
+        it('throws', () => {
           instance.initialised = false;
-          output = instance.setValue(values);
-        });
-
-        it('returns this', () => {
-          expect(output).to.deep.equal(instance);
+          instance.initialisedOK = undefined;
+          expect(() => instance.setValue(values)).to.throw(
+            TypeError,
+            'setValue called on a non-initialised instance of Choices',
+          );
         });
+      });
 
-        it('returns early', () => {
-          expect(_addChoiceStub.called).to.equal(false);
+      describe('initialised twice', () => {
+        it('throws', () => {
+          instance.initialised = true;
+          instance.initialisedOK = false;
+          expect(() => instance.setValue(values)).to.throw(
+            TypeError,
+            'setValue called for an element which has multiple instances of Choices initialised on it',
+          );
         });
       });
 
@@ -1359,17 +1380,24 @@ describe('choices', () => {
       });
 
       describe('not already initialised', () => {
-        beforeEach(() => {
+        it('throws', () => {
           instance.initialised = false;
-          output = instance.setChoiceByValue([]);
-        });
-
-        it('returns this', () => {
-          expect(output).to.deep.equal(instance);
+          instance.initialisedOK = undefined;
+          expect(() => instance.setChoiceByValue([])).to.throw(
+            TypeError,
+            'setChoiceByValue called on a non-initialised instance of Choices',
+          );
         });
+      });
 
-        it('returns early', () => {
-          expect(findAndSelectChoiceByValueStub.called).to.equal(false);
+      describe('initialised twice', () => {
+        it('throws', () => {
+          instance.initialised = true;
+          instance.initialisedOK = false;
+          expect(() => instance.setChoiceByValue([])).to.throw(
+            TypeError,
+            'setChoiceByValue called for an element which has multiple instances of Choices initialised on it',
+          );
         });
       });