EPBDS-16211: fields that mount or go later cost the form one pass in two more cases

CLAUDE.md

@@ -132,11 +132,19 @@ 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.
+  in between. `Modal` brackets its popup's fields the same way as it opens and closes, since nothing mounts when a
+  popup closes.
 
 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. `form-mount-budget.test.js`
-and `later-mount-budget.test.js` count what the bursts cost.
+effect cleanups before any layout effect, so there a field that goes is not bracketed.
+
+react-final-form also probes each field in its first render, registering and unregistering it for its first
+state. While validation runs, the probe registers without its validator, as it would on a document's first render:
+with one, it validated every field in the form, once per new field. The field gets its error when its node's
+bracket closes. That depends on its first registration from an effect coming inside a bracket, since
+react-final-form takes the first notification after it as the probe's state.
+
+`form-mount-budget.test.js` and `later-mount-budget.test.js` count what the bursts cost.
 
 To find what a cost comes from, count before naming the cause: wrap the suspected function or component with
 `jest.mock` and count calls per action. Comparing timings with and without a feature does not name a cause.

src/core/engine/components/Modal.tsx

@@ -1,5 +1,6 @@
-import React, { useContext, useEffect, useRef, useState } from 'react'
+import React, { useContext, useEffect, useLayoutEffect, 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 View from '../../components/View'
@@ -8,6 +9,10 @@ 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',
@@ -70,7 +75,7 @@ let popups = 0
  * of the document constructed LAST: on a page with two documents, the first one's popup showed
  * the second one's language around its own content.
  */
-const Modal = ({ translate }: { translate?: Translate }) => {
+const Modal = ({ translate, form }: { translate?: Translate, form?: unknown }) => {
     const popup = useContext(AppContext)
     const { isOpen, title, content, togglePopupState } = popup
     const activeClass = isOpen ? ' active' : ''
@@ -95,6 +100,12 @@ const Modal = ({ translate }: { translate?: Translate }) => {
         }
     }, [isOpen])
 
+    // 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])
+
     const onKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
         // A control inside that used the key claims it: an open listbox closes on Escape and says so.
         if (event.key !== 'Escape' || event.defaultPrevented) return

src/core/engine/rules.tsx

@@ -458,7 +458,7 @@ export class UIRender extends DocumentInstance {
                     document with no content, and a host relies on Enter in a field submitting nothing. */}
                 {content}
                 {childAfter}
-                <Modal translate={this.translate || Active.translate} />
+                <Modal translate={this.translate || Active.translate} form={this.form} />
             </Container>
 
         // The configuration props are published here, around the whole subtree, rather than

src/core/modules/form/__tests__/batchedForm.document.test.js

@@ -88,6 +88,29 @@ describe('a document whose fields registered silently as it mounted', () => {
         expect(heard[heard.length - 1]).toBe(true)
     })
 
+    it('tells them it is valid when a popup that holds an empty required field closes', async () => {
+        const meta = {
+            view: 'Col',
+            items: [
+                { view: 'Input', name: 'title', label: 'Title' },
+                { view: 'Button', children: 'Open details', onClick: { name: 'popupOpen', args: ['details'] } },
+                { view: 'Popup', id: 'details', title: 'Details', items: [{ view: 'Input', name: 'detail', label: 'Detail', validate: 'required' }] },
+            ],
+        }
+        render(<UIRender meta={meta} data={{ title: 'x', detail: '' }} initialValues={{ title: 'x', detail: '' }} onSubmit={() => {}} />)
+        const { form, heard } = formHolding('title')
+        expect(heard).toEqual([true])
+
+        await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Open details' })) })
+        expect(screen.getByLabelText('Detail')).toBeTruthy()
+        expect(heard[heard.length - 1]).toBe(false)
+
+        await act(async () => { fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) })
+        expect(screen.queryByRole('dialog')).toBeNull()
+        expect(form.getState().valid).toBe(true)
+        expect(heard[heard.length - 1]).toBe(true)
+    })
+
     it('tells them so when the row with the empty required field is removed', () => {
         jest.useFakeTimers()
         try {

src/core/modules/form/__tests__/batchedForm.test.js

@@ -134,6 +134,25 @@ describe('createBatchedForm', () => {
         expect(values[values.length - 1]).toEqual({})
     })
 
+    it('registers a probe without its validator while validation runs, so that it validates nothing', () => {
+        const { form } = setUp({ a: '', b: '' })
+        const validated = { count: 0 }
+        const counted = () => value => {
+            validated.count += 1
+            return value ? undefined : 'Required'
+        }
+        form.registerField('a', () => undefined, { error: true }, { getValidator: counted })
+        validated.count = 0
+        const heard = []
+        // react-final-form's probe in a field's first render: registered silently, and unregistered at once.
+        form.registerField('b', state => { heard.push(state) }, { error: true, valid: true }, { silent: true, getValidator: counted })()
+        expect(validated.count).toBe(0)
+        expect(heard).toEqual([expect.objectContaining({ error: undefined, valid: true })])
+        // The field's own registration validates, and finds the error.
+        form.registerField('b', () => undefined, { error: true }, { getValidator: counted })
+        expect(form.getFieldState('b').error).toBe('Required')
+    })
+
     it('leaves a registration that asks to be silent as it is, and its unregistering', () => {
         const { form } = setUp({ a: 1 })
         const batches = { count: 0 }

src/core/modules/form/batchedForm.ts

@@ -43,8 +43,9 @@ export function registrationsOf (form: unknown): Registrations | undefined {
  * one, the form's listeners are told when it was the last field of its name. While validation is paused, that is
  * held until it resumes, and done once for all the fields that went.
  *
- * A registration that asks to be silent itself, as react-final-form's probe in a field's first render does, is
- * left as it is. So is every registration made while validation runs.
+ * A registration that asks to be silent itself, as react-final-form's probe in a field's first render does, keeps
+ * its `silent`, and while validation runs, it registers without its validator (see below). Every other registration
+ * made while validation runs is left as it is.
  */
 export function createBatchedForm<FormValues = Record<string, any>> (config: Config<FormValues>): FormApi<FormValues> {
   const form = createForm<FormValues>(config)
@@ -75,7 +76,21 @@ export function createBatchedForm<FormValues = Record<string, any>> (config: Con
     setConfig(name, value)
   }
   form.registerField = (name, subscriber, subscription, fieldConfig) => {
-    if (!form.isValidationPaused() || fieldConfig?.silent) return registerField(name, subscriber, subscription, fieldConfig)
+    if (fieldConfig?.silent) {
+      // react-final-form's probe in a field's first render, registered and unregistered at once for the state the
+      // field first renders with. With a validator and validation running, final-form validated every field for
+      // it: a burst of n fields mounting beside m others ran about n·m validators before any bracket opened, since
+      // the probe runs in render. Without one, the field first renders with no error yet, as every field does on a
+      // document's first render, where react-final-form has validation paused. Its error comes when its node's
+      // bracket closes. That bracket matters: react-final-form takes the first notification after the field's
+      // registration from its effect as the state the probe gave it, so that registration must not validate.
+      // Inside a bracket it registers silently, and the error comes in a later notification. Without the
+      // brackets, a field the user touched came back from a tab switch without its error, as the tab-switch test
+      // of `UIRender.form-flows.test.js` shows.
+      if (form.isValidationPaused()) return registerField(name, subscriber, subscription, fieldConfig)
+      return registerField(name, subscriber, subscription, {...fieldConfig, getValidator: undefined})
+    }
+    if (!form.isValidationPaused()) return registerField(name, subscriber, subscription, fieldConfig)
     unheard = true
     const unregister = registerField(name, subscriber, subscription, {...fieldConfig, silent: true})
     return () => {

src/demo/markdowns/changelog.md

@@ -866,6 +866,12 @@ Nothing to do for these:
 
 #### Fixes
 
+- **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
+  went one by one when it closed, each validating every field left, since nothing mounts then whose node
+  brackets them: the dialog brackets them now, and closing a popup that holds such a table takes 27 ms
+  instead of 447. On React 16 a popup's fields still go one by one.
 - **A number shown with decimals is formatted by a formatter made once.** `renderFloat`, which the
   `Currency`, `Float`, `Double5` and `Percent` renderers and `Counter` show numbers through, formatted
   each fraction with `toLocaleString` and its options: a new `Intl.NumberFormat` for every value, 9.65 µs

src/library/__tests__/later-mount-budget.test.js

@@ -13,9 +13,13 @@
  *  - a tab switch to the table computed 21008 field states and ran 5252 validators, and one away from it 20200 and
  *    5050;
  *  - a `showIf` that showed it computed 21524 and ran 5252, and one that hid it 21322 and 5151.
- * Now a switch to it computes 12 states per field, its own two as it registers and the rest in the passes when the
- * bracket closes, and validates each field twice; a switch away has no field left to tell. Hiding it validates each
- * field once, from the `form.change` that hides it: a field changed with no `validateFields` validates them all.
+ * Now a switch to it computes 11 states per field, and validates each field once, when the bracket closes; a switch
+ * away has no field left to tell. Hiding it validates each field once, from the `form.change` that hides it: a field
+ * changed with no `validateFields` validates them all.
+ *
+ * Until 2026-10-14, react-final-form's probe in each field's first render validated every field registered then,
+ * 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.
  *
  * 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
@@ -87,7 +91,7 @@ describe('fields that mount after the first render: a 50-row table of two fields
         const fields = formHolding('rows').getRegisteredFields()
         const away = await costOf(() => fireEvent.click(screen.getByRole('tab', { name: 'A' })))
         expect(fields).toHaveLength(2 * ROWS + 1)
-        expect(to).toEqual({ computed: 12 * fields.length, validated: 2 * fields.length })
+        expect(to).toEqual({ computed: 11 * fields.length, validated: fields.length })
         expect(away).toEqual(REMOVALS_BRACKETED ? { computed: 0, validated: 0 } : { computed: 20200, validated: 5050 })
         expect(screen.queryByRole('textbox', { name: /Note/ })).toBeNull()
     })
@@ -100,8 +104,45 @@ describe('fields that mount after the first render: a 50-row table of two fields
         const fields = form.getRegisteredFields()
         const hidden = await costOf(() => form.change('show', 'no'))
         expect(fields).toHaveLength(2 * ROWS + 2)
-        expect(shown).toEqual({ computed: 1431, validated: 2 * (fields.length - 1) })
+        expect(shown).toEqual({ computed: 1330, validated: fields.length - 1 })
         expect(hidden).toEqual(REMOVALS_BRACKETED ? { computed: 822, validated: fields.length - 1 } : { computed: 21322, validated: 5151 })
         expect(form.getRegisteredFields()).toEqual(['show'])
     })
+
+    it('costs a `showIf` that shows it beside 100 other required fields a pass over all of them, not one per new field', async () => {
+        const others = Array.from({ length: 100 }, (_, i) => ({ view: 'Input', name: `other${i}`, validate: 'required' }))
+        const meta = { view: 'Col', items: [{ view: 'Input', name: 'show', label: 'Show' }, ...others, { ...table, showIf: { name: 'show', equal: 'yes' } }] }
+        const values = { ...data(), ...Object.fromEntries(others.map(({ name }) => [name, 'x'])) }
+        render(<UIRender meta={meta} data={values} initialValues={values} onSubmit={() => {}} />)
+        const form = formHolding('show')
+        const shown = await costOf(() => form.change('show', 'yes'))
+        expect(form.getRegisteredFields()).toHaveLength(2 * ROWS + 102)
+        // One pass over the fields already there for the change, and one over them all when the bracket closes.
+        expect(shown).toEqual({ computed: 3230, validated: 301 })
+    })
+
+    it('costs a popup that opens it beside 100 other required fields, and closes again, a pass over them each', async () => {
+        // Nothing mounts when a popup closes, whose node would bracket its fields going: the dialog brackets them.
+        const others = Array.from({ length: 100 }, (_, i) => ({ view: 'Input', name: `other${i}`, validate: 'required' }))
+        const meta = {
+            view: 'Col',
+            items: [
+                { view: 'Button', children: 'Open details', onClick: { name: 'popupOpen', args: ['details'] } },
+                ...others,
+                { view: 'Popup', id: 'details', title: 'Details', items: [table] },
+            ],
+        }
+        const values = { ...data(), ...Object.fromEntries(others.map(({ name }) => [name, 'x'])) }
+        render(<UIRender meta={meta} data={values} initialValues={values} onSubmit={() => {}} />)
+        const form = formHolding('other0')
+        const opened = await costOf(() => fireEvent.click(screen.getByRole('button', { name: 'Open details' })))
+        expect(form.getRegisteredFields()).toHaveLength(2 * ROWS + 101)
+        const closed = await costOf(() => fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }))
+        expect(screen.queryByRole('dialog')).toBeNull()
+        expect(form.getRegisteredFields()).toHaveLength(100)
+        expect(opened).toEqual({ computed: 2212, validated: 201 })
+        // Once its fields have gone, every field left is validated and told once. Before, each field that went
+        // 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 })
+    })
 })