From 311f05dfa7c2f0dc19099e105046e83a6903ef1a Mon Sep 17 00:00:00 2001 From: Aliaksandr Samuseu Date: Tue, 13 Oct 2026 16:32:51 +0200 Subject: [PATCH 1/2] EPBDS-16211: a bracket a Suspense boundary reopens no longer leaves validation paused An engine node opens its bracket in a layout effect and closes it in a passive effect. A Suspense boundary that hides the document and shows it again runs the layout effects again, and not the passive ones, on React 18 and 19: the bracket opened then never closed, and the form kept validation paused, so a required field emptied after that showed no error and left the form valid. `useBracket` (components/useBracket.ts) opens a bracket only for a bracket and key that no passive effect has closed one for since it last ran. RenderComponent and Modal use it. --- CLAUDE.md | 4 +- .../components/__tests__/useBracket.test.js | 109 ++++++++++++++++++ src/core/components/useBracket.ts | 37 ++++++ src/core/engine/components/Modal.tsx | 10 +- src/core/engine/mapper.tsx | 18 ++- .../__tests__/batchedForm.document.test.js | 38 +++++- src/core/modules/form/batchedForm.ts | 5 +- 7 files changed, 199 insertions(+), 22 deletions(-) create mode 100644 src/core/components/__tests__/useBracket.test.js create mode 100644 src/core/components/useBracket.ts diff --git a/CLAUDE.md b/CLAUDE.md index 067acde7..87419062 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -133,7 +133,9 @@ resumes. Two things pause it: - the brackets an engine node opens in a layout effect and closes in a passive effect, on mount and when its `showIf` flips (`registrationsOf`, used by `RenderComponent` in `mapper.tsx`). Fields register in passive effects, in between. `Modal` brackets its popup's fields the same way as it opens and closes, since nothing mounts when a - popup closes. + popup closes. Both open and close them through `useBracket` (`components/useBracket.ts`), which opens none when a + Suspense boundary shows a component again: React 18 and 19 run its layout effects then, and not its passive ones, + so nothing would close it, and the form would stay paused. A field that goes while validation is paused tells nothing until it resumes. React 16 runs a removed component's effect cleanups before any layout effect, so there a field that goes is not bracketed. diff --git a/src/core/components/__tests__/useBracket.test.js b/src/core/components/__tests__/useBracket.test.js new file mode 100644 index 00000000..f843a0ec --- /dev/null +++ b/src/core/components/__tests__/useBracket.test.js @@ -0,0 +1,109 @@ +/** + * WHAT A COMPONENT'S BRACKET COVERS, AND THAT IT ALWAYS CLOSES (`useBracket`, 2026-10-14). + * ============================================================================================= + * + * The bracket opens in a layout effect and closes in a passive effect, so the passive effects of the components + * below it, where fields register with their form, run while it is open. A Suspense boundary that hides the + * component and shows it again runs only its layout effects again, on React 18 and 19: a bracket opened then never + * closed, and the form it paused never validated again. React 16 and 17 run no effect then, and StrictMode runs + * their effects once, so their legs pass those two cases either way. + */ +import React, { StrictMode, Suspense, useEffect } from 'react' +import { act, render } from '@testing-library/react' +import { useBracket } from '../useBracket' + +const MAJOR = Number(React.version.split('.')[0]) + +/** A bracket that records its opening and closing, and what the component below it did, at which depth. */ +function recorded () { + const record = { log: [], depth: 0 } + record.bracket = { + begin: () => { record.depth += 1; record.log.push('begin') }, + end: () => { record.depth -= 1; record.log.push('end') }, + } + return record +} + +function Below ({ record, k }) { + useEffect(() => { record.log.push(`below ${k} at ${record.depth}`) }, [record, k]) + return null +} + +function Bracketed ({ record, bracket, k }) { + useBracket(bracket, k) + return +} + +/** What suspends while `pending` holds a promise. */ +let pending = null +function Later () { + if (pending) throw pending.promise + return null +} + +describe('useBracket', () => { + it('opens before the passive effects below it, and closes after them', () => { + const record = recorded() + render() + expect(record.log).toEqual(['begin', 'below a at 1', 'end']) + }) + + it('opens and closes again when its key changes, and not when it only renders again', () => { + const record = recorded() + const { rerender } = render() + rerender() + expect(record.log).toEqual(['begin', 'below a at 1', 'end']) + + rerender() + expect(record.log).toEqual(['begin', 'below a at 1', 'end', 'begin', 'below b at 1', 'end']) + }) + + it('opens nothing when a Suspense boundary hides it and shows it again', async () => { + const record = recorded() + const page = () => ( + Loading

}> + + + + ) + const { container, rerender } = render(page()) + let release + pending = { promise: new Promise(resolve => { release = resolve }) } + act(() => { rerender(page()) }) + expect(container.textContent).toBe('Loading') + + pending = null + // React 16 and 17 retry the boundary from the scheduler, which act does not drive there. + await act(async () => { + release() + await new Promise(resolve => setTimeout(resolve, 50)) + }) + expect(container.textContent).toBe('') + expect(record.log).toEqual(['begin', 'below a at 1', 'end']) + expect(record.depth).toBe(0) + }) + + it('opens and closes again for StrictMode\'s second mount, which runs the effects below it again', () => { + const record = recorded() + render() + const once = ['begin', 'below a at 1', 'end'] + expect(record.log).toEqual(MAJOR >= 18 ? [...once, ...once] : once) + }) + + // React 19's `` cleans up both kinds of effect of what it hides, and runs both again when it shows it. + ;(React.Activity ? it : it.skip)('opens and closes again when an shows it again', () => { + const { Activity } = React + const record = recorded() + const page = mode => + const { rerender } = render(page('visible')) + rerender(page('hidden')) + rerender(page('visible')) + expect(record.log).toEqual(['begin', 'below a at 1', 'end', 'begin', 'below a at 1', 'end']) + }) + + it('does nothing without a bracket', () => { + const record = recorded() + render() + expect(record.log).toEqual(['below a at 0']) + }) +}) diff --git a/src/core/components/useBracket.ts b/src/core/components/useBracket.ts new file mode 100644 index 00000000..7209385f --- /dev/null +++ b/src/core/components/useBracket.ts @@ -0,0 +1,37 @@ +import { useEffect, useLayoutEffect, useRef } from 'react' + +// A layout effect in a browser. On the server, where no effect runs, an effect: React 16 and 17 warn about a layout +// effect there. +const useBeforePaintEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect + +/** Brackets around what a commit mounts and unmounts in its passive effects. They nest. */ +export type Bracket = { begin: () => void, end: () => void } + +/** + * Opens `bracket` in a layout effect and closes it in a passive effect, on mount and when `bracket` or `key` + * changes. A commit runs its passive effects after its layout effects, so what mounts or goes in them comes between, + * as fields that register with a form and unregister do (`registrationsOf`, `modules/form/batchedForm.ts`). + * + * A Suspense boundary that hid the component and shows it again runs its layout effects again, and not its passive + * ones, on React 18 and 19. Until 2026-10-14 a bracket opened then all the same, and nothing closed it: the form kept + * validation paused for good, and a required field emptied after that showed no error. So the layout effect opens a + * bracket only for a `bracket` and `key` that no passive effect has closed one for since it last ran. + * StrictMode's second mount in development cleans the passive effect up and runs it again, so it is bracketed. + * + * @param bracket - what to open and close; nothing, for a component outside any form + * @param key - what, when it changes, mounts or removes what the bracket is for: whether a node is shown, or a popup open + */ +export function useBracket (bracket: Bracket | undefined, key: unknown): void { + // The `bracket` and `key` the passive effect closed a bracket for, until that effect is cleaned up. + const closed = useRef<{ bracket: Bracket | undefined, key: unknown } | null>(null) + useBeforePaintEffect(() => { + const last = closed.current + if (last !== null && last.bracket === bracket && Object.is(last.key, key)) return + bracket?.begin() + }, [bracket, key]) + useEffect(() => { + bracket?.end() + closed.current = { bracket, key } + return () => { closed.current = null } + }, [bracket, key]) +} diff --git a/src/core/engine/components/Modal.tsx b/src/core/engine/components/Modal.tsx index 2da6ae38..858eb735 100644 --- a/src/core/engine/components/Modal.tsx +++ b/src/core/engine/components/Modal.tsx @@ -1,18 +1,15 @@ -import React, { useContext, useEffect, useLayoutEffect, useRef, useState } from 'react' +import React, { useContext, useEffect, useRef, useState } from 'react' import { createPortal } from 'react-dom' import { registrationsOf } from '../../modules/form/batchedForm' import Button from '../../components/Button' import Text from '../../components/Text' +import { useBracket } from '../../components/useBracket' import View from '../../components/View' import { l, localiseTranslation } from '../../utils' import { _ } from '../../utils/translations' import { AppContext } from '../../contexts' import type { Translate } from '../../utils/_envs' -// A layout effect in a browser. On the server, where no effect runs, an effect: React 16 and 17 warn about a layout -// effect there. -const useBeforePaintEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect - localiseTranslation({ CANCEL: { [l.ENGLISH]: 'Cancel', @@ -103,8 +100,7 @@ const Modal = ({ translate, form }: { translate?: Translate, form?: unknown }) = // A popup's fields go when it closes, and nothing mounts then whose node would bracket their going: the // dialog brackets what opening and closing it registers and unregisters, in `form`, the document's, which // its content's fields register with (`registrationsOf`, `modules/form/batchedForm.ts`). - useBeforePaintEffect(() => { registrationsOf(form)?.begin() }, [form, isOpen]) - useEffect(() => { registrationsOf(form)?.end() }, [form, isOpen]) + useBracket(registrationsOf(form), isOpen) const onKeyDown = (event: React.KeyboardEvent) => { // A control inside that used the key claims it: an open listbox closes on Escape and says so. diff --git a/src/core/engine/mapper.tsx b/src/core/engine/mapper.tsx index 2beaae4c..2f9426d0 100644 --- a/src/core/engine/mapper.tsx +++ b/src/core/engine/mapper.tsx @@ -1,4 +1,4 @@ -import React, { memo, useContext, useEffect, useLayoutEffect } from 'react' +import React, { memo, useContext } from 'react' import { registrationsOf } from '../modules/form/batchedForm' import AutoSave from '../modules/form/views/AutoSave' import { FIELD } from '../modules/variables' @@ -23,6 +23,7 @@ import Space from '../components/Space' import Table from '../components/Table' import Text from '../components/Text' import Tooltip from '../components/Tooltip' +import { useBracket } from '../components/useBracket' import View from '../components/View' import { Active, isList, isNumeric, toFlatList, toJSON } from '../utils' import { TIME_DURATION_INSTANT } from '../utils/constants' @@ -39,10 +40,6 @@ import Data from './Data' import { AppContext, TableColumnsContext } from '../contexts' import type { RenderErrorReport } from './Render' -// A layout effect in a browser. On the server, where no effect runs, an effect: React 16 and 17 warn about a layout -// effect there. -const useBeforePaintEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect - /** * A meta node's attributes, as the mapper receives them from `Render`. `any`, deliberately: they are * open JSON, which `validateMeta` checks at runtime and the published contract types describe, and @@ -243,12 +240,11 @@ const RenderComponent = ({ /* General showIf logic — the whole decision lives in `showIf.ts`, testable on its own. */ const shown = shouldRender({ showIf, data, _data, relativeData, relativePath, relativeIndex, instance }) // The fields that mount below this node, or that go when it hides, register and unregister as one burst, which - // the form tells every field of once (`registrationsOf`, `modules/form/batchedForm.ts`). Its layout effect runs - // before any field below registers, in the passive effects, and its passive effect after they all have. When a - // tab, a step or a page changes, the new content's nodes bracket the old content's fields going as well: those - // go in the same commit's passive effects, which come between. - useBeforePaintEffect(() => { registrationsOf(form)?.begin() }, [form, shown]) - useEffect(() => { registrationsOf(form)?.end() }, [form, shown]) + // the form tells every field of once (`registrationsOf`, `modules/form/batchedForm.ts`). The bracket opens in a + // layout effect, before any field below registers in the passive effects, and closes in a passive effect, after + // they all have (`useBracket`). When a tab, a step or a page changes, the new content's nodes bracket the old + // content's fields going as well: those go in the same commit's passive effects, which come between. + useBracket(registrationsOf(form), shown) if (!shown) return null switch (view) { diff --git a/src/core/modules/form/__tests__/batchedForm.document.test.js b/src/core/modules/form/__tests__/batchedForm.document.test.js index fa66f400..bb759b90 100644 --- a/src/core/modules/form/__tests__/batchedForm.document.test.js +++ b/src/core/modules/form/__tests__/batchedForm.document.test.js @@ -9,7 +9,7 @@ * valid. Removing a row is not such a case, and is checked too: the removal changes the form's values, and its * listeners hear the state that follows from that. */ -import React from 'react' +import React, { Suspense } from 'react' import { act, fireEvent, render, screen } from '@testing-library/react' import UIRender from '../../../../library/main' import { formsStorage } from '../../../state/formRegistry' @@ -111,6 +111,42 @@ describe('a document whose fields registered silently as it mounted', () => { expect(heard[heard.length - 1]).toBe(true) }) + it('keeps validating after a Suspense boundary hides the document and shows it again', async () => { + // React 18 and 19 run the layout effects of what such a boundary shows again, and not its passive ones: a + // node's bracket opened then, and nothing closed it (`useBracket`), so the form never validated again. React + // 16 and 17 run neither, and pass either way. + let pending = null + const Later = () => { + if (pending) throw pending.promise + return null + } + const meta = { view: 'Input', name: 'note', label: 'Note', validate: 'required' } + const page = () => ( + Loading

}> + {}} /> + + + ) + const { rerender } = render(page()) + const { form, heard } = formHolding('note') + let release + pending = { promise: new Promise(resolve => { release = resolve }) } + act(() => { rerender(page()) }) + expect(screen.getByText('Loading')).toBeTruthy() + + pending = null + await act(async () => { + release() + // React 16 and 17 retry the boundary from the scheduler, which act does not drive there. + await new Promise(resolve => setTimeout(resolve, 50)) + }) + expect(screen.queryByText('Loading')).toBeNull() + + act(() => { form.change('note', '') }) + expect(form.getState().valid).toBe(false) + expect(heard[heard.length - 1]).toBe(false) + }) + it('tells them so when the row with the empty required field is removed', () => { jest.useFakeTimers() try { diff --git a/src/core/modules/form/batchedForm.ts b/src/core/modules/form/batchedForm.ts index 4ffd245e..15a9209a 100644 --- a/src/core/modules/form/batchedForm.ts +++ b/src/core/modules/form/batchedForm.ts @@ -3,8 +3,9 @@ import type { Config, FormApi } from 'final-form' /** * Brackets around a burst of fields registering and unregistering, which an engine node opens in its layout effect - * and closes in its passive effect (`RenderComponent` in `engine/mapper.tsx`), on mount and when its `showIf` flips. - * Brackets nest. The outermost one pauses validation when nothing else has, and resumes it when it closes. + * and closes in its passive effect (`RenderComponent` in `engine/mapper.tsx`), on mount and when its `showIf` flips, + * through `useBracket` (`components/useBracket.ts`). Brackets nest. The outermost one pauses validation when nothing + * else has, and resumes it when it closes. */ export type Registrations = { begin: () => void, end: () => void } From a922320056ea461b01c2b320ff5acbc3f091ccde Mon Sep 17 00:00:00 2001 From: Aliaksandr Samuseu Date: Tue, 13 Oct 2026 16:48:19 +0200 Subject: [PATCH 2/2] EPBDS-16211: fields that go on their own cost the form one pass An Expand's content goes when its collapse's animation ends, from its own timer, and a table's rows go when they collapse, the table's own state, or when a host's data has fewer of them. Nothing mounts then whose node would bracket the fields that go, so each one validated every field left and told them all, as a closed popup's did before. `Expand` takes a `bracket`, which the engine hands it, and ExpandList through it, and opens it around its content mounting and going. `TableView` brackets a change of which rows are expanded, and of how many items it has. Both go through `useBracket`. Beside 100 other required fields, in jsdom: an Expand's collapse 60800 field states and 15150 validators before, 600 and 100 now; a table's 50 rows collapsing 25302 and 6275, now 606 and 101; data with 5 of 50 rows 57450 and 14307, now 1755 and 333. In Chromium, collapsing an Expand that holds a 200-row table beside 200 required fields took 535 ms, and takes 124. --- CLAUDE.md | 9 ++- src/core/components/Expand.tsx | 11 +++ .../__tests__/Expand.lifecycle.test.js | 36 ++++++++- src/core/engine/components/TableView.tsx | 15 +++- src/core/engine/mapper.tsx | 5 +- .../__tests__/batchedForm.document.test.js | 50 ++++++++++++ src/core/modules/form/batchedForm.ts | 5 +- src/demo/markdowns/changelog.md | 6 ++ .../__tests__/later-mount-budget.test.js | 77 ++++++++++++++++++- 9 files changed, 200 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 87419062..4c81e950 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,10 +132,11 @@ resumes. Two things pause it: - react-final-form, for the first render; - the brackets an engine node opens in a layout effect and closes in a passive effect, on mount and when its `showIf` flips (`registrationsOf`, used by `RenderComponent` in `mapper.tsx`). Fields register in passive effects, - in between. `Modal` brackets its popup's fields the same way as it opens and closes, since nothing mounts when a - popup closes. Both open and close them through `useBracket` (`components/useBracket.ts`), which opens none when a - Suspense boundary shows a component again: React 18 and 19 run its layout effects then, and not its passive ones, - so nothing would close it, and the form would stay paused. + in between. Fields can also go when nothing mounts and no `showIf` flips, and the component that removes them + brackets them the same way: `Modal` as a popup opens and closes, `Expand` when a collapse's animation ends, and + `TableView` when rows collapse or go with the items. All of them open and close brackets through `useBracket` + (`components/useBracket.ts`), which opens none when a Suspense boundary shows a component again: React 18 and 19 + run its layout effects then, and not its passive ones, so nothing would close it, and the form would stay paused. A field that goes while validation is paused tells nothing until it resumes. React 16 runs a removed component's effect cleanups before any layout effect, so there a field that goes is not bracketed. diff --git a/src/core/components/Expand.tsx b/src/core/components/Expand.tsx index 4301c5f0..5237d76f 100644 --- a/src/core/components/Expand.tsx +++ b/src/core/components/Expand.tsx @@ -5,6 +5,8 @@ import AnimateHeight from './AnimateHeight' import Icon from './Icon' import { STYLE } from './styles' import Text from './Text' +import { useBracket } from './useBracket' +import type { Bracket } from './useBracket' import View from './View' /** What `onClick` is told on every change: the new state, and which `Expand` it was. */ @@ -39,6 +41,11 @@ export type ExpandProps = { renderLabel?: (title: React.ReactNode) => React.ReactNode /** Argument to pass to 'onClick' callback as `index` */ index?: string | number + /** + * Opened before the content mounts or goes, and closed after (`useBracket`): the engine hands it the brackets of + * the document's form, so that the fields of the content register, or unregister, as one burst + */ + bracket?: Bracket [key: string]: unknown } @@ -100,6 +107,7 @@ function Expand (props: ExpandProps) { iconClosed = 'chevron-right', iconOpened = 'chevron-down', renderLabel, + bracket, ...rest } = props const [state, setState] = useState(() => ({ @@ -142,6 +150,9 @@ function Expand (props: ExpandProps) { const {expanded, changing} = state const hasContent = children != null const shown = hasContent && (expanded || changing) + // The content goes a `duration` after a collapse, from the timer above, when nothing else mounts or changes that + // would bracket its fields going: so the Expand brackets them itself, and those that come when it expands. + useBracket(bracket, shown) // A render function's content is built when it is first shown, and kept while `children` is the same, // however often it is collapsed and shown again: the function is called once (`Expand.lifecycle.test.js`). // A ref held it until 2026-10-08. It is state now, stored by the render that builds it, which then runs diff --git a/src/core/components/__tests__/Expand.lifecycle.test.js b/src/core/components/__tests__/Expand.lifecycle.test.js index ef2511d3..783df085 100644 --- a/src/core/components/__tests__/Expand.lifecycle.test.js +++ b/src/core/components/__tests__/Expand.lifecycle.test.js @@ -1,4 +1,4 @@ -import React from 'react' +import React, { useEffect } from 'react' import { act, fireEvent, render, screen } from '@testing-library/react' import '@testing-library/jest-dom' import Expand from '../Expand' @@ -10,6 +10,8 @@ const wrap = ui => ( const labelOf = container => container.querySelector('.app__expand > .text') +const MAJOR = Number(React.version.split('.')[0]) + describe('Expand lifecycle contracts', () => { beforeEach(() => jest.useFakeTimers()) afterEach(() => jest.useRealTimers()) @@ -310,4 +312,36 @@ describe('Expand lifecycle contracts', () => { cleared.mockRestore() } }) + + it('brackets its content mounting, and its content going when the collapse is over', () => { + // The engine hands an Expand the brackets of its document's form (`useBracket`). The content goes from a + // timer, a `duration` after the collapse, when nothing else would bracket its fields going. + const log = [] + let depth = 0 + const bracket = { + begin: () => { depth += 1; log.push('begin') }, + end: () => { depth -= 1; log.push('end') }, + } + const Content = () => { + useEffect(() => { + log.push(`mounted at ${depth}`) + return () => { log.push(`gone at ${depth}`) } + }, []) + return Content + } + const { container } = render(wrap()) + // The mount's own bracket, with nothing in it: the content is collapsed. + expect(log).toEqual(['begin', 'end']) + + fireEvent.click(labelOf(container)) + expect(log.slice(2)).toEqual(['begin', 'mounted at 1', 'end']) + + fireEvent.click(labelOf(container)) + expect(log.slice(5)).toEqual([]) + act(() => jest.advanceTimersByTime(100)) + expect(screen.queryByText('Content')).not.toBeInTheDocument() + // React 16 runs a removed component's effect cleanups during the commit, before any layout effect. + expect(log.slice(5)).toEqual(MAJOR > 16 ? ['begin', 'gone at 1', 'end'] : ['gone at 0', 'begin', 'end']) + expect(depth).toBe(0) + }) }) diff --git a/src/core/engine/components/TableView.tsx b/src/core/engine/components/TableView.tsx index f1017cfa..e4cce39d 100644 --- a/src/core/engine/components/TableView.tsx +++ b/src/core/engine/components/TableView.tsx @@ -6,6 +6,8 @@ import Row from '../../components/Row' import ScrollView from '../../components/ScrollView' import Table from '../../components/Table' import Text from '../../components/Text' +import { useBracket } from '../../components/useBracket' +import type { Bracket } from '../../components/useBracket' import View from '../../components/View' import { ENGINE_PROPS, FIELD_ONLY_PROPS, omitProps } from '../../components/domProps' import { by, get, hasListValue, isEqual, isEqualList, isFunction, toJSON } from '../../utils' @@ -108,6 +110,11 @@ export type TableViewProps = { className?: string name?: string fieldArrayName?: string + /** + * Opened before rows that expand or collapse, or come or go with the items, mount or go, and closed after + * (`useBracket`): the engine hands it the brackets of the document's form + */ + bracket?: Bracket [key: string]: unknown } @@ -484,6 +491,12 @@ function TableView (props: TableViewProps) { Object.assign(self, {props, state, headers, itemsSorted, renderHeader, renderItem, renderItemsVertical, renderItemData}) + // The fields of rows that collapse, and of rows that go with the items, go when nothing mounts or changes that would + // bracket their going: the table brackets them itself, and those of rows that expand or come. Rows are keyed by + // their index, so rows go only when there are fewer. A page's rows go when another page's mount, which bracket them. + useBracket(props.bracket, state.items) + useBracket(props.bracket, itemsSorted.length) + if (!headers) return {'Table has no data!'} const { fill, className, sorts, onSort, extraHeaders, renderExtraItem, showEmptyAs, vertical, colGroup, usePagination, @@ -494,7 +507,7 @@ function TableView (props: TableViewProps) { // carried over: a dead entry in the list whose whole job is keeping props off the DOM // reads as protection that is not there. itemsExpanded: _6, translate: _7, additionalCellsStyles: _8, rowsPerPage: _9, - fieldArrayName: _10, + fieldArrayName: _10, bracket: _11, ...rest } = props const {activePage, rowsPerPage} = state diff --git a/src/core/engine/mapper.tsx b/src/core/engine/mapper.tsx index 2f9426d0..d069c2e4 100644 --- a/src/core/engine/mapper.tsx +++ b/src/core/engine/mapper.tsx @@ -321,11 +321,11 @@ const RenderComponent = ({ } if (props.name != null && props.title == null) props.title = props.name if (items.length) props.children = () => items.map(Render) - return + return } case FIELD.TYPE.EXPAND_LIST: { - return + return } case FIELD.TYPE.BUTTON: { @@ -527,6 +527,7 @@ const RenderComponent = ({ fieldArrayName={fieldArrayName} {...table} translate={translate} + bracket={registrationsOf(form)} /> } diff --git a/src/core/modules/form/__tests__/batchedForm.document.test.js b/src/core/modules/form/__tests__/batchedForm.document.test.js index bb759b90..264a72c2 100644 --- a/src/core/modules/form/__tests__/batchedForm.document.test.js +++ b/src/core/modules/form/__tests__/batchedForm.document.test.js @@ -111,6 +111,56 @@ describe('a document whose fields registered silently as it mounted', () => { expect(heard[heard.length - 1]).toBe(true) }) + it('tells them it is valid when an Expand that holds an empty required field collapses', async () => { + const meta = { + view: 'Col', + items: [ + { view: 'Input', name: 'title', label: 'Title' }, + { view: 'Expand', title: 'Details', items: [{ view: 'Input', name: 'detail', label: 'Detail', validate: 'required' }] }, + ], + } + render( {}} />) + const { form, heard } = formHolding('title') + const title = screen.getByRole('button', { name: 'Details' }) + expect(heard).toEqual([true]) + + await act(async () => { fireEvent.click(title) }) + expect(screen.getByLabelText('Detail')).toBeTruthy() + expect(heard[heard.length - 1]).toBe(false) + + await act(async () => { fireEvent.click(title) }) + // The content goes when the collapse's animation ends, 500 ms after the click. + await act(async () => { await new Promise(resolve => setTimeout(resolve, 600)) }) + expect(screen.queryByLabelText('Detail')).toBeNull() + expect(form.getState().valid).toBe(true) + expect(heard[heard.length - 1]).toBe(true) + }) + + it('tells them it is valid when a table\'s rows collapse, one of them with an empty required field', async () => { + const meta = { + view: 'Table', + name: 'rows', + headers: [ + { id: 'note', label: 'Note' }, + { id: '#', renderHeader: { view: 'Checkbox', label: 'Expand all', onChange: 'handleToggleExpandAll' } }, + ], + renderItem: { view: 'Input', name: 'detail', label: 'Detail', validate: 'required' }, + } + const data = () => ({ rows: [{ note: 'A', detail: 'a' }, { note: 'B', detail: '' }] }) + render( {}} />) + const { form, heard } = formHolding('rows') + expect(heard).toEqual([true]) + + await act(async () => { fireEvent.click(screen.getByLabelText('Expand all')) }) + expect(screen.getAllByLabelText('Detail')).toHaveLength(2) + expect(heard[heard.length - 1]).toBe(false) + + await act(async () => { fireEvent.click(screen.getByLabelText('Expand all')) }) + expect(screen.queryByLabelText('Detail')).toBeNull() + expect(form.getState().valid).toBe(true) + expect(heard[heard.length - 1]).toBe(true) + }) + it('keeps validating after a Suspense boundary hides the document and shows it again', async () => { // React 18 and 19 run the layout effects of what such a boundary shows again, and not its passive ones: a // node's bracket opened then, and nothing closed it (`useBracket`), so the form never validated again. React diff --git a/src/core/modules/form/batchedForm.ts b/src/core/modules/form/batchedForm.ts index 15a9209a..8d99dc75 100644 --- a/src/core/modules/form/batchedForm.ts +++ b/src/core/modules/form/batchedForm.ts @@ -4,8 +4,9 @@ import type { Config, FormApi } from 'final-form' /** * Brackets around a burst of fields registering and unregistering, which an engine node opens in its layout effect * and closes in its passive effect (`RenderComponent` in `engine/mapper.tsx`), on mount and when its `showIf` flips, - * through `useBracket` (`components/useBracket.ts`). Brackets nest. The outermost one pauses validation when nothing - * else has, and resumes it when it closes. + * through `useBracket` (`components/useBracket.ts`). `Modal`, `Expand` and `TableView` do the same around the fields + * they remove on their own. Brackets nest. The outermost one pauses validation when nothing else has, and resumes it + * when it closes. */ export type Registrations = { begin: () => void, end: () => void } diff --git a/src/demo/markdowns/changelog.md b/src/demo/markdowns/changelog.md index cfdb9fc2..70b9ac2f 100644 --- a/src/demo/markdowns/changelog.md +++ b/src/demo/markdowns/changelog.md @@ -866,6 +866,12 @@ Nothing to do for these: #### Fixes +- **Fields that go on their own cost the form one pass.** An Expand's content goes when its collapse's animation + ends, and a table's rows go when they collapse or when a host's data has fewer of them. Nothing mounts then whose + node would bracket the fields that go, so each one validated every field left, as a closed popup's did. `Expand` + and the table bracket them now. In Chromium, collapsing an Expand that holds a 200-row table beside 200 required + fields takes 124 ms instead of 535: 8 to 11 ms of it is script, and the rest is the frames of the animation. On + React 16 these fields still go one by one. - **Fields that mount or go later cost the form one pass in two more cases.** react-final-form's probe of a field's first render validated every field in the form while validation ran, once for each new field: in Chromium, showing a 200-row table beside 200 required fields took 153 ms, and takes 98. A popup's fields diff --git a/src/library/__tests__/later-mount-budget.test.js b/src/library/__tests__/later-mount-budget.test.js index 47bf67ea..41888c67 100644 --- a/src/library/__tests__/later-mount-budget.test.js +++ b/src/library/__tests__/later-mount-budget.test.js @@ -21,6 +21,12 @@ * before any bracket opened. Here that was each field once more. Beside 100 other required fields, it was 101 times * each new field: showing the table there ran 10502 validators, 10201 of them from the probes. * + * Fields can also go when nothing mounts and no `showIf` flips: a popup that closes, an Expand whose collapse ends, rows + * that collapse, or that go with the host's data. The component that removes them brackets them then: `Modal`, + * `Expand` and `TableView` (2026-10-14). Before, beside 100 other required fields, each of these validated every + * field left once for each field that went: closing the popup ran 15150 validators, the Expand's collapse 15150, + * collapsing the table's rows 6275, and data with 5 of its 50 rows 14307. + * * React 16 runs a removed component's effect cleanups during the commit, before any layout effect, so no bracket is * open when the table's fields go: there they still go one by one, and cost what they did. A number here that falls * is progress: lower it. One that rises is a regression. @@ -57,8 +63,9 @@ jest.mock('final-form', () => { }) const ROWS = 50 +const MAJOR = Number(React.version.split('.')[0]) // Whether the fields that go are bracketed too: not on React 16 (see above). -const REMOVALS_BRACKETED = Number(React.version.split('.')[0]) > 16 +const REMOVALS_BRACKETED = MAJOR > 16 const table = { view: 'Table', name: 'rows', @@ -69,18 +76,21 @@ const table = { } const data = () => ({ show: 'no', rows: Array.from({ length: ROWS }, (_, i) => ({ note: `note ${i}`, qty: i })) }) -/** What an action costs the form. */ -async function costOf (action) { +/** What an action costs the form, with what it sets off within `wait` milliseconds. */ +async function costOf (action, wait = 100) { mockCount.computed = 0 mockCount.validated = 0 await act(async () => { action() // A tab changes after its 50 ms transition (`Tabs.tsx`). - await new Promise(resolve => setTimeout(resolve, 100)) + await new Promise(resolve => setTimeout(resolve, wait)) }) return { ...mockCount } } +const besideOthers = () => Array.from({ length: 100 }, (_, i) => ({ view: 'Input', name: `other${i}`, validate: 'required' })) +const valuesOf = fields => Object.fromEntries(fields.map(({ name }) => [name, 'x'])) + const formHolding = key => [...formsStorage.values()].map(entry => entry.form).find(candidate => key in candidate.getState().values) describe('fields that mount after the first render: a 50-row table of two fields per row', () => { @@ -145,4 +155,63 @@ describe('fields that mount after the first render: a 50-row table of two fields // validated and told every field left, 15150 validators in all, which React 16 still does. expect(closed).toEqual(REMOVALS_BRACKETED ? { computed: 600, validated: 100 } : { computed: 60800, validated: 15150 }) }) + + it('costs an Expand that holds it beside 100 other required fields, opening and collapsing, a pass over them each', async () => { + // The content goes when the collapse's animation ends, 500 ms after the click, from a timer: nothing mounts or + // changes then that would bracket its fields going, so the Expand brackets them. + const others = besideOthers() + const meta = { view: 'Col', items: [...others, { view: 'Expand', title: 'Details', items: [table] }] } + const values = { ...data(), ...valuesOf(others) } + render( {}} />) + const form = formHolding('other0') + const title = screen.getByRole('button', { name: 'Details' }) + const opened = await costOf(() => fireEvent.click(title)) + expect(form.getRegisteredFields()).toHaveLength(2 * ROWS + 101) + // The click starts the collapse, and nothing goes yet. + expect(await costOf(() => fireEvent.click(title))).toEqual({ computed: 0, validated: 0 }) + const collapsed = await costOf(() => {}, 600) + expect(form.getRegisteredFields()).toHaveLength(100) + expect(opened).toEqual({ computed: 2011, validated: 201 }) + expect(collapsed).toEqual(REMOVALS_BRACKETED ? { computed: 600, validated: 100 } : { computed: 60800, validated: 15150 }) + }) + + it('costs every row of a table expanding beside 100 other required fields, and collapsing again, a pass over them each', async () => { + // Which rows are expanded is the table's own state: nothing mounts or changes when they collapse that would + // bracket their fields going, so the table brackets them. + const others = besideOthers() + const rows = { + view: 'Table', + name: 'rows', + headers: [ + { id: 'note', label: 'Note' }, + { id: '#', renderHeader: { view: 'Checkbox', label: 'Expand all', onChange: 'handleToggleExpandAll' } }, + ], + renderItem: { view: 'Input', name: 'detail', label: 'Detail', validate: 'required' }, + } + const values = { rows: data().rows.map(({ note }, i) => ({ note, detail: `detail ${i}` })), ...valuesOf(others) } + render( {}} />) + const form = formHolding('other0') + const expanded = await costOf(() => fireEvent.click(screen.getByLabelText('Expand all'))) + expect(form.getRegisteredFields()).toHaveLength(ROWS + 101) + const collapsed = await costOf(() => fireEvent.click(screen.getByLabelText('Expand all'))) + expect(form.getRegisteredFields()).toHaveLength(101) + expect(expanded).toEqual({ computed: 1459, validated: 151 }) + expect(collapsed).toEqual(REMOVALS_BRACKETED ? { computed: 606, validated: 101 } : { computed: 25302, validated: 6275 }) + }) + + it('costs data with 5 of its 50 rows, beside 100 other required fields, a pass over what is left', async () => { + // The table's rows are keyed by their index, so rows go only when there are fewer, and nothing mounts then that + // would bracket their fields going: the table brackets them. + const others = besideOthers() + const meta = { view: 'Col', items: [...others, table] } + const values = count => ({ rows: data().rows.slice(0, count), ...valuesOf(others) }) + const { rerender } = render( {}} />) + const form = formHolding('other0') + const fewer = await costOf(() => rerender( {}} />)) + expect(form.getRegisteredFields()).toHaveLength(2 * 5 + 101) + // React 18 computes the form's state once more here, two field states for each field left, with the bracket or + // without it: measured on 18.3.1, against 17.0.2 and 19.3.0. + const bracketed = MAJOR === 18 ? { computed: 1977, validated: 333 } : { computed: 1755, validated: 333 } + expect(fewer).toEqual(REMOVALS_BRACKETED ? bracketed : { computed: 57201, validated: 14217 }) + }) })