Skip to content

Commit 75b4308

Browse files
authored
fix(jsx/dom): apply select value after children are rendered (#4847)
* fix(jsx/dom): apply select value after children are rendered When options and value were set simultaneously on a select element, the value was applied before option children existed in the DOM, causing selectedIndex to fall back to 0. Defer select value assignment to after children rendering, similar to Preact's approach. This ensures options exist in the DOM before value matching, while other props like `multiple` are still applied before children. * fix(jsx/dom): fix select value sync edge cases For multiple selects with no matching option, selectedIndex was incorrectly forced to 0. Only reset selectedIndex for single selects, preserving the browser's default -1 for unmatched multiple selects. * perf(jsx/dom): use child.tag instead of el.nodeName for select check Avoid DOM property access in the hot render loop by checking the JSX node's tag property directly.
1 parent f47b559 commit 75b4308

File tree

2 files changed

+114
-9
lines changed

2 files changed

+114
-9
lines changed

src/jsx/dom/index.test.tsx

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ describe('DOM', () => {
904904
it('assign undefined', async () => {
905905
let setValue: (value: string | undefined) => void = () => {}
906906
const App = () => {
907-
const [value, _setValue] = useState<string | undefined>('a')
907+
const [value, _setValue] = useState<string | undefined>('b')
908908
setValue = _setValue
909909
return (
910910
<select value={value}>
@@ -922,6 +922,96 @@ describe('DOM', () => {
922922
await Promise.resolve()
923923
const select = root.querySelector('select') as HTMLSelectElement
924924
expect(select.value).toBe('a') // select the first option
925+
expect(select.selectedIndex).toBe(0)
926+
})
927+
928+
it('apply value after options are added', async () => {
929+
let setOptions: (options: string[]) => void = () => {}
930+
const App = () => {
931+
const [options, _setOptions] = useState<string[]>([])
932+
setOptions = _setOptions
933+
return (
934+
<select value='option2'>
935+
{options.map((opt) => (
936+
<option key={opt} value={opt}>
937+
{opt}
938+
</option>
939+
))}
940+
</select>
941+
)
942+
}
943+
render(<App />, root)
944+
setOptions(['option1', 'option2', 'option3'])
945+
await Promise.resolve()
946+
const select = root.querySelector('select') as HTMLSelectElement
947+
expect(select.value).toBe('option2')
948+
expect(select.selectedIndex).toBe(1)
949+
expect(select.options[1].selected).toBe(true)
950+
})
951+
952+
it('select the first option when undefined after options are added', async () => {
953+
let setOptions: (options: string[]) => void = () => {}
954+
const App = () => {
955+
const [options, _setOptions] = useState<string[]>([])
956+
setOptions = _setOptions
957+
return (
958+
<select value={undefined}>
959+
{options.map((opt) => (
960+
<option key={opt} value={opt}>
961+
{opt}
962+
</option>
963+
))}
964+
</select>
965+
)
966+
}
967+
render(<App />, root)
968+
setOptions(['option1', 'option2', 'option3'])
969+
await Promise.resolve()
970+
const select = root.querySelector('select') as HTMLSelectElement
971+
expect(select.value).toBe('option1')
972+
expect(select.selectedIndex).toBe(0)
973+
expect(select.options[0].selected).toBe(true)
974+
})
975+
976+
it('do not select the first option for invalid multiple value', () => {
977+
const App = () => {
978+
return (
979+
<select multiple value='z'>
980+
<option value='a'>A</option>
981+
<option value='b'>B</option>
982+
<option value='c'>C</option>
983+
</select>
984+
)
985+
}
986+
render(<App />, root)
987+
const select = root.querySelector('select') as HTMLSelectElement
988+
expect(select.value).toBe('')
989+
expect(select.selectedIndex).toBe(-1)
990+
expect([...select.options].every((option) => !option.selected)).toBe(true)
991+
})
992+
993+
it('keep invalid multiple value unselected after options are added', async () => {
994+
let setOptions: (options: string[]) => void = () => {}
995+
const App = () => {
996+
const [options, _setOptions] = useState<string[]>([])
997+
setOptions = _setOptions
998+
return (
999+
<select multiple value='z'>
1000+
{options.map((opt) => (
1001+
<option key={opt} value={opt}>
1002+
{opt}
1003+
</option>
1004+
))}
1005+
</select>
1006+
)
1007+
}
1008+
render(<App />, root)
1009+
setOptions(['a', 'b', 'c'])
1010+
await Promise.resolve()
1011+
const select = root.querySelector('select') as HTMLSelectElement
1012+
expect(select.value).toBe('')
1013+
expect(select.selectedIndex).toBe(-1)
1014+
expect([...select.options].every((option) => !option.selected)).toBe(true)
9251015
})
9261016
})
9271017

src/jsx/dom/render.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,20 @@ const toAttributeName = (element: SupportedElement, key: string): string =>
141141
? key.replace(/([A-Z])/g, '-$1').toLowerCase()
142142
: key
143143

144+
const normalizeFormValue = (value: Props['value']) =>
145+
value === null || value === undefined || value === false ? null : value
146+
147+
const applySelectValue = (select: HTMLSelectElement, props: Props): void => {
148+
if (!('value' in props)) {
149+
return
150+
}
151+
152+
select.value = normalizeFormValue(props['value'])
153+
if (!select.multiple && select.selectedIndex === -1) {
154+
select.selectedIndex = 0
155+
}
156+
}
157+
144158
const applyProps = (
145159
container: SupportedElement,
146160
attributes: Props,
@@ -188,18 +202,15 @@ const applyProps = (
188202
} else {
189203
if (key === 'value') {
190204
const nodeName = container.nodeName
191-
if (nodeName === 'INPUT' || nodeName === 'TEXTAREA' || nodeName === 'SELECT') {
192-
;(container as unknown as HTMLInputElement).value =
193-
value === null || value === undefined || value === false ? null : value
205+
if (nodeName === 'SELECT') {
206+
// Deferred to applySelectValue() after children are rendered
207+
continue
208+
} else if (nodeName === 'INPUT' || nodeName === 'TEXTAREA') {
209+
;(container as unknown as HTMLInputElement).value = normalizeFormValue(value)
194210

195211
if (nodeName === 'TEXTAREA') {
196212
container.textContent = value
197213
continue
198-
} else if (nodeName === 'SELECT') {
199-
if ((container as unknown as HTMLSelectElement).selectedIndex === -1) {
200-
;(container as unknown as HTMLSelectElement).selectedIndex = 0
201-
}
202-
continue
203214
}
204215
}
205216
} else if (
@@ -404,8 +415,12 @@ const applyNodeObject = (node: NodeObject, container: Container, isNew: boolean)
404415
el = child.e ||= child.n
405416
? (document.createElementNS(child.n, child.tag as string) as SVGElement | MathMLElement)
406417
: document.createElement(child.tag as string)
418+
407419
applyProps(el as HTMLElement, child.props, child.pP)
408420
applyNodeObject(child, el as HTMLElement, isNewLocal)
421+
if (child.tag === 'select') {
422+
applySelectValue(el as HTMLSelectElement, child.props)
423+
}
409424
}
410425
}
411426
if (child.tag === HONO_PORTAL_ELEMENT) {

0 commit comments

Comments
 (0)