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 }) + }) })