fix select regressions

docs/pages/resources/changelog.md

@@ -17,7 +17,8 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 - Added Norwegian translations for Bokmål and Nynorsk [#2268]
 - Added Ukrainian translation [#2270]
 - Added support for <kbd>Enter</kbd> to `<sl-split-panel>` to align with ARIA APG's [window splitter pattern](https://www.w3.org/WAI/ARIA/apg/patterns/windowsplitter/) [#2234]
-- Fixed a bug in `<sl-select>` where it was using the wrong tag name,. [#2287]
+- Fixed a bug in `<sl-select>` when setting the value property before the element connected. [#2255]
+- Fixed a bug in `<sl-select>` where it was using the wrong tag name. [#2287]
 - Fixed a bug in `<sl-carousel>` that caused the navigation icons to be reversed
 - Fixed a bug in `<sl-select>` that prevented label changes in `<sl-option>` from updating the controller [#1971]
 - Fixed a bug in `<sl-textarea>` that caused a console warning in Firefox when typing [#2107]

src/components/select/select.component.ts

@@ -1,6 +1,5 @@
 import { animateTo, stopAnimations } from '../../internal/animate.js';
 import { classMap } from 'lit/directives/class-map.js';
-import { defaultValue } from '../../internal/default-value.js';
 import { FormControlController } from '../../internal/form.js';
 import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry.js';
 import { HasSlotController } from '../../internal/slot.js';
@@ -102,21 +101,35 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   /** The name of the select, submitted as a name/value pair with form data. */
   @property() name = '';
 
+  private _value: string | string[] = '';
+
+  get value() {
+    return this._value;
+  }
+
   /**
    * The current value of the select, submitted as a name/value pair with form data. When `multiple` is enabled, the
    * value attribute will be a space-delimited list of values based on the options selected, and the value property will
    * be an array. **For this reason, values must not contain spaces.**
    */
-  @property({
-    converter: {
-      fromAttribute: (value: string) => value.split(' '),
-      toAttribute: (value: string[]) => value.join(' ')
+  @state()
+  set value(val: string | string[]) {
+    if (this.multiple) {
+      val = Array.isArray(val) ? val : val.split(' ');
+    } else {
+      val = Array.isArray(val) ? val.join(' ') : val;
     }
-  })
-  value: string | string[] = '';
+
+    if (this._value === val) {
+      return;
+    }
+
+    this.valueHasChanged = true;
+    this._value = val;
+  }
 
   /** The default value of the form control. Primarily used for resetting the form control. */
-  @defaultValue() defaultValue: string | string[] = '';
+  @property({ attribute: 'value' }) defaultValue: string | string[] = '';
 
   /** The select's size. */
   @property({ reflect: true }) size: 'small' | 'medium' | 'large' = 'medium';
@@ -451,6 +464,8 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   private handleClearClick(event: MouseEvent) {
     event.stopPropagation();
 
+    this.valueHasChanged = true;
+
     if (this.value !== '') {
       this.setSelectedOptions([]);
       this.displayInput.focus({ preventScroll: true });
@@ -522,6 +537,8 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   private handleTagRemove(event: SlRemoveEvent, option: SlOption) {
     event.stopPropagation();
 
+    this.valueHasChanged = true;
+
     if (!this.disabled) {
       this.toggleOptionSelection(option, false);
 
@@ -598,6 +615,9 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
     // Update selected options cache
     this.selectedOptions = options.filter(el => el.selected);
 
+    // Keep a reference to the previous `valueHasChanged`. Changes made here don't count has changing the value.
+    const cachedValueHasChanged = this.valueHasChanged;
+
     // Update the value and display label
     if (this.multiple) {
       this.value = this.selectedOptions.map(el => el.value);
@@ -613,12 +633,14 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
       this.value = selectedOption?.value ?? '';
       this.displayLabel = selectedOption?.getTextLabel?.() ?? '';
     }
+    this.valueHasChanged = cachedValueHasChanged;
 
     // Update validity
     this.updateComplete.then(() => {
       this.formControlController.updateValidity();
     });
   }
+
   protected get tags() {
     return this.selectedOptions.map((option, index) => {
       if (index < this.maxOptionsVisible || this.maxOptionsVisible <= 0) {
@@ -649,8 +671,29 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
     }
   }
 
-  @watch('value', { waitUntilFirstUpdate: true })
+  attributeChangedCallback(name: string, oldVal: string | null, newVal: string | null) {
+    super.attributeChangedCallback(name, oldVal, newVal);
+
+    /** This is a backwards compatibility call. In a new major version we should make a clean separation between "value" the attribute mapping to "defaultValue" property and "value" the property not reflecting. */
+    if (name === 'value') {
+      const cachedValueHasChanged = this.valueHasChanged;
+      this.value = this.defaultValue;
+
+      // Set it back to false since this isn't an interaction.
+      this.valueHasChanged = cachedValueHasChanged;
+    }
+  }
+
+  @watch(['defaultValue', 'value'], { waitUntilFirstUpdate: true })
   handleValueChange() {
+    if (!this.valueHasChanged) {
+      const cachedValueHasChanged = this.valueHasChanged;
+      this.value = this.defaultValue;
+
+      // Set it back to false since this isn't an interaction.
+      this.valueHasChanged = cachedValueHasChanged;
+    }
+
     const allOptions = this.getAllOptions();
     const value = Array.isArray(this.value) ? this.value : [this.value];
 

src/components/select/select.test.ts

@@ -740,6 +740,52 @@ describe('<sl-select>', () => {
         expect(new FormData(form).getAll('select')).to.have.members(['foo', 'bar', 'baz']);
       });
     });
+
+    /**
+     * @see {https://github.com/shoelace-style/shoelace/issues/2254}
+     */
+    it('Should account for if `value` changed before connecting', async () => {
+      const select = await fixture<SlSelect>(html`
+        <sl-select label="Search By" multiple clearable .value=${['foo', 'bar']}>
+          <sl-option value="foo">Foo</sl-option>
+          <sl-option value="bar">Bar</sl-option>
+        </sl-select>
+      `);
+
+      // just for safe measure.
+      await aTimeout(10);
+
+      expect(select.value).to.deep.equal(['foo', 'bar']);
+    });
+
+    /**
+     * @see {https://github.com/shoelace-style/shoelace/issues/2254}
+     */
+    it('Should still work if using the value attribute', async () => {
+      const select = await fixture<SlSelect>(html`
+        <sl-select label="Search By" multiple clearable value="foo bar">
+          <sl-option value="foo">Foo</sl-option>
+          <sl-option value="bar">Bar</sl-option>
+        </sl-select>
+      `);
+
+      // just for safe measure.
+      await aTimeout(10);
+
+      expect(select.value).to.deep.equal(['foo', 'bar']);
+
+      await clickOnElement(select);
+      await select.updateComplete;
+      await clickOnElement(select.querySelector("[value='foo']")!);
+
+      await select.updateComplete;
+      await aTimeout(10);
+      expect(select.value).to.deep.equal(['bar']);
+
+      select.setAttribute('value', 'foo bar');
+      await aTimeout(10);
+      expect(select.value).to.deep.equal(['foo', 'bar']);
+    });
   });
 
   runFormControlBaseTests('sl-select');