From 9dfe8e2650101f813db4794c8804acde14dcad92 Mon Sep 17 00:00:00 2001 From: Bilal Vural <11067812+bilalvur@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:25:52 +0100 Subject: [PATCH 1/4] fix: restore controlled form controls after onChange React writes a controlled control's value and checked state back after its change handlers run. Redact only synced them on re-render, so a handler that rejected an edit by not calling setState left the typed text, checkbox toggle, radio selection or select choice in the DOM. The restore runs after the element's last Redact listener for the native input or change event, so onChange, onInput and their capture forms all read the edit, also with real user input, which runs microtasks between listeners. It is queued behind the handler's own updates (microtask, or act's queue), so it reads the props they render and keeps the caret of an accepted edit. Hydration records the props too, and a radio restores the rest of its group. Fixes #115. Co-Authored-By: Claude Opus 5.5 --- .changeset/restore-controlled-inputs.md | 5 + packages/redact/src/dom/dom.ts | 22 +- .../redact/src/dom/features/hydration/full.ts | 3 + packages/redact/src/dom/form-controls.ts | 51 ++++- scripts/actions-browser.config.mjs | 2 +- scripts/size-check.mjs | 16 +- tests/issue115-controlled-restore.browser.tsx | 27 +++ tests/issue115-controlled-restore.test.tsx | 216 ++++++++++++++++++ 8 files changed, 330 insertions(+), 12 deletions(-) create mode 100644 .changeset/restore-controlled-inputs.md create mode 100644 tests/issue115-controlled-restore.browser.tsx create mode 100644 tests/issue115-controlled-restore.test.tsx diff --git a/.changeset/restore-controlled-inputs.md b/.changeset/restore-controlled-inputs.md new file mode 100644 index 0000000..0cd25dc --- /dev/null +++ b/.changeset/restore-controlled-inputs.md @@ -0,0 +1,5 @@ +--- +'@tanstack/redact': patch +--- + +Restore a controlled input's, textarea's or select's rendered value, and a checkbox's or radio group's checked state, after its change handlers run, including after hydration. An edit, click or choice the app does not accept no longer stays in the DOM. ([#115](https://github.com/TanStack/redact/issues/115)) diff --git a/packages/redact/src/dom/dom.ts b/packages/redact/src/dom/dom.ts index c282da0..b2c8f57 100644 --- a/packages/redact/src/dom/dom.ts +++ b/packages/redact/src/dom/dom.ts @@ -2,6 +2,7 @@ import { createSyntheticEvent } from './synthetic-event'; import { UNITLESS_STYLE } from '../core/unitless-style'; import { devToolsMetadata } from '../core/devtools'; import { formActions } from './form-actions'; +import { scheduleControlledRestore } from './form-controls'; import { attributeName, attributeValue } from '../core/attributes'; import { isString } from "../core/functions"; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -104,6 +105,7 @@ function setStyle(el: HTMLElement, next: any, prev: any): void { } } } +let listenerCount = 0; function setEventHandler(el: Element, name: string, next: any): void { // Only function handlers are accepted. Reject strings/objects/etc — the // legacy `onclick="..."` HTML attribute (string handler) is a known XSS @@ -145,12 +147,30 @@ function setEventHandler(el: Element, name: string, next: any): void { devToolsMetadata.current = devToolsMetadata.hosts.get(el) || null; try { entry.c(event); } finally { devToolsMetadata.current = previous; } } else entry.c(event); - } finally { event.currentTarget = null; } + } finally { + event.currentTarget = null; + // Restore a controlled control after its last listener for this + // event, so every handler reads the edit. + if ((e.type === 'input' || e.type === 'change') && (el as any).__p) { + let last: any; + for (const key in handlers) { + const other = handlers[key]; + if (other?.e.includes(e.type) && (!last || other.o > last.o)) + last = other; + } + if (last === entry) + scheduleControlledRestore(el as HTMLInputElement); + } + } }) as ((e: Event) => void) & { c: Function; e: string[]; + o: number; }; entry.e = events; + // Dispatch order: capture listeners run first at the target, then + // bubble ones, each in the order they were added. + entry.o = ++listenerCount + (capture ? 0 : 2 ** 30); existing = handlers[name] = entry; for (const event of events) el.addEventListener(event, entry, capture); diff --git a/packages/redact/src/dom/features/hydration/full.ts b/packages/redact/src/dom/features/hydration/full.ts index e768033..f49c886 100644 --- a/packages/redact/src/dom/features/hydration/full.ts +++ b/packages/redact/src/dom/features/hydration/full.ts @@ -287,6 +287,9 @@ export function adoptHostDom(fiber: Fiber, parent: Fiber): boolean { // Form defaults precede event registration so input type owns event mapping. if (tag === 'input' || tag === 'textarea') queueHydrationFormState(candidate as Element, props, tag); + // Hydration skips the form syncs, which record props for controlled restores. + if (tag === 'input' || tag === 'textarea' || tag === 'select') + (candidate as any).__p = props; hydrateProps(fiber, candidate as Element, props, tag, isSvg); // React SSR adds transport fields for progressively enhanced Server Actions. // They have no corresponding client fibers and must not consume child slots. diff --git a/packages/redact/src/dom/form-controls.ts b/packages/redact/src/dom/form-controls.ts index d40d876..edf6d42 100644 --- a/packages/redact/src/dom/form-controls.ts +++ b/packages/redact/src/dom/form-controls.ts @@ -1,5 +1,7 @@ import { trackInputValue } from './dom'; +import { ReactSharedInternals } from '../react'; export function syncTextareaValue(el: HTMLTextAreaElement, props: any, mounting?: boolean): void { + (el as any).__p = props; if (mounting) { const children = props.children; const initial = props.value ?? props.defaultValue ?? (Array.isArray(children) ? children[0] : children) ?? ''; @@ -12,15 +14,14 @@ export function syncTextareaValue(el: HTMLTextAreaElement, props: any, mounting? return; } const value = props.value; - if (value != null && el.value !== '' + value) { - el.value = '' + value; - trackInputValue(el); - } + if (value != null) + writeInputValue(el, value); const fallback = props.defaultValue ?? value ?? ''; if (el.defaultValue !== '' + fallback) el.defaultValue = '' + fallback; } export function syncInputValue(el: HTMLInputElement, props: any, prev?: any): void { + (el as any).__p = props; if (!prev) { el.checked = el.defaultChecked = !!(props.checked ?? props.defaultChecked); } @@ -46,11 +47,7 @@ export function syncInputValue(el: HTMLInputElement, props: any, prev?: any): vo return; } if (value != null) { - // Number inputs keep equivalent user spelling, including trailing zeroes. - if (el.type === 'number' ? (value === 0 && el.value === '') || el.value != value : el.value !== '' + value) { - el.value = '' + value; - trackInputValue(el); - } + writeInputValue(el, value); const nextDefault = el.type === 'number' && el.value == value ? el.value : '' + value; if (el.defaultValue !== nextDefault) el.defaultValue = nextDefault; @@ -63,7 +60,43 @@ export function syncInputValue(el: HTMLInputElement, props: any, prev?: any): vo el.removeAttribute('value'); } } +function writeInputValue(el: HTMLInputElement | HTMLTextAreaElement, value: any): void { + // Number inputs keep equivalent user spelling, including trailing zeroes. + if (el.type === 'number' ? (value === 0 && el.value === '') || el.value != value : el.value !== '' + value) { + el.value = '' + value; + trackInputValue(el); + } +} +// Like React, write the rendered value and checked state back after onChange, +// so an edit or click the app did not accept never stays in the DOM. Updates +// from the handler flush in a microtask (or act's queue) queued before this +// one, so the restore reads the props they render. +export function scheduleControlledRestore(el: HTMLInputElement): void { + if (!(el as any).__p) + return; + const restore = () => { + // Clicking a radio unchecks the rest of its group. + for (const input of el.type === 'radio' && el.name ? (el.getRootNode() as ParentNode).querySelectorAll('input[type=radio]') : [el]) { + const props = (input as any).__p; + if (!props || input.name !== el.name) + continue; + if (input.tagName === 'SELECT') + syncSelectValue(input as any, props, props); + else { + if (props.value != null) + writeInputValue(input, props.value); + if (props.checked != null) + input.checked = !!props.checked; + } + } + }; + if (process.env.NODE_ENV !== 'production' && ReactSharedInternals.A) + ReactSharedInternals.A.push(restore); + else + queueMicrotask(restore); +} export function syncSelectValue(select: HTMLSelectElement, props: any, prev?: any): void { + (select as any).__p = props; const multiple = select.multiple; const value = props.value ?? ((!prev || !!prev.multiple !== multiple) ? props.defaultValue ?? (prev && []) : null); if (value == null) diff --git a/scripts/actions-browser.config.mjs b/scripts/actions-browser.config.mjs index 1bc6933..2f21d79 100644 --- a/scripts/actions-browser.config.mjs +++ b/scripts/actions-browser.config.mjs @@ -6,6 +6,6 @@ export default { cacheDir: resolve('node_modules/.vite/actions-browser'), test: { ...config.test, - include: ['actions.test.tsx', 'transition-retention.test.tsx', 'activity-transition.test.tsx', 'select-defaults.test.tsx', 'event-parity.test.tsx', 'lazy-wrapped-components.test.tsx', 'thenable-children.test.tsx', 'root-suspension.test.tsx', 'find-dom-node.test.tsx', 'document-shell.test.tsx'], + include: ['actions.test.tsx', 'transition-retention.test.tsx', 'activity-transition.test.tsx', 'select-defaults.test.tsx', 'event-parity.test.tsx', 'lazy-wrapped-components.test.tsx', 'thenable-children.test.tsx', 'root-suspension.test.tsx', 'find-dom-node.test.tsx', 'document-shell.test.tsx', 'issue115-controlled-restore.browser.tsx'], }, } diff --git a/scripts/size-check.mjs b/scripts/size-check.mjs index fc69f74..681e6ff 100644 --- a/scripts/size-check.mjs +++ b/scripts/size-check.mjs @@ -63,11 +63,25 @@ const STORE_HYDRATION_SIZE_ALLOWANCE = { 'redact/dom-client (hydration=stub)': [3, 0], } +// Controlled input restore cost (#115) on main 37c051c, [source, built]. +// Pending maintainer approval. Only entries exceeding existing limits receive +// their measured increment. +const CONTROLLED_RESTORE_SIZE_ALLOWANCE = { + 'client total (default Vite)': [235, 259], + 'redact/dom-client (activity=stub)': [242, 224], + 'redact/dom-client (fragmentRefs=stub)': [241, 256], + 'redact/dom-client (viewTransitions=stub)': [273, 268], + 'redact/dom-client (context=stub)': [224, 0], + 'redact/dom-client (forwardRef=stub)': [229, 0], + 'redact/dom-client (hydration=stub)': [240, 237], + 'redact/dom-client (nano)': [200, 207], +} + let failed = false for (const tree of ['src', 'dist']) { for (const row of await measureSizes(tree)) { const baseBudget = BUDGETS[row.name] - const budget = baseBudget === undefined ? undefined : baseBudget + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + const budget = baseBudget === undefined ? undefined : baseBudget + (CONTROLLED_RESTORE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) const ok = budget !== undefined && row.gz <= budget failed ||= !ok console.log(`${ok ? 'PASS' : 'FAIL'} ${tree} ${row.name}: ${row.gz} B gzip, budget ${budget ?? 'missing'}`) diff --git a/tests/issue115-controlled-restore.browser.tsx b/tests/issue115-controlled-restore.browser.tsx new file mode 100644 index 0000000..c488f7b --- /dev/null +++ b/tests/issue115-controlled-restore.browser.tsx @@ -0,0 +1,27 @@ +import { expect, it } from 'vitest' +import { userEvent } from '@vitest/browser/context' +import { createElement as h } from 'react' +import { createRoot } from 'react-dom/client' +import { flushSync } from 'react-dom' + +// Real typing runs microtasks between listeners, which dispatchEvent never does. +it('lets every listener on a controlled input read the typed edit', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const seen: string[] = [] + flushSync(() => root.render(h('input', { + value: '', + onChange: (event: any) => seen.push('change ' + event.target.value), + onInput: (event: any) => seen.push('input ' + event.target.value), + }))) + const field = container.querySelector('input')! + try { + await userEvent.type(field, 'a') + expect(seen.sort()).toEqual(['change a', 'input a']) + expect(field.value).toBe('') + } finally { + flushSync(() => root.unmount()) + container.remove() + } +}) diff --git a/tests/issue115-controlled-restore.test.tsx b/tests/issue115-controlled-restore.test.tsx new file mode 100644 index 0000000..dd36749 --- /dev/null +++ b/tests/issue115-controlled-restore.test.tsx @@ -0,0 +1,216 @@ +import { afterEach, expect, it } from 'vitest' +import { act, createElement as h, useState } from 'react' +import { createRoot, hydrateRoot } from 'react-dom/client' +import { flushSync } from 'react-dom' +import { renderToString } from 'react-dom/server' + +const cleanups: Array<() => void> = [] +afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup() }) + +function mount(element: any): HTMLElement { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + flushSync(() => root.render(element)) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + return container +} + +async function hydrate(element: any): Promise { + const container = document.createElement('div') + container.innerHTML = renderToString(element) + document.body.appendChild(container) + const root = hydrateRoot(container, element) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + await settle() + return container +} + +// Updates from an event handler flush in a microtask, as do restores. +const settle = () => new Promise(resolve => setTimeout(resolve)) + +const type = async (field: HTMLInputElement | HTMLTextAreaElement, value: string) => { + field.value = value + field.dispatchEvent(new Event('input', { bubbles: true })) + await settle() +} + +const click = async (input: HTMLInputElement) => { + input.click() + await settle() +} + +function Digits({ tag = 'input', seen = [] }: { tag?: string, seen?: string[] }) { + const [value, setValue] = useState('') + return h(tag, { + value, + onChange: (event: any) => { + seen.push(event.target.value) + if (/^\d*$/.test(event.target.value)) setValue(event.target.value) + }, + }) +} + +it.each(['input', 'textarea'])('restores a controlled %s value that onChange rejected', async tag => { + const seen: string[] = [] + const field = mount(h(Digits, { tag, seen })).firstChild as HTMLInputElement + await type(field, '1') + await type(field, '1a') + expect(field.value).toBe('1') + await type(field, 'abc') + expect(field.value).toBe('1') + expect(seen).toEqual(['1', '1a', 'abc']) + // The restored value is the renderer's, so blur reports no change. + field.dispatchEvent(new Event('change', { bubbles: true })) + await settle() + expect(seen).toEqual(['1', '1a', 'abc']) +}) + +it('restores after updates that flushSync renders', async () => { + const field = mount(h(Digits)).firstChild as HTMLInputElement + for (const value of ['1', '1a']) { + field.value = value + flushSync(() => field.dispatchEvent(new Event('input', { bubbles: true }))) + } + await settle() + expect(field.value).toBe('1') +}) + +function Text() { + const [value, setValue] = useState('ac') + return h('input', { value, onChange: (event: any) => setValue(event.target.value) }) +} + +it('keeps the caret of an accepted edit', async () => { + const field = mount(h(Text)).firstChild as HTMLInputElement + field.value = 'abc' + field.setSelectionRange(2, 2) + field.dispatchEvent(new Event('input', { bubbles: true })) + await settle() + expect(field.value).toBe('abc') + expect(field.selectionStart).toBe(2) +}) + +it('lets onChangeCapture and onChange both read the typed value', async () => { + const seen: string[] = [] + const field = mount(h('input', { + value: '', + onChangeCapture: (event: any) => seen.push('capture ' + event.target.value), + onChange: (event: any) => seen.push('bubble ' + event.target.value), + })).firstChild as HTMLInputElement + await type(field, 'a') + expect(seen).toEqual(['capture a', 'bubble a']) + expect(field.value).toBe('') +}) + +it('restores a value rejected by onChangeCapture alone', async () => { + const field = mount(h('input', { value: 'x', onChangeCapture: () => {} })).firstChild as HTMLInputElement + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('keeps an equivalent spelling of a controlled number', async () => { + const field = mount(h('input', { type: 'number', value: 1, onChange: () => {} })).firstChild as HTMLInputElement + await type(field, '1.0') + expect(field.value).toBe('1.0') + await type(field, '2') + expect(field.value).toBe('1') +}) + +it('leaves an uncontrolled input alone', async () => { + const field = mount(h('input', { defaultValue: '', onChange: () => {} })).firstChild as HTMLInputElement + await type(field, 'a') + expect(field.value).toBe('a') +}) + +it.each([false, true])('restores checked=%s on a checkbox when onChange ignores the click', async checked => { + const seen: boolean[] = [] + const box = mount(h('input', { + type: 'checkbox', + checked, + onChange: (event: any) => seen.push(event.target.checked), + })).firstChild as HTMLInputElement + await click(box) + expect(seen).toEqual([!checked]) + expect(box.checked).toBe(checked) +}) + +it('restores the checked radio of a group when onChange ignores the click', async () => { + const container = mount(h('div', null, + h('input', { type: 'radio', name: 'size', value: 's', checked: true, onChange: () => {} }), + h('input', { type: 'radio', name: 'size', value: 'm', checked: false, onChange: () => {} }), + )) + const [small, medium] = [...container.querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(medium) + expect(small.checked).toBe(true) + expect(medium.checked).toBe(false) +}) + +it('follows a radio group that onChange accepts', async () => { + function Sizes() { + const [size, setSize] = useState('s') + return h('div', null, ['s', 'm'].map(value => h('input', { + key: value, type: 'radio', name: 'size', value, + checked: size === value, + onChange: (event: any) => setSize(event.target.value), + }))) + } + const [small, medium] = [...mount(h(Sizes)).querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(medium) + expect(small.checked).toBe(false) + expect(medium.checked).toBe(true) +}) + +it('restores after updates that act renders, keeping the caret', async () => { + const digits = mount(h(Digits)).firstChild as HTMLInputElement + for (const value of ['1', '1a']) { + await act(async () => { + digits.value = value + digits.dispatchEvent(new Event('input', { bubbles: true })) + }) + } + expect(digits.value).toBe('1') + const text = mount(h(Text)).firstChild as HTMLInputElement + await act(async () => { + text.value = 'abc' + text.setSelectionRange(2, 2) + text.dispatchEvent(new Event('input', { bubbles: true })) + }) + expect(text.selectionStart).toBe(2) +}) + +it.each(['input', 'textarea'])('restores a hydrated %s', async tag => { + const field = (await hydrate(h(Digits, { tag }))).firstChild as HTMLInputElement + await type(field, 'a') + expect(field.value).toBe('') + await type(field, '1') + expect(field.value).toBe('1') +}) + +it('restores a hydrated radio group', async () => { + const container = await hydrate(h('div', null, + h('input', { type: 'radio', name: 'size', value: 's', checked: true, onChange: () => {} }), + h('input', { type: 'radio', name: 'size', value: 'm', checked: false, onChange: () => {} }), + )) + const [small, medium] = [...container.querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(medium) + expect(small.checked).toBe(true) + expect(medium.checked).toBe(false) +}) + +it('restores a value rejected by onInput alone', async () => { + const field = mount(h('input', { value: 'x', onInput: () => {} })).firstChild as HTMLInputElement + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('restores a controlled select when onChange ignores the choice', async () => { + const select = mount(h('select', { value: 'a', onChange: () => {} }, + h('option', { value: 'a' }, 'A'), + h('option', { value: 'b' }, 'B'), + )).firstChild as HTMLSelectElement + select.value = 'b' + select.dispatchEvent(new Event('change', { bubbles: true })) + await settle() + expect(select.value).toBe('a') +}) From f01872bb88288c31111889e5112f2ea8507a0988 Mon Sep 17 00:00:00 2001 From: Bilal Vural <11067812+bilalvur@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:32:02 +0100 Subject: [PATCH 2/4] fix: restore a checkbox, radio or select after change, not input These controls fire input before change. With onInput and onChange both set, the restore ran after the input listener, and since real clicks run microtasks between events, onChange read the restored state instead of the click. A change listener now owns the restore for them; an onInput alone still restores after input. Refs #115. Co-Authored-By: Claude Opus 5.5 --- packages/redact/src/dom/dom.ts | 7 ++++-- scripts/size-check.mjs | 18 ++++++++------- tests/issue115-controlled-restore.browser.tsx | 22 +++++++++++++++++++ 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/redact/src/dom/dom.ts b/packages/redact/src/dom/dom.ts index b2c8f57..5640a91 100644 --- a/packages/redact/src/dom/dom.ts +++ b/packages/redact/src/dom/dom.ts @@ -152,13 +152,16 @@ function setEventHandler(el: Element, name: string, next: any): void { // Restore a controlled control after its last listener for this // event, so every handler reads the edit. if ((e.type === 'input' || e.type === 'change') && (el as any).__p) { - let last: any; + let last: any, change = false; for (const key in handlers) { const other = handlers[key]; if (other?.e.includes(e.type) && (!last || other.o > last.o)) last = other; + // A checkbox, radio or select fires input before change, so + // its change listener restores after both. + change ||= e.type === 'input' && other?.e[0] === 'change'; } - if (last === entry) + if (last === entry && !change) scheduleControlledRestore(el as HTMLInputElement); } } diff --git a/scripts/size-check.mjs b/scripts/size-check.mjs index 681e6ff..26db2a9 100644 --- a/scripts/size-check.mjs +++ b/scripts/size-check.mjs @@ -67,14 +67,16 @@ const STORE_HYDRATION_SIZE_ALLOWANCE = { // Pending maintainer approval. Only entries exceeding existing limits receive // their measured increment. const CONTROLLED_RESTORE_SIZE_ALLOWANCE = { - 'client total (default Vite)': [235, 259], - 'redact/dom-client (activity=stub)': [242, 224], - 'redact/dom-client (fragmentRefs=stub)': [241, 256], - 'redact/dom-client (viewTransitions=stub)': [273, 268], - 'redact/dom-client (context=stub)': [224, 0], - 'redact/dom-client (forwardRef=stub)': [229, 0], - 'redact/dom-client (hydration=stub)': [240, 237], - 'redact/dom-client (nano)': [200, 207], + 'client total (default Vite)': [260, 279], + 'redact/dom-client (activity=stub)': [257, 239], + 'redact/dom-client (fragmentRefs=stub)': [251, 268], + 'redact/dom-client (viewTransitions=stub)': [294, 292], + 'redact/dom-client (portal=stub)': [232, 0], + 'redact/dom-client (context=stub)': [240, 0], + 'redact/dom-client (memo=stub)': [258, 0], + 'redact/dom-client (forwardRef=stub)': [245, 253], + 'redact/dom-client (hydration=stub)': [253, 250], + 'redact/dom-client (nano)': [221, 223], } let failed = false diff --git a/tests/issue115-controlled-restore.browser.tsx b/tests/issue115-controlled-restore.browser.tsx index c488f7b..9cb6354 100644 --- a/tests/issue115-controlled-restore.browser.tsx +++ b/tests/issue115-controlled-restore.browser.tsx @@ -25,3 +25,25 @@ it('lets every listener on a controlled input read the typed edit', async () => container.remove() } }) + +it('lets onChange on a checkbox with onInput read the click', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const seen: boolean[] = [] + flushSync(() => root.render(h('input', { + type: 'checkbox', + checked: false, + onInput: () => {}, + onChange: (event: any) => seen.push(event.target.checked), + }))) + const box = container.querySelector('input')! + try { + await userEvent.click(box) + expect(seen).toEqual([true]) + expect(box.checked).toBe(false) + } finally { + flushSync(() => root.unmount()) + container.remove() + } +}) From 9fa185597dd82e99b628a9eff2ce092be8591757 Mon Sep 17 00:00:00 2001 From: Bilal Vural <11067812+bilalvur@users.noreply.github.com> Date: Fri, 30 Oct 2026 19:09:49 +0100 Subject: [PATCH 3/4] fix: restore controlled form controls after the whole dispatch The restore ran in a microtask after the control's last Redact listener, and real input runs microtasks between listeners. So an ancestor's onInput or a later native listener read the restored value, and an ancestor that accepted the edit lost it. A capture listener that stopped propagation also skipped the restore: the target's bubble listener, which owned it, never ran. The control's listeners now mark the native event, and listeners on the root and portal containers restore once it reaches them, as React's root listener does. A stopped or non-bubbling event restores at once, since it never reaches a container. Listening on the container rather than the document keeps the restore when a native listener on the container stops propagation, and covers shadow roots. A checkbox, radio or select with a change listener still restores after change; with only onInput it restores after input. Refs #115. Co-Authored-By: Claude Opus 5.5 --- packages/redact/src/dom/dom.ts | 23 +----- .../redact/src/dom/features/portal/full.ts | 4 +- packages/redact/src/dom/form-controls.ts | 22 +++++- packages/redact/src/dom/root-internal.ts | 2 + scripts/size-check.mjs | 22 +++--- tests/issue115-controlled-restore.browser.tsx | 38 +++++++++- tests/issue115-controlled-restore.test.tsx | 75 ++++++++++++++++++- 7 files changed, 151 insertions(+), 35 deletions(-) diff --git a/packages/redact/src/dom/dom.ts b/packages/redact/src/dom/dom.ts index 5640a91..821303c 100644 --- a/packages/redact/src/dom/dom.ts +++ b/packages/redact/src/dom/dom.ts @@ -2,7 +2,7 @@ import { createSyntheticEvent } from './synthetic-event'; import { UNITLESS_STYLE } from '../core/unitless-style'; import { devToolsMetadata } from '../core/devtools'; import { formActions } from './form-actions'; -import { scheduleControlledRestore } from './form-controls'; +import { queueRestore } from './form-controls'; import { attributeName, attributeValue } from '../core/attributes'; import { isString } from "../core/functions"; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -105,7 +105,6 @@ function setStyle(el: HTMLElement, next: any, prev: any): void { } } } -let listenerCount = 0; function setEventHandler(el: Element, name: string, next: any): void { // Only function handlers are accepted. Reject strings/objects/etc — the // legacy `onclick="..."` HTML attribute (string handler) is a known XSS @@ -149,31 +148,13 @@ function setEventHandler(el: Element, name: string, next: any): void { } else entry.c(event); } finally { event.currentTarget = null; - // Restore a controlled control after its last listener for this - // event, so every handler reads the edit. - if ((e.type === 'input' || e.type === 'change') && (el as any).__p) { - let last: any, change = false; - for (const key in handlers) { - const other = handlers[key]; - if (other?.e.includes(e.type) && (!last || other.o > last.o)) - last = other; - // A checkbox, radio or select fires input before change, so - // its change listener restores after both. - change ||= e.type === 'input' && other?.e[0] === 'change'; - } - if (last === entry && !change) - scheduleControlledRestore(el as HTMLInputElement); - } + queueRestore(e, el); } }) as ((e: Event) => void) & { c: Function; e: string[]; - o: number; }; entry.e = events; - // Dispatch order: capture listeners run first at the target, then - // bubble ones, each in the order they were added. - entry.o = ++listenerCount + (capture ? 0 : 2 ** 30); existing = handlers[name] = entry; for (const event of events) el.addEventListener(event, entry, capture); diff --git a/packages/redact/src/dom/features/portal/full.ts b/packages/redact/src/dom/features/portal/full.ts index 0ec1b6d..a203561 100644 --- a/packages/redact/src/dom/features/portal/full.ts +++ b/packages/redact/src/dom/features/portal/full.ts @@ -1,11 +1,13 @@ import { FiberTag, type Fiber, type ReactNode } from '../../../core'; import { REACT_PORTAL_TYPE } from '../../../react'; +import { listenForRestores } from '../../form-controls'; import { installRenderer, registerTypeMatcher, registerElementMarker, reconcileRaw, childrenToArray, } from '../../reconcile'; function renderPortal(fiber: Fiber): void { - const { children } = fiber.v as { + const { children, container } = fiber.v as { children: ReactNode; container: Element; }; + listenForRestores(container); reconcileRaw(fiber, children); fiber.m = fiber.v; } diff --git a/packages/redact/src/dom/form-controls.ts b/packages/redact/src/dom/form-controls.ts index edf6d42..c4a9150 100644 --- a/packages/redact/src/dom/form-controls.ts +++ b/packages/redact/src/dom/form-controls.ts @@ -67,11 +67,31 @@ function writeInputValue(el: HTMLInputElement | HTMLTextAreaElement, value: any) trackInputValue(el); } } +// Like React, restore from the root and portal containers, after every listener +// on the control and its ancestors has read the edit. A checkbox, radio or +// select fires input before change, so with a change listener it restores +// after change. +export function queueRestore(e: any, el: any): void { + if (el.__p && (e.type === 'change' || e.type === 'input' && !(/^(checkbox|radio|select|file)/.test(el.type) && Object.values(el.__h).some((other: any) => other?.e.includes('change'))))) + e.__r = el; + // A stopped or non-bubbling event never reaches the container. + if (e.cancelBubble || !e.bubbles) + restorePending(e); +} +export function listenForRestores(container: Node): void { + container.addEventListener('input', restorePending); + container.addEventListener('change', restorePending); +} +function restorePending(e: any): void { + if (e.__r) + scheduleControlledRestore(e.__r); + e.__r = null; +} // Like React, write the rendered value and checked state back after onChange, // so an edit or click the app did not accept never stays in the DOM. Updates // from the handler flush in a microtask (or act's queue) queued before this // one, so the restore reads the props they render. -export function scheduleControlledRestore(el: HTMLInputElement): void { +function scheduleControlledRestore(el: HTMLInputElement): void { if (!(el as any).__p) return; const restore = () => { diff --git a/packages/redact/src/dom/root-internal.ts b/packages/redact/src/dom/root-internal.ts index 59a2a3f..d01699d 100644 --- a/packages/redact/src/dom/root-internal.ts +++ b/packages/redact/src/dom/root-internal.ts @@ -1,3 +1,4 @@ +import { listenForRestores } from './form-controls'; import { FiberTag, createFiber, type Fiber, type FiberRoot, type RecoverableErrorHandler } from '../core'; export interface RootOptionsInternal { identifierStrategy?: 'react'; @@ -19,6 +20,7 @@ export function createFiberRoot(container: Element | Document | DocumentFragment if (options.identifierStrategy === 'react') root.ri = true; attachRootFiber(root, container); + listenForRestores(container); return root; } export function attachRootFiber(root: FiberRoot, container: Element | Document | DocumentFragment): void { diff --git a/scripts/size-check.mjs b/scripts/size-check.mjs index 26db2a9..82a2431 100644 --- a/scripts/size-check.mjs +++ b/scripts/size-check.mjs @@ -67,16 +67,18 @@ const STORE_HYDRATION_SIZE_ALLOWANCE = { // Pending maintainer approval. Only entries exceeding existing limits receive // their measured increment. const CONTROLLED_RESTORE_SIZE_ALLOWANCE = { - 'client total (default Vite)': [260, 279], - 'redact/dom-client (activity=stub)': [257, 239], - 'redact/dom-client (fragmentRefs=stub)': [251, 268], - 'redact/dom-client (viewTransitions=stub)': [294, 292], - 'redact/dom-client (portal=stub)': [232, 0], - 'redact/dom-client (context=stub)': [240, 0], - 'redact/dom-client (memo=stub)': [258, 0], - 'redact/dom-client (forwardRef=stub)': [245, 253], - 'redact/dom-client (hydration=stub)': [253, 250], - 'redact/dom-client (nano)': [221, 223], + 'redact/dom-client': [253, 278], + 'client total': [264, 259], + 'client total (default Vite)': [286, 309], + 'redact/dom-client (activity=stub)': [270, 267], + 'redact/dom-client (fragmentRefs=stub)': [310, 323], + 'redact/dom-client (viewTransitions=stub)': [330, 334], + 'redact/dom-client (portal=stub)': [240, 261], + 'redact/dom-client (context=stub)': [252, 280], + 'redact/dom-client (memo=stub)': [277, 286], + 'redact/dom-client (forwardRef=stub)': [261, 268], + 'redact/dom-client (hydration=stub)': [296, 286], + 'redact/dom-client (nano)': [261, 262], } let failed = false diff --git a/tests/issue115-controlled-restore.browser.tsx b/tests/issue115-controlled-restore.browser.tsx index 9cb6354..b668655 100644 --- a/tests/issue115-controlled-restore.browser.tsx +++ b/tests/issue115-controlled-restore.browser.tsx @@ -1,6 +1,6 @@ import { expect, it } from 'vitest' import { userEvent } from '@vitest/browser/context' -import { createElement as h } from 'react' +import { createElement as h, useState } from 'react' import { createRoot } from 'react-dom/client' import { flushSync } from 'react-dom' @@ -47,3 +47,39 @@ it('lets onChange on a checkbox with onInput read the click', async () => { container.remove() } }) + +function render(element: any): [HTMLElement, () => void] { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + flushSync(() => root.render(element)) + return [container, () => { flushSync(() => root.unmount()); container.remove() }] +} + +// React restores after its root listener, which runs after the control's own. +it('lets a native listener added after Redact\'s read the typed edit', async () => { + const seen: string[] = [] + const [container, cleanup] = render(h('input', { value: '', onChange: () => {} })) + const field = container.querySelector('input')! + field.addEventListener('input', () => seen.push(field.value)) + try { + await userEvent.type(field, 'a') + expect(seen).toEqual(['a']) + expect(field.value).toBe('') + } finally { cleanup() } +}) + +it('keeps an edit that an ancestor\'s onInput accepts', async () => { + function Form() { + const [value, setValue] = useState('') + return h('div', { onInput: (event: any) => setValue(event.target.value) }, + h('input', { value, onChange: () => {} })) + } + const [container, cleanup] = render(h(Form)) + const field = container.querySelector('input')! + try { + await userEvent.type(field, 'ab') + expect(field.value).toBe('ab') + } finally { cleanup() } +}) + diff --git a/tests/issue115-controlled-restore.test.tsx b/tests/issue115-controlled-restore.test.tsx index dd36749..9e17ff6 100644 --- a/tests/issue115-controlled-restore.test.tsx +++ b/tests/issue115-controlled-restore.test.tsx @@ -1,7 +1,7 @@ import { afterEach, expect, it } from 'vitest' import { act, createElement as h, useState } from 'react' import { createRoot, hydrateRoot } from 'react-dom/client' -import { flushSync } from 'react-dom' +import { createPortal, flushSync } from 'react-dom' import { renderToString } from 'react-dom/server' const cleanups: Array<() => void> = [] @@ -214,3 +214,76 @@ it('restores a controlled select when onChange ignores the choice', async () => await settle() expect(select.value).toBe('a') }) + +it('restores when onChange stops propagation', async () => { + const field = mount(h('input', { value: 'x', onChange: (event: any) => event.stopPropagation() })).firstChild as HTMLInputElement + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('restores when onChangeCapture stops propagation before onChange', async () => { + const seen: string[] = [] + const field = mount(h('input', { + value: 'x', + onChangeCapture: (event: any) => event.stopPropagation(), + onChange: () => seen.push('bubble'), + })).firstChild as HTMLInputElement + await type(field, 'xy') + expect(seen).toEqual([]) + expect(field.value).toBe('x') +}) + +it('restores when an ancestor\'s onInput stops propagation', async () => { + const container = mount(h('div', { onInput: (event: any) => event.stopPropagation() }, + h('input', { value: 'x', onChange: () => {} }), + )) + const field = container.querySelector('input')! + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('restores when a native listener on the container stops propagation', async () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + container.addEventListener('input', event => event.stopPropagation()) + flushSync(() => root.render(h('input', { value: 'x', onChange: () => {} }))) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + const field = container.firstChild as HTMLInputElement + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('restores after an input event that does not bubble', async () => { + const field = mount(h('input', { value: 'x', onChange: () => {} })).firstChild as HTMLInputElement + field.value = 'xy' + field.dispatchEvent(new Event('input')) + await settle() + expect(field.value).toBe('x') +}) + +it('restores a checkbox rendered in a shadow root', async () => { + const host = document.createElement('div') + document.body.appendChild(host) + const container = host.attachShadow({ mode: 'open' }).appendChild(document.createElement('div')) + const root = createRoot(container) + flushSync(() => root.render(h('input', { type: 'checkbox', checked: false, onChange: () => {} }))) + cleanups.push(() => { flushSync(() => root.unmount()); host.remove() }) + const box = container.firstChild as HTMLInputElement + await click(box) + expect(box.checked).toBe(false) +}) + +it('restores an input portaled into a detached container', async () => { + const target = document.createElement('div') + mount(createPortal(h('input', { value: 'x', onChange: () => {} }), target)) + const field = target.firstChild as HTMLInputElement + await type(field, 'xy') + expect(field.value).toBe('x') +}) + +it('restores a checkbox whose only handler is onInput', async () => { + const box = mount(h('input', { type: 'checkbox', checked: false, onInput: () => {} })).firstChild as HTMLInputElement + await click(box) + expect(box.checked).toBe(false) +}) From c425e2ac639630ee5294aec3e5aa30c54228b9ad Mon Sep 17 00:00:00 2001 From: Bilal Vural <11067812+bilalvur@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:06:12 +0100 Subject: [PATCH 4/4] fix: call onChange for clicks on detached checkboxes and radios Browsers fire no `change` event when a checkbox or radio that is not connected to a document is clicked, for example `input.click()` on an element in a detached container. Redact maps a checkbox's or radio's onChange to the native `change` event, so neither the handler nor the controlled restore from #116 ran there. React reads checkbox and radio changes from `click`, which also reaches detached elements. The onChange listener now also listens for `click`, but acts on it only while the element is detached, and for a radio only when it was not checked before: its controlled `checked` prop, the checked state the last restore saw, or its default. Attached controls keep their `change` event, so their timing, the effect of preventDefault on the click and native `change` dispatches are unchanged. A full switch to `click` would change all of these for every app to cover a case that only arises outside the document. Co-Authored-By: Claude Opus 5.5 --- .changeset/detached-checkbox-change.md | 5 + packages/redact/src/dom/dom.ts | 29 +++- packages/redact/src/dom/form-controls.ts | 4 +- scripts/actions-browser.config.mjs | 2 +- scripts/size-check.mjs | 11 +- tests/detached-checkbox-change.browser.tsx | 86 ++++++++++ tests/detached-checkbox-change.test.tsx | 175 +++++++++++++++++++++ 7 files changed, 302 insertions(+), 10 deletions(-) create mode 100644 .changeset/detached-checkbox-change.md create mode 100644 tests/detached-checkbox-change.browser.tsx create mode 100644 tests/detached-checkbox-change.test.tsx diff --git a/.changeset/detached-checkbox-change.md b/.changeset/detached-checkbox-change.md new file mode 100644 index 0000000..219c09f --- /dev/null +++ b/.changeset/detached-checkbox-change.md @@ -0,0 +1,5 @@ +--- +'@tanstack/redact': patch +--- + +Call `onChange` for a checkbox or radio clicked in a container that is not in the document, and restore it when controlled, as React does. Browsers fire no `change` event there. diff --git a/packages/redact/src/dom/dom.ts b/packages/redact/src/dom/dom.ts index 821303c..1ec840b 100644 --- a/packages/redact/src/dom/dom.ts +++ b/packages/redact/src/dom/dom.ts @@ -2,7 +2,7 @@ import { createSyntheticEvent } from './synthetic-event'; import { UNITLESS_STYLE } from '../core/unitless-style'; import { devToolsMetadata } from '../core/devtools'; import { formActions } from './form-actions'; -import { queueRestore } from './form-controls'; +import { queueRestore, scheduleControlledRestore } from './form-controls'; import { attributeName, attributeValue } from '../core/attributes'; import { isString } from "../core/functions"; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -113,10 +113,13 @@ function setEventHandler(el: Element, name: string, next: any): void { const enabled = isFunction(next); const capture = name.endsWith('Capture') && !name.endsWith('PointerCapture'); const lower = name.slice(2, capture ? -7 : undefined).toLowerCase(); - const events = lower === 'change' && (el.tagName === 'TEXTAREA' || - (el.tagName === 'INPUT' && !/^(checkbox|radio|file)$/.test((el as HTMLInputElement).type))) - ? ['input', 'change'] - : [lower === 'focus' ? 'focusin' : lower === 'blur' ? 'focusout' : lower === 'doubleclick' ? 'dblclick' : lower]; + const type = (el as HTMLInputElement).type; + // Browsers fire no `change` for a checkbox or radio clicked outside a + // document; React listens for `click` there, so onChange still runs. + const events = lower !== 'change' ? [lower === 'focus' ? 'focusin' : lower === 'blur' ? 'focusout' : lower === 'doubleclick' ? 'dblclick' : lower] + : /^(checkbox|radio)$/.test(type) ? ['click', 'change'] + : el.tagName === 'TEXTAREA' || el.tagName === 'INPUT' && type !== 'file' ? ['input', 'change'] + : ['change']; // Key by the React prop name (not DOM event name) so `onChange` and // `onInput` on the same text input — which both dispatch from the native // `input` event — can coexist without clobbering each other's handlers. @@ -133,7 +136,15 @@ function setEventHandler(el: Element, name: string, next: any): void { return; if (!existing) { const entry = ((e: Event) => { - if (events.length > 1) { + const click = e.type === 'click' && events[1]; + if (click) { + // Only a click that checked a detached radio or toggled a + // detached checkbox; attached ones get their `change` event. + const props = (el as any).__p || {}; + if (el.isConnected || (el as HTMLInputElement).type === 'radio' && (props.checked ?? (el as any).__k ?? props.defaultChecked)) + return; + } + else if (events[0] === 'input') { const value = (el as HTMLInputElement).value; if (e.type === 'change' && (el as any).__v === value) return; @@ -148,7 +159,11 @@ function setEventHandler(el: Element, name: string, next: any): void { } else entry.c(event); } finally { event.currentTarget = null; - queueRestore(e, el); + // A detached click has no container to reach, so it restores here. + if (click) + scheduleControlledRestore(el as HTMLInputElement); + else + queueRestore(e, el); } }) as ((e: Event) => void) & { c: Function; diff --git a/packages/redact/src/dom/form-controls.ts b/packages/redact/src/dom/form-controls.ts index c4a9150..76b6818 100644 --- a/packages/redact/src/dom/form-controls.ts +++ b/packages/redact/src/dom/form-controls.ts @@ -91,7 +91,7 @@ function restorePending(e: any): void { // so an edit or click the app did not accept never stays in the DOM. Updates // from the handler flush in a microtask (or act's queue) queued before this // one, so the restore reads the props they render. -function scheduleControlledRestore(el: HTMLInputElement): void { +export function scheduleControlledRestore(el: HTMLInputElement): void { if (!(el as any).__p) return; const restore = () => { @@ -107,6 +107,8 @@ function scheduleControlledRestore(el: HTMLInputElement): void { writeInputValue(input, props.value); if (props.checked != null) input.checked = !!props.checked; + // Detached radios compare a click against this. + (input as any).__k = input.checked; } } }; diff --git a/scripts/actions-browser.config.mjs b/scripts/actions-browser.config.mjs index 2f21d79..d67d4d3 100644 --- a/scripts/actions-browser.config.mjs +++ b/scripts/actions-browser.config.mjs @@ -6,6 +6,6 @@ export default { cacheDir: resolve('node_modules/.vite/actions-browser'), test: { ...config.test, - include: ['actions.test.tsx', 'transition-retention.test.tsx', 'activity-transition.test.tsx', 'select-defaults.test.tsx', 'event-parity.test.tsx', 'lazy-wrapped-components.test.tsx', 'thenable-children.test.tsx', 'root-suspension.test.tsx', 'find-dom-node.test.tsx', 'document-shell.test.tsx', 'issue115-controlled-restore.browser.tsx'], + include: ['actions.test.tsx', 'transition-retention.test.tsx', 'activity-transition.test.tsx', 'select-defaults.test.tsx', 'event-parity.test.tsx', 'lazy-wrapped-components.test.tsx', 'thenable-children.test.tsx', 'root-suspension.test.tsx', 'find-dom-node.test.tsx', 'document-shell.test.tsx', 'issue115-controlled-restore.browser.tsx', 'detached-checkbox-change.browser.tsx'], }, } diff --git a/scripts/size-check.mjs b/scripts/size-check.mjs index 82a2431..058c4c1 100644 --- a/scripts/size-check.mjs +++ b/scripts/size-check.mjs @@ -81,11 +81,20 @@ const CONTROLLED_RESTORE_SIZE_ALLOWANCE = { 'redact/dom-client (nano)': [261, 262], } +// Detached checkbox and radio onChange cost on #116 (9fa1855), [source, built]. +// Pending maintainer approval. Only entries exceeding existing limits receive +// their measured increment. +const DETACHED_CHANGE_SIZE_ALLOWANCE = { + 'redact/dom-client (lazy=stub)': [84, 77], + 'redact/dom-client (classComponents=stub)': [73, 79], + 'redact/dom-client (hydration=stub)': [86, 94], +} + let failed = false for (const tree of ['src', 'dist']) { for (const row of await measureSizes(tree)) { const baseBudget = BUDGETS[row.name] - const budget = baseBudget === undefined ? undefined : baseBudget + (CONTROLLED_RESTORE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + const budget = baseBudget === undefined ? undefined : baseBudget + (DETACHED_CHANGE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (CONTROLLED_RESTORE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) const ok = budget !== undefined && row.gz <= budget failed ||= !ok console.log(`${ok ? 'PASS' : 'FAIL'} ${tree} ${row.name}: ${row.gz} B gzip, budget ${budget ?? 'missing'}`) diff --git a/tests/detached-checkbox-change.browser.tsx b/tests/detached-checkbox-change.browser.tsx new file mode 100644 index 0000000..e1cb997 --- /dev/null +++ b/tests/detached-checkbox-change.browser.tsx @@ -0,0 +1,86 @@ +import { afterEach, expect, it } from 'vitest' +import { userEvent } from '@vitest/browser/context' +import { createElement as h, useState } from 'react' +import { createRoot } from 'react-dom/client' +import { flushSync } from 'react-dom' + +const cleanups: Array<() => void> = [] +afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup() }) + +function mount(element: any, attached: boolean): HTMLElement { + const container = document.createElement('div') + if (attached) document.body.appendChild(container) + const root = createRoot(container) + flushSync(() => root.render(element)) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + return container +} + +const settle = () => new Promise(resolve => setTimeout(resolve)) + +function Form({ seen, accept }: { seen: string[], accept: boolean }) { + const [checked, setChecked] = useState(false) + const [value, setValue] = useState('a') + return h('div', null, + h('input', { + type: 'checkbox', + checked, + onChange: (event: any) => { + seen.push('checkbox ' + event.target.checked) + if (accept) setChecked(event.target.checked) + }, + }), + ['a', 'b'].map(key => h('input', { + key, + type: 'radio', + name: 'browser-' + accept, + checked: value === key, + onChange: () => { + seen.push('radio ' + key) + if (accept) setValue(key) + }, + })), + h('input', { + type: 'checkbox', + onChange: () => seen.push('prevented'), + onClick: (event: any) => event.preventDefault(), + }), + ) +} + +for (const accept of [true, false]) { + it(`handles real clicks on attached controls (accept: ${accept})`, async () => { + const seen: string[] = [] + const [checkbox, a, b, prevented] = [...mount(h(Form, { seen, accept }), true).querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement, HTMLInputElement, HTMLInputElement] + for (const input of [checkbox, a, b, b, prevented]) { + await userEvent.click(input) + await settle() + } + expect(seen).toEqual(accept + ? ['checkbox true', 'radio b'] + : ['checkbox true', 'radio b', 'radio b']) + expect([checkbox.checked, a.checked, b.checked, prevented.checked]).toEqual(accept + ? [true, false, true, false] + : [false, true, false, false]) + }) + + // Browsers fire no change event here, so onChange comes from the click. + it(`handles clicks on detached controls (accept: ${accept})`, async () => { + const seen: string[] = [] + const container = mount(h(Form, { seen, accept }), false) + const native: string[] = [] + container.addEventListener('change', () => native.push('change')) + const [checkbox, a, b] = [...container.querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement, HTMLInputElement] + for (const input of [checkbox, a, b, b]) { + input.click() + await settle() + } + expect(native).toEqual([]) + expect(seen).toEqual(accept + ? ['checkbox true', 'radio b'] + : ['checkbox true', 'radio b', 'radio b']) + expect([checkbox.checked, a.checked, b.checked]).toEqual(accept + ? [true, false, true] + : [false, true, false]) + }) +} diff --git a/tests/detached-checkbox-change.test.tsx b/tests/detached-checkbox-change.test.tsx new file mode 100644 index 0000000..f90951b --- /dev/null +++ b/tests/detached-checkbox-change.test.tsx @@ -0,0 +1,175 @@ +import { afterEach, expect, it } from 'vitest' +import { act, createElement as h, useState } from 'react' +import { createRoot } from 'react-dom/client' +import { flushSync } from 'react-dom' + +// Browsers fire no `change` for a checkbox or radio clicked outside a +// document. React listens for `click` there, so onChange still runs. +const cleanups: Array<() => void> = [] +afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup() }) + +function mount(element: any, attached = false): HTMLElement { + const container = document.createElement('div') + if (attached) document.body.appendChild(container) + const root = createRoot(container) + flushSync(() => root.render(element)) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + return container +} + +// Updates from an event handler flush in a microtask, as do restores. +const settle = () => new Promise(resolve => setTimeout(resolve)) + +const click = async (input: Element) => { + (input as HTMLElement).click() + await settle() +} + +function Checkbox({ seen, accept = true }: { seen: boolean[], accept?: boolean }) { + const [checked, setChecked] = useState(false) + return h('input', { + type: 'checkbox', + checked, + onChange: (event: any) => { + seen.push(event.target.checked) + if (accept) setChecked(event.target.checked) + }, + }) +} + +function Radios({ seen, accept = true, controlled = true, initial = 'a' }: { seen: string[], accept?: boolean, controlled?: boolean, initial?: string }) { + const [value, setValue] = useState(initial) + return h('div', null, ['a', 'b'].map(key => h('input', { + key, + type: 'radio', + name: 'detached', + value: key, + ...(controlled ? { checked: value === key } : { defaultChecked: initial === key }), + onChange: () => { + seen.push(key) + if (accept) setValue(key) + }, + }))) +} + +it.each([false, true])('calls onChange once per checkbox click (attached: %s)', async attached => { + const seen: boolean[] = [] + const input = mount(h(Checkbox, { seen }), attached).firstChild as HTMLInputElement + await click(input) + expect(input.checked).toBe(true) + await click(input) + expect(input.checked).toBe(false) + expect(seen).toEqual([true, false]) +}) + +it.each([false, true])('restores a checkbox that onChange rejected (attached: %s)', async attached => { + const seen: boolean[] = [] + const input = mount(h(Checkbox, { seen, accept: false }), attached).firstChild as HTMLInputElement + await click(input) + expect(seen).toEqual([true]) + expect(input.checked).toBe(false) +}) + +it('keeps an accepted detached checkbox click under act', async () => { + const seen: boolean[] = [] + const input = mount(h(Checkbox, { seen })).firstChild as HTMLInputElement + await act(() => input.click()) + expect(seen).toEqual([true]) + expect(input.checked).toBe(true) +}) + +it.each([false, true])('calls onChange only when a controlled radio becomes checked (attached: %s)', async attached => { + const seen: string[] = [] + const [a, b] = [...mount(h(Radios, { seen }), attached).querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(a) + await click(b) + await click(b) + await click(a) + expect(seen).toEqual(['b', 'a']) + expect([a.checked, b.checked]).toEqual([true, false]) +}) + +it.each([false, true])('calls onChange only when an uncontrolled radio becomes checked (attached: %s)', async attached => { + const seen: string[] = [] + const [a, b] = [...mount(h(Radios, { seen, controlled: false }), attached).querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(a) + await click(b) + await click(a) + await click(a) + expect(seen).toEqual(['b', 'a']) +}) + +it.each([false, true])('restores a radio group that onChange rejected (attached: %s)', async attached => { + const seen: string[] = [] + const [a, b] = [...mount(h(Radios, { seen, accept: false }), attached).querySelectorAll('input')] as [HTMLInputElement, HTMLInputElement] + await click(b) + expect(seen).toEqual(['b']) + expect([a.checked, b.checked]).toEqual([true, false]) +}) + +it('runs onChangeCapture before onChange on a detached checkbox', async () => { + const seen: string[] = [] + const container = mount(h('input', { + type: 'checkbox', + onChange: () => seen.push('bubble'), + onChangeCapture: () => seen.push('capture'), + })) + await click(container.firstChild as Element) + expect(seen).toEqual(['capture', 'bubble']) +}) + +it('calls onChange for a label click on a detached checkbox', async () => { + const seen: boolean[] = [] + const container = mount(h('label', null, h('input', { type: 'checkbox', onChange: (event: any) => seen.push(event.target.checked) }), 'Label')) + await click(container.firstChild as Element) + expect(seen).toEqual([true]) +}) + +it('does not call onChange for an attached checkbox click that onClick prevents', async () => { + const seen: string[] = [] + const input = mount(h('input', { + type: 'checkbox', + onChange: () => seen.push('change'), + onClick: (event: any) => { seen.push('click'); event.preventDefault() }, + }), true).firstChild as HTMLInputElement + await click(input) + expect(seen).toEqual(['click']) + expect(input.checked).toBe(false) +}) + +// React reads the change from the click, which preventDefault does not stop. +it('calls onChange for a detached checkbox click that onClick prevents', async () => { + const seen: string[] = [] + const input = mount(h('input', { + type: 'checkbox', + onClick: (event: any) => { seen.push('click'); event.preventDefault() }, + onChange: (event: any) => seen.push('change ' + event.target.checked), + })).firstChild as HTMLInputElement + await click(input) + expect(seen).toEqual(['click', 'change true']) + expect(input.checked).toBe(false) +}) + +it('keeps onChange on a native change event of a detached checkbox', async () => { + const seen: boolean[] = [] + const input = mount(h('input', { type: 'checkbox', onChange: (event: any) => seen.push(event.target.checked) })).firstChild as HTMLInputElement + input.checked = true + input.dispatchEvent(new Event('change', { bubbles: true })) + await settle() + expect(seen).toEqual([true]) +}) + +it('ignores clicks after a detached checkbox becomes a text input', async () => { + let setType!: (type: string) => void + const seen: string[] = [] + function App() { + const [type, set] = useState('checkbox') + setType = set + return h('input', { type, onChange: (event: any) => seen.push(event.target.type) }) + } + const input = mount(h(App)).firstChild as HTMLInputElement + await click(input) + flushSync(() => setType('text')) + await click(input) + expect(seen).toEqual(['checkbox']) +})