Repository navigation
fix(react): one Ctrl+Z pops one undo, however many useGlobalUndo instances are mounted, and a focused field keeps its own undo (objectui#11081) - #11104
Conversation
…ances are mounted, and a focused field keeps its own undo (objectui#11081) Every mounted useGlobalUndo added its own window keydown listener and each popped the shared globalUndoManager, so one Ctrl+Z undid one operation per mounted instance. The listener also ignored the event target, so Ctrl+Z in a text field was preventDefault-ed and reverted a saved write instead. Instances now register in a module-level set; one listener, added on the first registration and removed on the last, pops at most one operation per keypress and answers it through the longest-mounted live instance. A keypress whose target is an input, textarea, select or contenteditable region is left to the field. The hook's signature and its returned undo/redo are unchanged. Claude-Session: https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec Co-authored-by: Claude <noreply@anthropic.com>
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract reviewServed-tier: Inputs: card #11081 (body, triage ① Derived judgments
② Semver level
③ Boundary flags
Deviations, each answered:
Declared narrowing — closed by the gate verdicts: Out-of-scope findings (carrier 承接者:无, noted in the PR's acceptance notes, not filed), each answered:
Implemented-by: VERDICT: PASS Generated by Claude Code |
2 participants
Fixes #11081
Clause-②: no
What was wrong
Every mounted
useGlobalUndoadded its ownwindowkeydownlistener, and each one popped the one sharedglobalUndoManager. The console mounts several instances at once, so one Ctrl+Z reverted one saved record write per mounted instance, as many as were stacked. The handler also never read the event target: Ctrl+Z typed into a text field waspreventDefaulted, the field's own undo did not run, and the last saved record write was reverted instead.What changes (
packages/react/src/hooks/useGlobalUndo.tsonly)Set. The first registration adds the one module-levelkeydownhandler; the last unregistration removes it. The registration effect runs once per mount and is keyed on the ref, whose identity React keeps, not on a memoisedundo/redo(AGENTS.md rule 10), so an instance's place in the keyboard order is its mount order.popUndo, orpopRedowith Shift), and exactly one instance answers it: itsdataSourceruns the write and itsonUndo/onRedofires.input,textarea,selector contenteditable region (isContentEditable, which also covers a node nested inside one), the handler returns beforepreventDefaultand before any pop, so the browser's own undo runs in the field.UseGlobalUndoOptions, the return shape, and the returnedundo/redo(what a toast's Undo button calls), which still run through the instance they belong to. The hook's emitted declaration was compared at base and head: the only difference is the added JSDoc paragraph.Which instance answers a keypress: the longest-mounted live one
Measured at
origin/main3e71a9825: anUndoableOperationcarriesid,type,objectName,recordId,timestamp,description,undoDataandredoData, and nothing that names the instance whose runner pushed it. The push happens in@object-ui/core'sActionRunner(globalUndoManager.push(result.undo)), outside the hook module. An owner field would change an exported core type and the runner, which the claim puts out of scope. So the rule is the dispatch's fallback: the first entry of the set, which is the longest-mounted live instance. When it unmounts, the next-oldest takes over.That is also the instance that already answered whenever a single operation was stacked. Its listener was first on
windowand popped the operation, and the rest found the stack empty; the card's probe measured exactly that. So a single-operation Ctrl+Z behaves as before, and only the extra pops are gone. The choice is written in the hook's JSDoc and in the block comment above the registry.Pins:
useGlobalUndo.oneListener-11081.test.tsxReal hook, real
globalUndoManager, realkeydownevents that bubble towindow.dataSource.delete, oneonUndo,defaultPreventedtrue).input,textarea,select, contenteditable region, and a node nested inside one (five cases): the stack is untouched,defaultPreventedis false, and no callback runs. A focusedbuttonstill pops (the control).keydownlistener. Unmounting all of them removes that same function, and a later keypress pops nothing and is notdefaultPrevented.keydownadds and removes balance after unmount, and nothing pops afterwards.undo()on an instance that does not own the keyboard runs that instance'sdataSourceandonUndo, not the keyboard owner's.Evidence (all at head
f89bd44d4)pnpm exec vitest run --maxWorkers=2 packages/react/plus the three tests that drive a real consumer's undo (useConsoleActionRuntime.undoneToast-11056.test.tsx,RecordDetailView.undoneToast-11056.test.tsx,undoProjectedRow-10404.test.tsx):Test Files 107 passed (107),Tests 1377 passed (1377), that is 104 react files plus 3.3e71a9825blob (on-disk proof: blob equal to the base blob,onUndoShortcutcount 3 to 0), then the new and existing pins ran:Tests 10 failed | 6 passed (16). Red: the three one-pop pins, the five editable-target pins and the two lifecycle pins. Green at base, as expected: the button control, the toast-styleundo()pin and the four existing option-timing pins. Restored withgit checkout HEAD: blob equal to HEAD,git diff HEADempty.ablation-replace.mjs(anchorif (isEditableTarget(e.target)) return;count 1 to 0, blob moved): exactly the five editable-target pins went red,Tests 5 failed | 7 passed (12). Restored: blob equal to HEAD.pnpm --filter @object-ui/react type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0, after building the dependency closure withpnpm --filter '@object-ui/react^...' build. The test tsconfig's--listFilesincludes the new pin file.check-changeset-presence,check-control-bytes,check:new-line-citations(0 new),check-changeset-no-major,check:changeset-claims,check:pending-changeset-literals,check:test-path-roots,check:vi-mock-specifiers,check:unreferenced-sources,check:self-importandcheck:phantom-depsall exit 0.check-governed-queue-guard --teston the three paths answers NOT GOVERNED.Declared narrowing.
pnpm lintand the fullpnpm testrun are CI's. ESLint ran on the two changed source files only. The population is those two files: ESLint linted both and ignored neither. The count, 2 files with 0 errors and 0 warnings, is read from--format json.eslint.config.jsdeclares no type-aware linting (noprojectService,parserOptionsorTypeCheckedconfig), so this diff cannot move the verdict on any untouched file. The app-shellAppContent.*tests that stubuseGlobalUndowere not run, because they replace the hook and cannot see this change.Not measured: a browser run of the console. Which instance is the oldest on a given route is read from source, not measured.
Acceptance notes
dataSource. An instance without one pops without reverting. That behavior is unchanged: it is what the first-registered listener did before. In the console, all three call sites pass the same adapter (read from source).UndoableOperationwould need an owner and theActionRunnerpush would have to set it. That is a core contract change, and it is not proposed here.Changeset:
.changeset/11081-one-undo-listener.md,'@object-ui/react': patch.Generated by Claude Code