From 4f78930e9965f337c9d566aacc4bacee24e47c7f Mon Sep 17 00:00:00 2001
From: Konnor Rogers <konnor5456@gmail.com>
Date: Thu, 10 Oct 2024 13:40:15 -0400
Subject: [PATCH] add more resilient lazy loading to sl-select (#2204)

* add more resilient lazy loading to sl-select

* prettier

* add more resilient lazy loading to sl-select

* remove unnecessary assertions

* remove unnecessary assertions

* fix bad logic

* prettier

* add changelog entry

* prettier
---
 docs/pages/components/select.md           | 103 ++++++++++++++++++
 docs/pages/resources/changelog.md         |   1 +
 src/components/select/select.component.ts |  35 ++++---
 src/components/select/select.test.ts      | 122 ++++++++++++++++++++++
 4 files changed, 248 insertions(+), 13 deletions(-)

diff --git a/docs/pages/components/select.md b/docs/pages/components/select.md
index 04184b05ca..15ef3da3a2 100644
--- a/docs/pages/components/select.md
+++ b/docs/pages/components/select.md
@@ -502,6 +502,109 @@ Remember that custom tags are rendered in a shadow root. To style them, you can
 </script>
 ```
 
+### Lazy loading options
+
+Lazy loading options is very hard to get right. `<wa-select>` largely follows how a native `<select>` works.
+
+Here are the following conditions:
+
+- If a `<wa-select>` is created without any options, but is given a `value` attribute, its `value` will be `""`, and then when options are added, if any of the options have a value equal to the `<wa-select>` value, the value of the `<wa-select>` will equal that of the option.
+
+EX: `<wa-select value="foo">` will have a value of `""` until `<wa-option value="foo">Foo</wa-option>` connects, at which point its value will become `"foo"` when submitting.
+
+- If a `<wa-select multiple>` with an initial value has multiple values, but only some of the options are present, it will only respect the options that are present, and if a selected option is loaded in later, _AND_ the value of the select has not changed via user interaction or direct property assignment, it will add the selected option to the form value and to the `.value` of the select.
+
+This can be hard to conceptualize, so heres a fairly large example showing how lazy loaded options work with `<wa-select>` and `<wa-select multiple>` when given initial value attributes. Feel free to play around with it in a codepen.
+
+```html:preview
+<form id="lazy-options-example">
+  <div>
+    <sl-select name="select-1" value="foo" label="Single select (with existing options)">
+      <sl-option value="bar">Bar</sl-option>
+      <sl-option value="baz">Baz</sl-option>
+    </sl-select>
+    <br>
+    <sl-button type="button">Add "foo" option</sl-button>
+  </div>
+
+  <br>
+
+  <div>
+    <sl-select name="select-2" value="foo" label="Single select (with no existing options)">
+    </sl-select>
+    <br>
+    <sl-button type="button">Add "foo" option</sl-button>
+  </div>
+
+  <br>
+
+  <div>
+    <sl-select name="select-3" value="foo bar baz" multiple label="Multiple Select (with existing options)">
+      <sl-option value="bar">Bar</sl-option>
+      <sl-option value="baz">Baz</sl-option>
+    </sl-select>
+    <br>
+    <sl-button type="button">Add "foo" option</sl-button>
+  </div>
+
+  <br>
+
+  <div>
+    <sl-select name="select-4" value="foo" multiple label="Multiple Select (with no existing options)">
+    </sl-select>
+    <br>
+    <sl-button type="button">Add "foo" option</sl-button>
+  </div>
+
+  <br><br>
+
+  <div style="display: flex; gap: 16px;">
+    <sl-button type="reset">Reset</sl-button>
+    <sl-button type="submit" variant="brand">Show FormData</sl-button>
+  </div>
+
+  <br>
+
+  <pre hidden><code id="lazy-options-example-form-data"></code></pre>
+
+  <br>
+</form>
+
+<script type="module">
+  function addFooOption(e) {
+    const addFooButton = e.target.closest("sl-button[type='button']")
+    if (!addFooButton) {
+      return
+    }
+    const select = addFooButton.parentElement.querySelector("sl-select")
+    if (select.querySelector("sl-option[value='foo']")) {
+      // Foo already exists. no-op.
+      return
+    }
+    const option = document.createElement("sl-option")
+    option.setAttribute("value", "foo")
+    option.innerText = "Foo"
+    select.append(option)
+  }
+  function handleLazySubmit (event) {
+    event.preventDefault()
+    const formData = new FormData(event.target)
+    const codeElement = document.querySelector("#lazy-options-example-form-data")
+    const obj = {}
+    for (const key of formData.keys()) {
+      const val = formData.getAll(key).length > 1 ? formData.getAll(key) : formData.get(key)
+      obj[key] = val
+    }
+    codeElement.textContent = JSON.stringify(obj, null, 2)
+    const preElement = codeElement.parentElement
+    preElement.removeAttribute("hidden")
+  }
+  const container = document.querySelector("#lazy-options-example")
+  container.addEventListener("click", addFooOption)
+  container.addEventListener("submit", handleLazySubmit)
+</script>
+```
+
 :::warning
 Be sure you trust the content you are outputting! Passing unsanitized user input to `getTag()` can result in XSS vulnerabilities.
 :::
diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md
index 584b751d0d..785a1826a6 100644
--- a/docs/pages/resources/changelog.md
+++ b/docs/pages/resources/changelog.md
@@ -15,6 +15,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 ## Next
 
 - Added Finnish translations [#2211]
+- Fixed a bug with with `<sl-select>` not respecting its initial value. [#2204]
 - Fixed a bug with certain bundlers when using dynamic imports [#2210]
 - Fixed a bug in `<sl-textarea>` causing scroll jumping when using `resize="auto"` [#2182]
 - Fixed a bug in `<sl-relative-time>` where the title attribute would show with redundant info [#2184]
diff --git a/src/components/select/select.component.ts b/src/components/select/select.component.ts
index b5d464a23f..6dd5f21a09 100644
--- a/src/components/select/select.component.ts
+++ b/src/components/select/select.component.ts
@@ -97,6 +97,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   @state() displayLabel = '';
   @state() currentOption: SlOption;
   @state() selectedOptions: SlOption[] = [];
+  @state() private valueHasChanged: boolean = false;
 
   /** The name of the select, submitted as a name/value pair with form data. */
   @property() name = '';
@@ -216,6 +217,10 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   connectedCallback() {
     super.connectedCallback();
 
+    setTimeout(() => {
+      this.handleDefaultSlotChange();
+    });
+
     // Because this is a form control, it shouldn't be opened initially
     this.open = false;
   }
@@ -310,6 +315,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
 
       // If it is open, update the value based on the current selection and close it
       if (this.currentOption && !this.currentOption.disabled) {
+        this.valueHasChanged = true;
         if (this.multiple) {
           this.toggleOptionSelection(this.currentOption);
         } else {
@@ -470,6 +476,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
     const oldValue = this.value;
 
     if (option && !option.disabled) {
+      this.valueHasChanged = true;
       if (this.multiple) {
         this.toggleOptionSelection(option);
       } else {
@@ -495,20 +502,20 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   }
 
   private handleDefaultSlotChange() {
+    if (!customElements.get('wa-option')) {
+      customElements.whenDefined('wa-option').then(() => this.handleDefaultSlotChange());
+    }
+
     const allOptions = this.getAllOptions();
-    const value = Array.isArray(this.value) ? this.value : [this.value];
+    const val = this.valueHasChanged ? this.value : this.defaultValue;
+    const value = Array.isArray(val) ? val : [val];
     const values: string[] = [];
 
     // Check for duplicate values in menu items
-    if (customElements.get('sl-option')) {
-      allOptions.forEach(option => values.push(option.value));
+    allOptions.forEach(option => values.push(option.value));
 
-      // Select only the options that match the new value
-      this.setSelectedOptions(allOptions.filter(el => value.includes(el.value)));
-    } else {
-      // Rerun this handler when <sl-option> is registered
-      customElements.whenDefined('sl-option').then(() => this.handleDefaultSlotChange());
-    }
+    // Select only the options that match the new value
+    this.setSelectedOptions(allOptions.filter(el => value.includes(el.value)));
   }
 
   private handleTagRemove(event: SlRemoveEvent, option: SlOption) {
@@ -586,8 +593,9 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
   // This method must be called whenever the selection changes. It will update the selected options cache, the current
   // value, and the display value
   private selectionChanged() {
+    const options = this.getAllOptions();
     // Update selected options cache
-    this.selectedOptions = this.getAllOptions().filter(el => el.selected);
+    this.selectedOptions = options.filter(el => el.selected);
 
     // Update the value and display label
     if (this.multiple) {
@@ -600,8 +608,9 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
         this.displayLabel = this.localize.term('numOptionsSelected', this.selectedOptions.length);
       }
     } else {
-      this.value = this.selectedOptions[0]?.value ?? '';
-      this.displayLabel = this.selectedOptions[0]?.getTextLabel() ?? '';
+      const selectedOption = this.selectedOptions[0];
+      this.value = selectedOption?.value ?? '';
+      this.displayLabel = selectedOption?.getTextLabel?.() ?? '';
     }
 
     // Update validity
@@ -750,7 +759,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
     const hasLabel = this.label ? true : !!hasLabelSlot;
     const hasHelpText = this.helpText ? true : !!hasHelpTextSlot;
     const hasClearIcon = this.clearable && !this.disabled && this.value.length > 0;
-    const isPlaceholderVisible = this.placeholder && this.value.length === 0;
+    const isPlaceholderVisible = this.placeholder && this.value && this.value.length <= 0;
 
     return html`
       <div
diff --git a/src/components/select/select.test.ts b/src/components/select/select.test.ts
index ce8576df64..1de3eced5d 100644
--- a/src/components/select/select.test.ts
+++ b/src/components/select/select.test.ts
@@ -593,6 +593,128 @@ describe('<sl-select>', () => {
 
     expect(tag.hasAttribute('pill')).to.be.true;
   });
+  describe('With lazily loaded options', () => {
+    describe('With no existing options', () => {
+      it('Should wait to select the option when the option exists for single select', async () => {
+        const form = await fixture<HTMLFormElement>(
+          html`<form><sl-select name="select" value="option-1"></sl-select></form>`
+        );
+        const el = form.querySelector<SlSelect>('sl-select')!;
+
+        expect(el.value).to.equal('');
+        expect(new FormData(form).get('select')).equal('');
+
+        const option = document.createElement('sl-option');
+        option.value = 'option-1';
+        option.innerText = 'Option 1';
+        el.append(option);
+
+        await aTimeout(10);
+        await el.updateComplete;
+        expect(el.value).to.equal('option-1');
+        expect(new FormData(form).get('select')).equal('option-1');
+      });
+
+      it('Should wait to select the option when the option exists for multiple select', async () => {
+        const form = await fixture<HTMLFormElement>(
+          html`<form><sl-select name="select" value="option-1" multiple></sl-select></form>`
+        );
+
+        const el = form.querySelector<SlSelect>('sl-select')!;
+        expect(Array.isArray(el.value)).to.equal(true);
+        expect(el.value.length).to.equal(0);
+
+        const option = document.createElement('sl-option');
+        option.value = 'option-1';
+        option.innerText = 'Option 1';
+        el.append(option);
+
+        await aTimeout(10);
+        await el.updateComplete;
+        expect(el.value.length).to.equal(1);
+        expect(el.value).to.have.members(['option-1']);
+        expect(new FormData(form).getAll('select')).have.members(['option-1']);
+      });
+    });
+
+    describe('With existing options', () => {
+      it('Should not select the option if options already exist for single select', async () => {
+        const form = await fixture<HTMLFormElement>(
+          html` <form>
+            <sl-select name="select" value="foo">
+              <sl-option value="bar">Bar</sl-option>
+              <sl-option value="baz">Baz</sl-option>
+            </sl-select>
+          </form>`
+        );
+
+        const el = form.querySelector<SlSelect>('sl-select')!;
+        expect(el.value).to.equal('');
+        expect(new FormData(form).get('select')).to.equal('');
+
+        const option = document.createElement('sl-option');
+        option.value = 'foo';
+        option.innerText = 'Foo';
+        el.append(option);
+
+        await aTimeout(10);
+        await el.updateComplete;
+        expect(el.value).to.equal('foo');
+        expect(new FormData(form).get('select')).to.equal('foo');
+      });
+
+      it('Should not select the option if options already exists for multiple select', async () => {
+        const form = await fixture<HTMLFormElement>(
+          html` <form>
+            <sl-select name="select" value="foo" multiple>
+              <sl-option value="bar">Bar</sl-option>
+              <sl-option value="baz">Baz</sl-option>
+            </sl-select>
+          </form>`
+        );
+
+        const el = form.querySelector<SlSelect>('sl-select')!;
+        expect(el.value).to.be.an('array');
+        expect(el.value.length).to.equal(0);
+
+        const option = document.createElement('sl-option');
+        option.value = 'foo';
+        option.innerText = 'Foo';
+        el.append(option);
+
+        await aTimeout(10);
+        await el.updateComplete;
+        expect(el.value).to.have.members(['foo']);
+        expect(new FormData(form).getAll('select')).to.have.members(['foo']);
+      });
+
+      it('Should only select the existing options if options already exists for multiple select', async () => {
+        const form = await fixture<HTMLFormElement>(
+          html` <form>
+            <sl-select name="select" value="foo bar baz" multiple>
+              <sl-option value="bar">Bar</sl-option>
+              <sl-option value="baz">Baz</sl-option>
+            </sl-select>
+          </form>`
+        );
+
+        const el = form.querySelector<SlSelect>('sl-select')!;
+        expect(el.value).to.have.members(['bar', 'baz']);
+        expect(el.value.length).to.equal(2);
+        expect(new FormData(form).getAll('select')).to.have.members(['bar', 'baz']);
+
+        const option = document.createElement('sl-option');
+        option.value = 'foo';
+        option.innerText = 'Foo';
+        el.append(option);
+
+        await aTimeout(10);
+        await el.updateComplete;
+        expect(el.value).to.have.members(['foo', 'bar', 'baz']);
+        expect(new FormData(form).getAll('select')).to.have.members(['foo', 'bar', 'baz']);
+      });
+    });
+  });
 
   runFormControlBaseTests('sl-select');
 });