diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e67835f5..4dc5d54e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -382,7 +382,7 @@ jobs: retention-days: 7 # What the published package's documents cost in Chromium: the main thread's busy time for a mount and for each - # action of four documents a host could render, a 200-row period table among them (e2e/perf/scenarios.perf.js, + # action of five documents a host could render, a 200-row period table among them (e2e/perf/scenarios.perf.js, # `npm run test:perf`). ADVISORY: a runner's speed varies from run to run, so this reports and never blocks. Its # ceilings sit about three times above what the code costs on a developer's machine, and at least 100 ms for an # action: they flag a cascade the size of the one the period table had before #209, not a small one, which the diff --git a/CLAUDE.md b/CLAUDE.md index b21d4246..385b34fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,9 +30,9 @@ The modernization roadmap (React 17/18 upgrade, `semantic-ui-react` exit, projec - `npm run test:pack` — `build-lib`, then the packaging budgets (`test:pack:budget`, `scripts/check-package-budget.js`) and a server-render smoke of the packed tarball (`test:pack:consumer`). `npm run test:pack:peers` repeats the smoke on React 16.14, 17.0.2 and 19.3.0, from the same fixtures as the jest legs. `npm run test:pack:browser` (after `build-lib`) loads the packed tarball in Chromium: a throwaway host (`scripts/packed-workspace.js`, shared with the server smoke) bundles it with its own React beside the published `static/all.css`, and `e2e/packed/tarball.packed.js` (`playwright.packed.config.js`) checks that it renders, responds and styles only its own wrapper; CI runs it in the `browser` job. `npm run test:pack:browser:peers` runs the same page on React 16.14, 17 and 19 from the jest legs' fixtures, mounting through `ReactDOM.render` on 16 and 17 as 0.34.x hosts do; CI runs it there too - `npm run test:perf` (after `build-lib`) — what the packed tarball's documents cost in Chromium. - **What it measures:** the main thread's busy time (the DevTools protocol's `TaskDuration`) for a mount, and the - median of five runs of each action (an edit, a blur, a pick), in four documents of - `scripts/fixtures/packed-perf-entry.js`. The documents include a 200-row period table and a 200-row table with a - Select column. + median of five runs of each action (an edit, a blur, a pick, a tab switch), in five documents of + `scripts/fixtures/packed-perf-entry.js`. The documents include a 200-row period table, a 200-row table with a + Select column, and two tabs whose second holds a 200-row table. - **Where it runs:** in the same throwaway host as `test:pack:browser`, under `playwright.perf.config.js`, with `e2e/perf/scenarios.perf.js`. It prints the medians and writes them to `test-results-perf/perf-results.json`. - **Comparing versions:** `--tarball=` measures a tarball packed elsewhere, such as another commit's, so two @@ -121,11 +121,22 @@ A new wrapper, mapping or copy made for a field's prop must be kept the same way `src/library/__tests__/*-budget.test.js` contracts count the renders that a mount, an edit and a blur cost. A number that falls is progress; one that rises is a regression. -A mount has a cost besides renders: every field registers with final-form. The form a document renders comes from -`createBatchedForm` (`modules/form/batchedForm.ts`), which `
` is handed as `form`. While react-final-form holds -validation back for the first render, a field registers silently, and every field hears once when validation resumes. -Until 2026-10-13 each registration published the state of every field before it, so a document of n fields computed -about n²/2 field states as it mounted. `form-mount-budget.test.js` counts them. +A mount has a cost besides renders: every field registers with final-form. Until 2026-10-13, each registration +published the state of every field before it and, once validation ran, validated them all, and each unregistration +did the same. A burst of n fields cost about n²/2 of each, on the first render, a tab switch, a `showIf` that flips +or a popup. + +The form a document renders now comes from `createBatchedForm` (`modules/form/batchedForm.ts`), which `` is +handed as `form`. While validation is paused, a field registers silently, and every field hears once when validation +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. + +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. 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. diff --git a/docs/UPGRADE-PLAN.md b/docs/UPGRADE-PLAN.md index 6ae9829e..30e9b478 100644 --- a/docs/UPGRADE-PLAN.md +++ b/docs/UPGRADE-PLAN.md @@ -2289,7 +2289,8 @@ Postponed 2026-09-30 (§9.7-F4), and no longer a React 19 prerequisite. **Postpo - **Also to check on the bump, since 2026-10-13:** `createBatchedForm` (`modules/form/batchedForm.ts`) leans on final-form's registration. A `silent` registration publishes only its own field's state, its unregistering is silent too, and `pauseValidation()` holds back the form's listeners. final-form 5.0.1's source keeps all three, read that day; run - `batchedForm.test.js`, `batchedForm.document.test.js` and `form-mount-budget.test.js` on the bump. Checked the same + `batchedForm.test.js`, `batchedForm.document.test.js`, `form-mount-budget.test.js` and `later-mount-budget.test.js` on the + bump. Checked the same day: npm's `latest` for `react-final-form` is still 7.0.1, and #1096 is still open. ### TS migration (§9.6) diff --git a/e2e/perf/scenarios.perf.js b/e2e/perf/scenarios.perf.js index eb755bd2..25c8fe6c 100644 --- a/e2e/perf/scenarios.perf.js +++ b/e2e/perf/scenarios.perf.js @@ -7,17 +7,21 @@ * thread's busy time, the DevTools protocol's `TaskDuration`, over the mount and over each action, an action being * what a user does and everything it sets off until the page is idle again (the 300 ms typing debounce included). * Each action runs five times, and the median is what counts. After its runs, an edit or a pick checks that its field - * shows the last run's value, so that an action that stops changing anything fails instead of measuring nothing. Each - * test writes its document's numbers to PERF_OUT before it checks them, so that a document over its ceilings still - * reports, and so do the ones after it. scripts/test-packed-perf.js runs it (`npm run test:perf`) and prints them; CI - * runs it in the advisory `browser-perf` job, which reports and never blocks. + * shows the last run's value, so that an action that stops changing anything fails instead of measuring nothing. A tab + * switch waits for the table's field to be there, and a switch back for it to be gone. Each test writes its + * document's numbers to PERF_OUT before it checks them, so that a document over its ceilings still reports, and so do + * the ones after it. scripts/test-packed-perf.js runs it (`npm run test:perf`) and prints them; CI runs it in the + * advisory `browser-perf` job, which reports and never blocks. * - * Each ceiling below is about three times what the current code costs on an Apple M4, and at least 100 ms for an - * action, so that a slower runner passes and so does an action's own noise. That flags a cascade the size of the one - * #209 to #213 removed from the period table: an edit there cost 374 ms before them and 67 ms after, and a first blur - * 187 ms and 41 ms. It does not flag a small one. Elsewhere in these documents the same changes moved an action's cost - * by less than 15 ms, and the jsdom contracts, which count renders, guard those. Lower a ceiling when a number falls - * for good. + * Each ceiling below is two and a half to three times what GitHub's runner measured for the code it was set with, and + * at least 100 ms for an action, so that a slower run passes and so does an action's own noise. The runner took about + * three times as long as an Apple M4 to mount a document (2026-10-13), and the M4 measured one build about twice as + * long one day as the next, so the ceilings follow the runner's numbers, which the `browser-perf` job prints. The tab + * switch's were set from the M4's at that ratio, before the runner had measured them. + * + * A ceiling flags a cascade the size of the one #209 to #213 removed from the period table, whose edit cost five + * times what it costs now. It does not flag a smaller one: the jsdom contracts, which count renders and field states, + * guard those. Lower a ceiling when a number falls for good. */ const fs = require('fs') const { test, expect } = require('@playwright/test') @@ -29,9 +33,10 @@ const results = {} /** Milliseconds of main-thread work, per action, that no current scenario should come near. */ const CEILINGS = { periods: { mount: 2000, edit: 210, blur: 130 }, - selectTable: { mount: 1400, edit: 100, blur: 100 }, + selectTable: { mount: 820, edit: 100, blur: 100 }, selectForm: { mount: 340, pick: 100, edit: 100 }, - numberTable: { mount: 1100, edit: 100 }, + numberTable: { mount: 780, edit: 100 }, + tabs: { mount: 330, switch: 720, back: 360 }, } const median = values => [...values].sort((a, b) => a - b)[Math.floor(values.length / 2)] @@ -135,6 +140,32 @@ test('a form of 100 named Selects: a pick, and an edit of its text field', async check('selectForm') }) +test('two tabs, the second a 200-row table: a switch to it, and back', async ({ page }) => { + const { mount, measure } = await open(page, 'tabs') + results.tabs = { mount: { median: mount, runs: [mount] } } + const rows = page.getByRole('tab', { name: 'Rows', exact: true }) + const summary = page.getByRole('tab', { name: 'Summary', exact: true }) + const cell = page.locator('input[name="rows[1].note"]') + // A switch waits for the table's field to be there, and a switch back for it to be gone. + const switches = [] + const backs = [] + for (let i = 0; i < REPEAT; i++) { + switches.push(await measure(async () => { + await rows.click() + await cell.waitFor() + })) + backs.push(await measure(async () => { + await summary.click() + await cell.waitFor({ state: 'detached' }) + })) + } + results.tabs.switch = { median: median(switches), runs: switches } + results.tabs.back = { median: median(backs), runs: backs } + await rows.click() + await expect(cell).toHaveValue('note 1') + check('tabs') +}) + test('a 200-row table of bounded number fields: an edit of a number', async ({ page }) => { const { mount, measure } = await open(page, 'numberTable') results.numberTable = { mount: { median: mount, runs: [mount] } } diff --git a/scripts/fixtures/packed-perf-entry.js b/scripts/fixtures/packed-perf-entry.js index 3bfdb51a..9a99d047 100644 --- a/scripts/fixtures/packed-perf-entry.js +++ b/scripts/fixtures/packed-perf-entry.js @@ -57,6 +57,28 @@ const SCENARIOS = { }, data: { other: 'text', ...Object.fromEntries(range(100).map(i => [`choice${i}`, 'a'])) }, }), + // Two tabs, the second a 200-row table of a required text column and a bounded number column, which mounts and + // unmounts with its tab. + tabs: () => ({ + meta: { + view: 'Tabs', + items: [ + { tab: 'Summary', content: { view: 'Text', children: 'Nothing to edit here' } }, + { + tab: 'Rows', + content: { + view: 'Table', + name: 'rows', + headers: [ + { id: 'note', label: 'Note', renderCell: { view: 'Input', name: 'note', validate: 'required' } }, + { id: 'qty', label: 'Qty', renderCell: { view: 'Input', type: 'number', name: 'qty', min: 0, max: 100000 } }, + ], + }, + }, + ], + }, + data: { rows: range(ROWS).map(i => ({ note: `note ${i}`, qty: i })) }, + }), // A column of number fields with bounds, beside a text column. numberTable: () => ({ meta: { diff --git a/src/core/engine/mapper.tsx b/src/core/engine/mapper.tsx index 5688cf08..2beaae4c 100644 --- a/src/core/engine/mapper.tsx +++ b/src/core/engine/mapper.tsx @@ -1,4 +1,5 @@ -import React, { memo, useContext } from 'react' +import React, { memo, useContext, useEffect, useLayoutEffect } from 'react' +import { registrationsOf } from '../modules/form/batchedForm' import AutoSave from '../modules/form/views/AutoSave' import { FIELD } from '../modules/variables' import { cn } from '../components' @@ -38,6 +39,10 @@ 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 @@ -236,7 +241,15 @@ const RenderComponent = ({ // components that read `Active.translate` as a prop default). const translate = (instance && instance.translate) || Active.translate /* General showIf logic — the whole decision lives in `showIf.ts`, testable on its own. */ - if (!shouldRender({ showIf, data, _data, relativeData, relativePath, relativeIndex, instance })) return null + 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]) + if (!shown) return null switch (view) { case FIELD.TYPE.DATA: diff --git a/src/core/modules/form/__tests__/batchedForm.document.test.js b/src/core/modules/form/__tests__/batchedForm.document.test.js index a595aaf1..5d4a6b3a 100644 --- a/src/core/modules/form/__tests__/batchedForm.document.test.js +++ b/src/core/modules/form/__tests__/batchedForm.document.test.js @@ -1,7 +1,8 @@ /** - * A document's fields register silently while its first render mounts (`createBatchedForm`, 2026-10-13). A field - * that goes later must still tell the form's listeners what final-form tells them when a field that is not silent - * goes: that its validator went with it, and with it the field's error. + * A document's fields register silently while its first render mounts, and so do the fields that mount later + * between the brackets of an engine node (`createBatchedForm`, `registrationsOf`, 2026-10-13). The form's listeners + * must still hear what final-form would have told them: that a field that came is not valid, and that one that + * went took its validator, and with it its error. * * A tab switch is the case where nothing else does. It changes no value, so the field going is the only thing that * happens to the form, and a field unregistered silently would leave the form's listeners with a form that is not @@ -22,6 +23,50 @@ function formHolding (key) { } describe('a document whose fields registered silently as it mounted', () => { + it('tells the form\'s listeners it is not valid when a tab switch brings a required field that is empty', async () => { + const meta = { + view: 'Tabs', + items: [ + { tab: 'A', content: { view: 'Text', children: 'Nothing here' } }, + { tab: 'B', content: { view: 'Input', name: 'b', label: 'B field', validate: 'required' } }, + ], + } + render( {}} />) + const { form, heard } = formHolding('b') + expect(heard).toEqual([true]) + + fireEvent.click(screen.getByRole('tab', { name: 'B' })) + await act(async () => { await new Promise(resolve => setTimeout(resolve, 100)) }) + + expect(screen.getByLabelText('B field')).toBeTruthy() + expect(form.getState().valid).toBe(false) + expect(heard[heard.length - 1]).toBe(false) + }) + + it('tells them so when a `showIf` shows a required field that is empty, and hides it again', () => { + const meta = { + view: 'Col', + items: [ + { view: 'Input', name: 'show', label: 'Show' }, + { view: 'Input', name: 'a', label: 'A field', validate: 'required', showIf: { name: 'show', equal: 'yes' } }, + ], + } + render( {}} />) + const { form, heard } = formHolding('show') + expect(screen.queryByLabelText('A field')).toBeNull() + expect(heard).toEqual([true]) + + act(() => { form.change('show', 'yes') }) + expect(screen.getByLabelText('A field')).toBeTruthy() + expect(form.getState().valid).toBe(false) + expect(heard[heard.length - 1]).toBe(false) + + act(() => { form.change('show', 'no') }) + expect(screen.queryByLabelText('A field')).toBeNull() + expect(form.getState().valid).toBe(true) + expect(heard[heard.length - 1]).toBe(true) + }) + it('tells the form\'s listeners it is valid when a tab switch takes its only invalid field away', async () => { const meta = { view: 'Tabs', diff --git a/src/core/modules/form/__tests__/batchedForm.test.js b/src/core/modules/form/__tests__/batchedForm.test.js index 647cac1b..3fb1602c 100644 --- a/src/core/modules/form/__tests__/batchedForm.test.js +++ b/src/core/modules/form/__tests__/batchedForm.test.js @@ -7,7 +7,8 @@ * The cost the batching removes is that count, not the subscribers' calls: a subscriber is called only when its * state changed. */ -import { createBatchedForm } from '../batchedForm' +import { createForm } from 'final-form' +import { createBatchedForm, registrationsOf } from '../batchedForm' const SUBSCRIPTION = { value: true, error: true, valid: true, pristine: true } @@ -172,3 +173,136 @@ describe('createBatchedForm', () => { expect(batches.count).toBe(1) }) }) + +describe('registrationsOf', () => { + const required = () => value => (value ? undefined : 'Required') + + it('is the brackets of a batched form, and of a copy of one; nothing for any other form', () => { + const form = createBatchedForm({ onSubmit: () => undefined }) + expect(registrationsOf(form)).toEqual({ begin: expect.any(Function), end: expect.any(Function) }) + // What react-final-form hands its render prop: a copy of the instance, with a `reset` of its own. + expect(registrationsOf({ ...form, reset: () => undefined })).toBe(registrationsOf(form)) + expect(registrationsOf(createForm({ onSubmit: () => undefined }))).toBeUndefined() + expect(registrationsOf(undefined)).toBeUndefined() + }) + + it('registers what comes between its brackets silently, and tells every field once when the outermost closes', () => { + const { form, computed, fields, register } = setUp(VALUES) + const { begin, end } = registrationsOf(form) + begin() + begin() + expect(form.isValidationPaused()).toBe(true) + NAMES.forEach(name => register(name)) + end() + expect(form.isValidationPaused()).toBe(true) + end() + expect(form.isValidationPaused()).toBe(false) + // As in the first render's batch: one for each registration, then once more and twice in the form's. + expect(computed.count).toBe(4 * NAMES.length) + NAMES.forEach((name, i) => { + expect(fields[name].map(state => state.value)).toEqual([i]) + }) + }) + + it('validates what registered between its brackets once, when they close', () => { + const { form, register } = setUp(Object.fromEntries(NAMES.map(name => [name, '']))) + const validated = { count: 0 } + const counted = () => value => { + validated.count += 1 + return value ? undefined : 'Required' + } + const { begin, end } = registrationsOf(form) + begin() + NAMES.forEach(name => register(name, { getValidator: counted })) + expect(validated.count).toBe(0) + end() + expect(validated.count).toBe(NAMES.length) + expect(Object.keys(form.getState().errors)).toHaveLength(NAMES.length) + }) + + it('holds back what the fields that go between its brackets would tell, and tells it once when they close', () => { + const { form, register } = setUp({ a: '', b: 'x' }) + const valid = [] + form.subscribe(state => { valid.push(state.valid) }, { valid: true }) + const { begin, end } = registrationsOf(form) + begin() + const unregister = register('a', { getValidator: required }) + register('b') + end() + expect(valid).toEqual([true, false]) + // A tab switch: the old tab's field goes between the brackets of the new tab's content. + begin() + unregister() + expect(valid).toEqual([true, false]) + end() + expect(form.getState().valid).toBe(true) + expect(valid).toEqual([true, false, true]) + }) + + it('holds back the same while react-final-form has validation paused, until it resumes it', () => { + const { form, register } = setUp({ a: '' }) + const valid = [] + form.subscribe(state => { valid.push(state.valid) }, { valid: true }) + const unregister = register('a', { getValidator: required }) + expect(valid).toEqual([true, false]) + // As its effect's cleanup does before the fields' own, under StrictMode or on unmount. + form.pauseValidation() + unregister() + expect(valid).toEqual([true, false]) + form.resumeValidation() + expect(valid).toEqual([true, false, true]) + }) + + it('leaves validation that something else paused to it, and settles when that resumes it', () => { + const { form, fields, register } = setUp({ a: '' }) + form.pauseValidation() + const { begin, end } = registrationsOf(form) + begin() + register('a', { getValidator: required }) + end() + expect(form.isValidationPaused()).toBe(true) + expect(fields.a.map(state => state.error)).toEqual([undefined]) + form.resumeValidation() + expect(fields.a.map(state => state.error)).toEqual([undefined, 'Required']) + }) + + it('leaves the resuming to what paused validation first, when that resumes before the bracket closes', () => { + const { form, fields, register } = setUp({ a: '' }) + const { begin, end } = registrationsOf(form) + form.pauseValidation() + begin() + register('a', { getValidator: required }) + form.resumeValidation() + expect(fields.a.map(state => state.error)).toEqual([undefined, 'Required']) + end() + expect(fields.a.map(state => state.error)).toEqual([undefined, 'Required']) + expect(form.isValidationPaused()).toBe(false) + }) + + it('closes nothing it did not open', () => { + const { form } = setUp() + const { end } = registrationsOf(form) + end() + expect(form.isValidationPaused()).toBe(false) + form.pauseValidation() + end() + expect(form.isValidationPaused()).toBe(true) + }) + + it('validates what has registered before a submit that comes between its brackets', () => { + const submitted = [] + const form = createBatchedForm({ onSubmit: values => { submitted.push(values) }, initialValues: { a: '' } }) + const { begin, end } = registrationsOf(form) + begin() + form.registerField('a', () => undefined, { value: true }, { getValidator: required }) + form.submit() + expect(submitted).toEqual([]) + expect(form.getState().errors).toEqual({ a: 'Required' }) + expect(form.isValidationPaused()).toBe(true) + end() + expect(form.isValidationPaused()).toBe(false) + form.change('a', 'x') + form.submit() + expect(submitted).toEqual([{ a: 'x' }]) + }) +}) diff --git a/src/core/modules/form/batchedForm.ts b/src/core/modules/form/batchedForm.ts index 316c0901..266938b0 100644 --- a/src/core/modules/form/batchedForm.ts +++ b/src/core/modules/form/batchedForm.ts @@ -2,37 +2,73 @@ import { createForm } from 'final-form' import type { Config, FormApi } from 'final-form' /** - * The final-form instance a document's `` renders, whose first render's fields hear of each other once - * (2026-10-13). + * 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. + */ +export type Registrations = { begin: () => void, end: () => void } + +const REGISTRATIONS = Symbol('registrations') + +/** + * The brackets of a form `createBatchedForm` made, or of a copy of one, which react-final-form hands out on every + * render: a copy has the instance's own properties, this one among them. Undefined for any other form. + */ +export function registrationsOf (form: unknown): Registrations | undefined { + return form ? (form as {[REGISTRATIONS]?: Registrations})[REGISTRATIONS] : undefined +} + +/** + * The final-form instance a document's `` renders, whose fields hear of each other once per burst of + * registrations rather than once per field (2026-10-13). * - * react-final-form pauses validation when it creates a form, and resumes it in its first effect, by which time - * every field of the first render has registered. The pause holds back validation and the form's listeners, but - * not the fields': a `registerField` that is not `silent` publishes the state of every field registered before it. - * A document of n fields therefore published about n²/2 field states as it mounted. Measured in Chromium, a - * 200-row table of two fields per row spent about 100 ms of its 215 ms mount there: final-form's - * `notifyFieldListeners` and `subscriptionFilter`, reached from react-final-form's `register`. + * WHY. A `registerField` that is not `silent` publishes the state of every field registered before it, and, with + * a validator, validates every field. A burst of n fields therefore computed about n²/2 field states, and ran + * about n²/2 validators. Unregistering does the same in reverse. Measured in Chromium, a 200-row table of two + * fields per row spent about 100 ms of its 215 ms mount in final-form's `notifyFieldListeners` and + * `subscriptionFilter`, and a tab switch to that table took about 240 ms, and one away from it about as long. * - * While validation is paused, a field registers `silent`. final-form then publishes the field's own state to it, - * which react-final-form takes as the field's initial state, and tells nobody else. When validation resumes, - * `batch` publishes every field's state and the form's, once, so each listener ends with what it would have had. - * `resumeValidation` publishes them too when a validator was held back, so that pass finds nothing left to tell; - * when none was, it publishes nothing, and the fields would not hear what a later field's default value changed. + * HOW. While validation is paused, a field registers `silent`: final-form then publishes the field's own state to + * it, which react-final-form takes as the field's initial state, and tells nobody else. When validation resumes, + * every field is told once: `resumeValidation` validates and publishes when a validator was held back, and `batch` + * publishes when none was, so that an earlier field hears what a later field's default value did to it. Two + * things pause validation: + * - react-final-form, from the moment it creates the form to its first effect, which covers the first render; + * - the outermost bracket of `registrationsOf`, which covers what mounts later, a tab or a popup or a row, and + * what goes in the same commit. * - * final-form keeps a field's `silent` for its unregistering too, which would then tell nobody that the field and - * its validator are gone. So such a field unregisters as one that is not silent does: with a validator, every field - * is validated again and every listener told, as `setConfig('validate', …)` does; without one, the form's listeners - * are told when it was the last field of its name. + * UNREGISTERING. final-form keeps a field's `silent` for its unregistering too, which would then tell nobody that + * the field and its validator are gone. So such a field unregisters as one that is not silent does: with a + * validator, every field is validated again and every listener told, as `setConfig('validate', …)` does; without + * 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. */ export function createBatchedForm> (config: Config): FormApi { const form = createForm(config) - const {registerField, resumeValidation, setConfig} = form + const {registerField, resumeValidation, setConfig, submit} = form // The form's own validator, which `setConfig('validate', …)` keeps and validates every field with. let validate = config.validate // A field registered silently since validation was paused, which the others have not heard of yet. let unheard = false + // What the fields that went while validation was paused left untold: a validator, or the last field of a name. + let revalidate = false + let retell = false + // The brackets open, and whether the outermost one paused validation itself. + let depth = 0 + let pausedHere = false + + /** Tells every field and the form's listeners, once, what the fields that came and went left untold. */ + const settle = () => { + const again = revalidate + const tell = unheard || retell + unheard = revalidate = retell = false + if (again) setConfig('validate', validate) + else if (tell) form.batch(() => undefined) + } + form.setConfig = (name, value) => { // A cast, not a guard: `value` is the option `name` names, which the checker cannot narrow by `name`. if (name === 'validate') validate = value as Config['validate'] @@ -45,15 +81,43 @@ export function createBatchedForm> (config: Con return () => { const validated = !!fieldConfig?.getValidator?.() unregister() - if (validated) setConfig('validate', validate) - else if (form.getFieldState(name) === undefined) form.batch(() => undefined) + const gone = form.getFieldState(name) === undefined + if (form.isValidationPaused()) { + revalidate ||= validated + retell ||= gone + } else if (validated) setConfig('validate', validate) + else if (gone) form.batch(() => undefined) } } form.resumeValidation = () => { resumeValidation() - if (!unheard) return - unheard = false - form.batch(() => undefined) + settle() + } + form.submit = () => { + // A submit between a commit and its effects, inside a bracket, submits what has registered so far validated. + if (pausedHere) { + resumeValidation() + form.pauseValidation() + } + return submit() + } + + const registrations: Registrations = { + begin () { + depth += 1 + if (depth > 1 || form.isValidationPaused()) return + pausedHere = true + form.pauseValidation() + }, + end () { + if (depth === 0) return + depth -= 1 + // A pause something else made is resumed by it, through `resumeValidation` above, which settles. + if (depth > 0 || !pausedHere) return + pausedHere = false + form.resumeValidation() + }, } + Object.assign(form, {[REGISTRATIONS]: registrations}) return form } diff --git a/src/demo/markdowns/changelog.md b/src/demo/markdowns/changelog.md index 28bdca17..156c2884 100644 --- a/src/demo/markdowns/changelog.md +++ b/src/demo/markdowns/changelog.md @@ -866,6 +866,14 @@ Nothing to do for these: #### Fixes +- **Fields that mount after the first render no longer tell each other of themselves one by one.** A tab + switch, a `showIf` that flips, a popup that opens or a row that expands registered its fields one at a + time, and each one published the state of every field before it and, with a validator, validated them + all. Each field that went did the same. In Chromium, a switch to a tab holding a 200-row table of two + fields per row took 241 ms, and a switch away from it 232 ms. An engine node now brackets the fields + that mount with it, and those that go in the same commit, as a tab's old content does, or with its own + `showIf`, and the form tells every field once: 78 ms and 39 ms. Fields that go on their own, as a closed + popup's do, still go one by one, and so do all that go on React 16. - **A document's fields no longer tell each other of themselves one by one as it mounts.** While a document's first render registered its fields, each one published the state of every field registered before it, so the mount's cost grew with the square of its fields: about half of the mount of a 200-row diff --git a/src/library/__tests__/later-mount-budget.test.js b/src/library/__tests__/later-mount-budget.test.js new file mode 100644 index 00000000..b6d10dc5 --- /dev/null +++ b/src/library/__tests__/later-mount-budget.test.js @@ -0,0 +1,107 @@ +/** + * WHAT FIELDS THAT MOUNT AFTER THE FIRST RENDER COST THEIR FORM, pinned (2026-10-13). + * ============================================================================================= + * + * A tab switch, a `showIf` that flips, a popup, a row expanded: fields that mount after the document's first + * render registered one by one while validation ran. Each registration published every field's state and ran + * every field's validator, and each one that went did the same, so a burst of n fields cost about n²/2 of each. + * An engine node now brackets the registrations it causes (`RenderComponent`, `engine/mapper.tsx`), and the form + * tells every field once when the outermost bracket closes (`registrationsOf`, `modules/form/batchedForm.ts`). + * + * Counted through each field's `isEqual`, which final-form calls once for the field's own state and twice for + * each field in the form's, and through each field's validator. Before, with the same 101 fields: + * - 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. + * + * 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. + */ +import React from 'react' +import { act, fireEvent, render, screen } from '@testing-library/react' +import UIRender from '../main' +import { formsStorage } from '../../core/state/formRegistry' + +/** Every field state final-form computes, and every validator it runs. */ +const mockCount = { computed: 0, validated: 0 } +jest.mock('final-form', () => { + const actual = jest.requireActual('final-form') + const createForm = config => { + const form = actual.createForm(config) + const registerField = form.registerField + form.registerField = (name, subscriber, subscription, fieldConfig = {}) => registerField(name, subscriber, subscription, { + ...fieldConfig, + isEqual: (a, b) => { + mockCount.computed += 1 + return fieldConfig.isEqual ? fieldConfig.isEqual(a, b) : a === b + }, + getValidator: fieldConfig.getValidator && (() => { + const validator = fieldConfig.getValidator() + return validator && ((...args) => { + mockCount.validated += 1 + return validator(...args) + }) + }), + }) + return form + } + return { __esModule: true, ...actual, createForm } +}) + +const ROWS = 50 +// Whether the fields that go are bracketed too: not on React 16 (see above). +const REMOVALS_BRACKETED = Number(React.version.split('.')[0]) > 16 +const table = { + view: 'Table', + name: 'rows', + headers: [ + { id: 'note', label: 'Note', renderCell: { view: 'Input', name: 'note', validate: 'required' } }, + { id: 'qty', label: 'Qty', renderCell: { view: 'Input', type: 'number', name: 'qty', min: 0, max: 1000 } }, + ], +} +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) { + 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)) + }) + return { ...mockCount } +} + +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', () => { + it('costs a tab switch to it, and away from it, a few passes over its fields', async () => { + const meta = { view: 'Tabs', items: [{ tab: 'A', content: { view: 'Text', children: 'Nothing here' } }, { tab: 'B', content: table }] } + render( {}} />) + const to = await costOf(() => fireEvent.click(screen.getByRole('tab', { name: 'B' }))) + 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(away).toEqual(REMOVALS_BRACKETED ? { computed: 0, validated: 0 } : { computed: 20200, validated: 5050 }) + expect(screen.queryByRole('textbox', { name: /Note/ })).toBeNull() + }) + + it('costs a `showIf` that shows it, and hides it again, a few passes over its fields', async () => { + const meta = { view: 'Col', items: [{ view: 'Input', name: 'show', label: 'Show' }, { ...table, showIf: { name: 'show', equal: 'yes' } }] } + render( {}} />) + const form = formHolding('show') + const shown = await costOf(() => form.change('show', 'yes')) + 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(hidden).toEqual(REMOVALS_BRACKETED ? { computed: 822, validated: fields.length - 1 } : { computed: 21322, validated: 5151 }) + expect(form.getRegisteredFields()).toEqual(['show']) + }) +})