Skip to content

Commit 2dbc774

Browse files
fix(plugin-form): a master-detail header keystroke is no longer dropped while the page has other work pending (#28494)
domain:ui#5 · os-sales · session_01L6bhgUJUXVQpyUbdmoks8V <!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuL9peF3uJZUvBPnozyR5Esc)_ Fixes #28471 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:主从表单(showcase「New Project + Tasks」这类)表头里打的字不会再被吞掉;live 规格在点 Create 前先确认名字还在输入框里。 - 为什么改:表单宿主上抓表头值给明细行的原生监听比 React 先拿到按键,它的状态更新是同步优先级,浏览器在两个监听器之间把它连同页面别的待处理更新一起渲染,表头输入框被写回旧值,按键就丢了,Create 报「Project Name is required」、不发请求。CI 两次红就是这个,后端没问题。 - 风险与回滚:只改一处,明细行接表头值的那次更新改成 transition(晚一拍渲染,表头不受影响);回滚即还原本 PR,无数据迁移。 - 先看:`ui/packages/plugin-form/src/MasterDetailForm.tsx`(`MasterDetailLines` 里的 `startTransition`)、`ui/packages/plugin-form/src/MasterDetailForm.headerKeystrokePendingWork-28471.test.tsx`、`ui/e2e/live/master-detail.spec.ts`。 - 两分钟验证:`pnpm --dir ui exec vitest run --root . packages/plugin-form/src/MasterDetailForm.headerKeystrokePendingWork-28471.test.tsx`,预期 1 个用例绿(表头打的 `paid` 留在输入框里,明细行的 Qty 随之锁定)。 - 需要你做的:无,席位落地。 ## What changes **Product fix (`@object-ui/plugin-form`).** `MasterDetailLines` reads the header's edits by listening to `input` / `change` on the form host and scraping the named controls, so the line grids' `parent` record follows the header. The host is inside the React root, so its native listener runs before React's root listener has passed the keystroke to the header's react-hook-form. Its `setEditedRecord` was priced by `window.event` (`input`), i.e. sync priority. The browser runs a microtask checkpoint after each listener, React flushes its sync work there, and React 19.3 renders pending default-priority lanes in the same pass (`getHighestPriorityLanes`: `lanes & 42` = Sync | InputContinuous | Default). When the page had such work pending (a fetch that had just answered), that render reached the header `ObjectForm` (not memoized), and the commit wrote the input's old controlled value `""` back through React's value tracker. React's change plugin then saw no change and never called `onChange`: the keystroke was dropped, the input went blank, and the Create click was refused by the required-field check without a request. The update is now `startTransition(() => setEditedRecord(next))`; a transition is not flushed in that checkpoint, so React reads the keystroke first. The lines still update right after. **Spec.** `master-detail.spec.ts` checks `input[name="name"]` still holds the typed name before it clicks Create, in both Create tests. With the defect back, the failure now names the dropped value at that line instead of a 15 s `waitForResponse` timeout. No wait, timeout or retry changed. **Changeset.** `.changeset/28471-master-detail-header-keystroke.md`, `@object-ui/plugin-form` patch. Branch: the harness-assigned `claude/project-thread-zka32v`, not `claude/issue-28471-*`; the card is #28471. ## Tests - `ui/packages/plugin-form/src/MasterDetailForm.headerKeystrokePendingWork-28471.test.tsx` (new, 1): a page re-render is scheduled outside `act` (a real default-priority update), then a keystroke is delivered the way a browser does. happy-dom leaves out two browser behaviours the defect needs, so the test supplies them: `window.event` is defined for the dispatch, and a bubble listener on the element right under the React root container calls `flushSync()` where the browser's checkpoint falls. Asserts the header input still holds the value and the `readonlyWhen: parent.status == 'paid'` cell locks. - `ui/e2e/live/master-detail.spec.ts`: the two `toHaveValue(name)` checks above. ## Acceptance notes - Root cause class: (a) in the card's terms, a click on a form that was not ready to submit, and the reason it was not ready is a product defect: a real user typing into a master-detail header while the page settles loses the keystroke the same way. Not (b): the click provoked no request at all. Not (c): no request reached the backend. - The failure-artifact zips of runs 41222427173 / 41239207302 could not be downloaded from the cloud container (the artifact host answers 403 to the proxy CONNECT), so the cause was reproduced locally with the CI build and backend instead. - Out of scope, not touched: `apps/console/src/pages/auth/LoginPage.tsx` also adds a native `input` listener (document, capture phase); it was not examined for the same pattern. ## Verification Head `409e49ef`, base `768ec4bf` (current `main` at branch time). Rig: the CI `Live E2E UI` setup on private ports (`ui/e2e/live/ci/start-backend.sh` on the showcase example, console built with `VITE_BASE_PATH=/ vite build`, `vite preview` with `DEV_PROXY_TARGET`), Chromium from `/opt/pw-browsers`, `--retries 0`, 1 worker. <details><summary>How the cause was found</summary> - CPU throttled with CDP `Emulation.setCPUThrottlingRate`. In every failing run the name input was empty at the click with "Project Name is required" shown, account and status intact, the button enabled, no batch request. - An init script wrapped every `input` listener: the value was still typed after the window, body and root capture listeners and after the host's scrape listener, and was `""` after the microtask that followed the scrape listener, before React's root bubble listener ran. - A hook on the input's value setter, matched by element identity, caught the write: `updateInput` / `commitHostUpdate` / `commitRoot` / `performSyncWorkOnRoot` / `processRootScheduleInMicrotask`. - A `__REACT_DEVTOOLS_GLOBAL_HOOK__.onCommitFiberRoot` probe showed that commit re-rendering from above `RouteElement` down through `MasterDetailForm` and `ObjectForm` to the input (page work batched into the sync flush). Passing runs had the same scrape listener but no page work pending at that instant. </details> <details><summary>Live e2e readings</summary> Old and new console builds made the CI way, differing only in `MasterDetailForm.tsx` (the built `plugin-form` chunk checked for the transition). Throttled runs used an untracked copy of the spec that only adds the CDP throttle and a 30 s heading wait in `beforeEach` (removed before commit). | Build | Spec | CPU | Create runs | Red | How | |---|---|---|---|---|---| | old | old | 4x | 12 | 0 | | | old | old | 6x | 16 | 5 | `page.waitForResponse: Timeout 15000ms`, the CI failure | | old | new | 6x | 16 | 6 | `toHaveValue` at the new check, received `""` | | new | new | 6x | 24 (36 tests) | 0 | | | new | new (committed file) | 1x | 10 (15 tests) | 0 | | CI command on the new build with a fresh backend: `pnpm --dir ui run test:e2e:live:ci --retries 0`, 16 passed, 0 failed. </details> <details><summary>Unit, type-check, lint, ablation</summary> - `@object-ui/plugin-form` `pnpm test`: 217 files, 2407 passed, 1 skipped, 0 failed. - `turbo run type-check --filter=@object-ui/plugin-form` (48 tasks, deps built): exit 0; this includes `tsc -p tsconfig.test.json`, which covers the new test. - ESLint on the changed files: new test 0 problems; `master-detail.spec.ts` 0 errors, 2 existing `no-explicit-any` warnings on untouched lines; `MasterDetailForm.tsx` 0 errors, 22 existing `no-explicit-any` warnings, none on the changed lines. - Ablation, `os-verify-lock.sh -- node scripts/ablation-replace.mjs` on `MasterDetailForm.tsx`, anchor `startTransition(() => setEditedRecord(next));` to `setEditedRecord(next);`: anchor 1 to 0, blob `851b4840` to `d19e610c`; the new test red, `expected '' to be 'paid'`; restored, blob equals HEAD, `git diff HEAD` empty. The live-e2e half of the ablation is the "old build, new spec" row above. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands` at `409e49ef` derived 34 commands; all 34 ran and exited 0 (including `pnpm --dir ui exec vitest run scripts/`, `check:published-files`, `check:type-check-debt`, `check:test-source-alias`, `check:nul-bytes` and the changeset gates). `dispatch-gates --ran`: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. Changed lines 185 (+183 / -2), under the 3000 threshold. </details> - Dogfood: not owed (no `!` in the title, no `(narrowing)` arm). --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgUJUXVQpyUbdmoks8V)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1d201b0 commit 2dbc774

4 files changed

Lines changed: 183 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/plugin-form': patch
3+
---
4+
5+
fix(plugin-form): a keystroke in a master-detail header is no longer dropped while the page has other work pending
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A master-detail form (`object-master-detail-form`, `MasterDetailForm`) could lose what the user typed into a header field. The form host listens to the header's `input` events to feed the line grids' `parent` record, and that listener runs before React has passed the keystroke to the header form. Its state update ran at sync priority, so the browser flushed it right there together with any default-priority work the page had pending, such as a fetch that had just answered. That render drew the header input again with its old value, React then saw no change, and the keystroke never reached the form. The field showed the text for a moment and went blank, and Create was refused with "is required" for a field the user had filled.
11+
12+
The line grids now take the header edit as a transition, which the browser does not flush at that point, so the header form always reads the keystroke first. The lines still follow the header: a `readonlyWhen: "parent.status == 'paid'"` cell locks as soon as the header says paid.

‎ui/e2e/live/master-detail.spec.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ import { selectOption, fillLookup } from './helpers';
2020
* server half, which a request-only assertion reads green when the batch is
2121
* refused, stores only the parent, or drops the child's parent reference.
2222
*
23+
* Each test also checks the header still holds the typed name before it
24+
* clicks Create (objectstack#28471). A keystroke the form dropped used to
25+
* surface 15 s later as "no batch request": the click was refused by the
26+
* required-field check on a name the user had typed. The check names that
27+
* half at the line where it broke.
28+
*
2329
* Auth: the page writes with the bearer token global-setup puts in the app
2430
* origin's localStorage; the read-back goes through the `request` fixture,
2531
* which carries the better-auth session cookie of the same storageState.
@@ -75,6 +81,7 @@ test('Create stores the populated parent through one atomic batch', async ({ pag
7581
await fillLookup(page, 'account', 'North');
7682
await selectOption(page, 'status', 'planned');
7783
await expect(page.getByText('Northwind', { exact: false })).toBeVisible();
84+
await expect(page.locator('input[name="name"]')).toHaveValue(name);
7885

7986
const response = await submitBatch(page);
8087

@@ -101,6 +108,7 @@ test('Create with a task line stores the child referencing the parent', async ({
101108
// the only one the select offers.
102109
await selectOption(page, 'status', 'planned');
103110
await expect(page.getByText('Northwind', { exact: false })).toBeVisible();
111+
await expect(page.locator('input[name="name"]')).toHaveValue(name);
104112

105113
// `showcase_task` is a rich child, so its relationship smart-default resolves
106114
// to the per-row `form` factor: "Add task" opens the child's full form
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* A keystroke in a master-detail header reaches the header's form even when
11+
* the page has other work pending at that instant (objectstack#28471).
12+
*
13+
* The form host's `input` listener (the scrape that feeds the line grids'
14+
* `parent`) runs BEFORE React's root listener, because the host sits inside
15+
* the React root. In a browser, the microtask checkpoint between those two
16+
* listeners flushed the scrape's update, which was sync priority, together
17+
* with the page's pending default-priority work. That render repainted the
18+
* header input with its old controlled value, React's value tracker then saw
19+
* no change, and the keystroke never reached react-hook-form: the live
20+
* showcase page's Create failed "Project Name is required" with no request.
21+
*
22+
* happy-dom runs every listener of a dispatch inside one JS call, with no
23+
* microtask checkpoint between them, so this file supplies the checkpoint: a
24+
* listener on the element between the host and the React root calls
25+
* `flushSync()`, which flushes exactly the work React's microtask would. The
26+
* pending work is a page re-render scheduled just before the keystroke.
27+
*/
28+
import { describe, it, expect, vi, afterEach } from 'vitest';
29+
import { render, screen, waitFor, act } from '@testing-library/react';
30+
import React from 'react';
31+
import { flushSync } from 'react-dom';
32+
import { registerAllFields } from '@object-ui/fields';
33+
import { MasterDetailForm } from './MasterDetailForm';
34+
35+
registerAllFields();
36+
37+
type Props = React.ComponentProps<typeof MasterDetailForm>;
38+
39+
const INV_SCHEMA = { name: 'inv', fields: { status: { type: 'text', label: 'Status' } } };
40+
41+
const SCHEMA = {
42+
objectName: 'inv',
43+
mode: 'create',
44+
fields: ['status'],
45+
details: [
46+
{
47+
childObject: 'inv_line',
48+
relationshipField: 'inv',
49+
columns: [
50+
{ name: 'product', label: 'Product', type: 'text' },
51+
{ name: 'qty', label: 'Qty', type: 'number', readonlyWhen: "parent.status == 'paid'" },
52+
],
53+
},
54+
],
55+
} as unknown as Props['schema'];
56+
57+
function dataSource() {
58+
return {
59+
getObjectSchema: vi.fn().mockResolvedValue(INV_SCHEMA),
60+
find: vi.fn().mockResolvedValue({ data: [] }),
61+
create: vi.fn(),
62+
update: vi.fn(),
63+
delete: vi.fn(),
64+
bulk: vi.fn(),
65+
} as unknown as Props['dataSource'];
66+
}
67+
68+
/** The page around the form; `bump.current()` schedules a re-render of it. */
69+
const bump = { current: () => {} };
70+
function Page({ ds }: { ds: Props['dataSource'] }) {
71+
const [n, setN] = React.useState(0);
72+
React.useEffect(() => {
73+
bump.current = () => setN((x) => x + 1);
74+
}, []);
75+
return <MasterDetailForm schema={{ ...SCHEMA, className: `page-${n}` }} dataSource={ds} />;
76+
}
77+
78+
/**
79+
* Type `value` into `input` the way a browser delivers a keystroke, with the
80+
* two pieces of browser behaviour happy-dom leaves out:
81+
*
82+
* - `window.event` is the event while it is dispatched. React reads it to
83+
* price an update scheduled from a non-React listener; without it every
84+
* such update is default priority and nothing is flushed mid-dispatch.
85+
* - A microtask checkpoint runs after each listener, where React flushes its
86+
* sync work. `checkpointAt` gets a bubble listener that does that flush,
87+
* so it falls after the listeners below it and before those above it.
88+
*
89+
* The native value changes first, past React's value tracker (which sees
90+
* only writes through its own setter), then one bubbling `input` event.
91+
*/
92+
function typeLikeABrowser(input: HTMLInputElement, value: string, checkpointAt: HTMLElement) {
93+
const before = Object.getOwnPropertyDescriptor(window, 'event');
94+
const checkpoint = () => flushSync(() => {});
95+
const ev = new Event('input', { bubbles: true });
96+
Object.defineProperty(window, 'event', { configurable: true, get: () => ev });
97+
checkpointAt.addEventListener('input', checkpoint);
98+
try {
99+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(input, value);
100+
input.dispatchEvent(ev);
101+
} finally {
102+
checkpointAt.removeEventListener('input', checkpoint);
103+
if (before) Object.defineProperty(window, 'event', before);
104+
else delete (window as { event?: unknown }).event;
105+
}
106+
}
107+
108+
const actEnv = globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean };
109+
const actEnvBefore = actEnv.IS_REACT_ACT_ENVIRONMENT;
110+
afterEach(() => {
111+
actEnv.IS_REACT_ACT_ENVIRONMENT = actEnvBefore;
112+
});
113+
114+
describe('MasterDetailForm — a header keystroke is never dropped (objectstack#28471)', () => {
115+
it('keeps the typed value when the page has default-priority work pending at the keystroke', async () => {
116+
const { container } = render(<Page ds={dataSource()} />);
117+
const status = await waitFor(() => {
118+
const el = container.querySelector('input[name="status"]') as HTMLInputElement | null;
119+
if (!el) throw new Error('header not ready');
120+
return el;
121+
});
122+
const qty = () => screen.getAllByLabelText('Qty')[0] as HTMLInputElement;
123+
await waitFor(() => expect(qty().disabled).toBe(false));
124+
125+
// The element right under the React root container: above the form host,
126+
// below React's root listener — where the browser's checkpoint fell.
127+
let checkpointAt: HTMLElement = status;
128+
while (checkpointAt.parentElement && checkpointAt.parentElement !== container) {
129+
checkpointAt = checkpointAt.parentElement;
130+
}
131+
132+
// The page's pending work is a real default-priority update, the kind a
133+
// landing fetch schedules, so it is scheduled outside `act`.
134+
actEnv.IS_REACT_ACT_ENVIRONMENT = false;
135+
bump.current();
136+
typeLikeABrowser(status, 'paid', checkpointAt);
137+
actEnv.IS_REACT_ACT_ENVIRONMENT = actEnvBefore;
138+
139+
// The keystroke reached the form: the input still shows it once the
140+
// pending work and the lines' update have rendered…
141+
await act(async () => {
142+
await new Promise((r) => setTimeout(r, 50));
143+
});
144+
expect(status.value).toBe('paid');
145+
// …and the lines read the same header the form holds.
146+
await waitFor(() => expect(qty().disabled).toBe(true));
147+
});
148+
});

‎ui/packages/plugin-form/src/MasterDetailForm.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
* `lookup`) FK on the child object.
3131
*/
3232

33-
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
33+
import React, { startTransition, useCallback, useEffect, useMemo, useRef, useState } from 'react';
3434
import type { BatchTransactionOperation, DataSource, GridFieldMetadata, I18nLabel } from '@object-ui/types';
3535
import { resolveAffordance, runBatchTransaction } from '@object-ui/core';
3636
import { LineItemsField, lookupAutofillByField, useBooleanValueLabel, type BooleanValueLabel, type GridColumn, type LookupAutofillByField } from '@object-ui/fields';
@@ -924,7 +924,20 @@ const MasterDetailLines: React.FC<MasterDetailLinesProps> = ({
924924
try { key = JSON.stringify(next); } catch { key = String(Math.random()); }
925925
if (key === editedKeyRef.current) return; // value-identical → no re-render
926926
editedKeyRef.current = key;
927-
setEditedRecord(next);
927+
// A TRANSITION, never a plain update (objectstack#28471). This listener
928+
// sits on the host, INSIDE the React root, so a keystroke reaches it
929+
// before React's own root listener has handed the value to the
930+
// header's react-hook-form. A plain update here is sync priority (the
931+
// event is `input`), and the browser's microtask checkpoint between
932+
// two listeners flushes it right there, together with whatever
933+
// default-priority work the page has pending (a fetch that just
934+
// landed): that render repaints the header input with its old
935+
// controlled value, React's value tracker then sees no change, and the
936+
// keystroke is dropped without a word — measured on the showcase
937+
// master-detail page, where a Create then failed "Project Name is
938+
// required" with the name visibly typed. A transition is not flushed
939+
// in that checkpoint, so React reads the keystroke first.
940+
startTransition(() => setEditedRecord(next));
928941
};
929942
host.addEventListener('input', onEvt);
930943
host.addEventListener('change', onEvt);

0 commit comments

Comments
 (0)