Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-form): block page unload while a modal/drawer form has unsaved input - #2998
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang added a commit
that referenced
this pull request
Jul 30, 2026
…ord on screen (#3005) `loading` in ModalForm / DrawerForm / TabbedForm / SplitForm was only ever set `true` once, by `useState(true)`, and thereafter only ever set `false`. A `recordId` change therefore re-entered the fetch effect WITHOUT going back through the loading branch: the form stayed mounted showing — and accepting edits to — record A's values, with nothing indicating a different record had been asked for, until B's response landed and replaced them in place. Anything typed in that window read as A's on screen and would have been submitted against B. The same effect had no staleness guard either, so two overlapping reads landed in COMPLETION order rather than request order: ask for B then C, and a slow B arriving last left the form showing B while the caller had asked for C. Both are the same defect from the user's side — the form displays a record nobody asked for — so both are fixed: - a change of record re-enters the loading state before the read, so the previous record is off screen while the next one is in flight. Gated on the record actually changing: the effect also re-runs on `initialData`/`initialValues` identity churn (callers rebuild those objects every render), and flashing the loading state for that would thrash; - the effect's cleanup marks its read stale, so a response that is no longer the one being awaited is dropped instead of overwriting a newer record. ObjectForm already re-entered loading before its fetch, which is why this only ever reproduced on the four sectioned variants. Also fixed, a consequence of the above: hiding the form unmounts the inner renderer, and that renderer is the only thing that reports dirtiness via `onDirtyChange` — it gets no chance to report `false` on the way out. Without clearing the flag, the overlay's unsaved-input guards (#2998) would stay armed for input belonging to a record no longer on screen: a plain refresh would prompt, and closing would offer to discard nothing. Verified by mutation — dropping only that line fails the two guard tests with `expected true to be false`. New `recordSwapLoading.test.tsx` drives each container through a real record swap with a `findOne` the test resolves by id, so both the stale-record and the out-of-order cases are deterministic rather than timing-dependent; all 11 fail against the previous code (8 of them on the two core assertions). Verified: `npx vitest run packages/plugin-form/ packages/components/ packages/app-shell/` — 315 files / 2627 tests pass; `@object-ui/plugin-form` type-check clean, lint 0 errors (the 2 unused-disable warnings are pre-existing, in EmbeddableForm/MasterDetailForm). Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…aved input The in-app discard guard (#2028) only intercepts Radix close paths — backdrop, Escape, the X. A browser refresh or tab close never reaches them: beforeunload fired with nobody calling preventDefault, so a dirty create/edit overlay silently lost everything the user had typed (the dialog itself is even restored afterwards via the ?recordId= URL param, but empty). Register a beforeunload listener while the overlay is open and dirty (and confirmOnDiscard hasn't been opted out), mirroring the existing StudioDesignSurface pillar guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The in-app discard guard (#2028) only intercepts Radix close paths — backdrop click, Escape, the X. A browser refresh or tab close never reaches them:
beforeunloadfired with nobody callingpreventDefault(), so a dirty create/edit overlay silently lost everything the user had typed. Worse, the overlay itself is restored afterwards via the?recordId=URL param — but empty, which reads as data loss with no warning at any point.Verified live: dispatching
beforeunloadon a dirty edit modal returneddefaultPrevented === false; a refresh dropped all entered values with zero confirmation.Fix
Register a
beforeunloadlistener while the overlay is open and dirty (andconfirmOnDiscardhasn't been opted out), in bothModalFormandDrawerForm— mirroring the existing StudioDesignSurface pillar guard (#2606). The listener is removed as soon as the form is pristine again, so normal navigation never prompts.Tests
6 new cases in
discardGuard.test.tsx(describe.eachover both overlays):defaultPrevented)confirmOnDiscard: false→ unload not blockedpnpm vitest run packages/plugin-form: 224 passed.type-check: clean.🤖 Generated with Claude Code