preact icon indicating copy to clipboard operation
preact copied to clipboard

`select` value update fails when set to a newly added `option` simultaneously

Open luisherranz opened this issue 9 months ago • 7 comments

  • [x] Check if updating to the latest Preact version resolves the issue

Describe the bug

Imagine you have a select element and its option elements in different Preact components, both subscribed to the same signal (or reading from the same context).

const Options = () =>
  items.value.map((item) => (
    <option key={item.id} value={item.id}>
      {item.id}
    </option>
  ));

const Select = ({ children }) => {
  const selectedItem = items.value.find((item) => item.selected).id;
  return <select value={selectedItem}>{children}</select>;
};

<Select>
  <Options />
</Select>

If you try to add a new option and immediately select it, Preact first updates the value attribute of the select element in the DOM before adding the new option, and therefore the change of the value attribute fails.

To Reproduce

  • Using signals: https://stackblitz.com/edit/vitejs-vite-cd6mlnbv
  • Using context: https://stackblitz.com/edit/vitejs-vite-4tf6mbza

Interestingly, the same code in React works fine: https://stackblitz.com/edit/vitejs-vite-m6klnuay

Steps to reproduce the behavior: just click on the button.

Expected behavior

After clicking the button, the select value should be the third, newly added option, but it's not. Actually, it is correct in the virtual DOM, but not in the DOM.

The same code works if you refactor it so that both elements are in a single element.

luisherranz avatar Mar 26 '25 15:03 luisherranz

There might be a missing case over here

JoviDeCroock avatar Mar 26 '25 15:03 JoviDeCroock

It's interesting that it doesn't fail when both elements are in the same component. Like, it seems to me that the order of DOM updates should be the same in both cases:

  1. The value of select is updated (and it fails because that option doesn't exist yet).
  2. The new option element is added.

Or the other way around (and then it would work fine).

But Preact should keep the same DOM update order whether the elements are in the same component or in different components.

luisherranz avatar Mar 26 '25 15:03 luisherranz

Preact does keep the same update order, it's just a bug 😅

JoviDeCroock avatar Mar 26 '25 15:03 JoviDeCroock

Okay so the issue is effectively that the diff happens in two passes due to memo not inheriting the forced update that goes along with context.

JoviDeCroock avatar Mar 26 '25 15:03 JoviDeCroock

Hey, thanks for the fix in https://github.com/preactjs/preact/pull/4738, @jovidecroock.

I've tested it in this StackBlitz, and it effectively solves the problem when reproduced with the context and memos, but it doesn't solve the problem when reproduced with the signals.

  • Context using 4738 PR: https://stackblitz.com/edit/vitejs-vite-xxgoai7e (solved)
  • Signals using 4738 PR: https://stackblitz.com/edit/vitejs-vite-6ssntjtq (not using memo, still broken)

When profiling, the context example has indeed changed from two commits to one, but the signals example is still doing two commits.

Could there be two different problems?

luisherranz avatar Mar 27 '25 06:03 luisherranz

Signals has its own shouldComponentUpdate override which is probably bailing as well 😅 I was looking into this but essentially this might need a different fix which might even come down to two-pass rendering

JoviDeCroock avatar Mar 27 '25 07:03 JoviDeCroock

I can confirm the Context/memo scenario is fixed by #4738, but the Signals variant still repros on 10.27.1.

Repro (no memo, Signals only)

  • StackBlitz (drop-in) create a signal items, render <select value={selectedId}><Options/></select>
  • In one tick push a new option into items and set selectedId to the new value.

My observation is that the DOM value on <select> is applied before the new <option> becomes part of the DOM, so the setter is effectively a no op in that commit. With Signals the update still seems to split into two commits, leading to the wrong final value.

And see Jovi's note above, Signals has its own shouldComponentUpdate like bailout. Looks like we need the same “single pass” guarantee for form controls or a post-commit value sync when selectedIndex === -1 at commit time.

NandanDevHub avatar Sep 12 '25 21:09 NandanDevHub