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 }