fix(plugin-form): swapping recordId no longer leaves the previous record on screen

.changeset/record-swap-hides-the-previous-record.md

@@ -0,0 +1,38 @@
+---
+"@object-ui/plugin-form": patch
+---
+
+fix(plugin-form): swapping `recordId` no longer leaves the previous record on screen
+
+`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 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.

packages/plugin-form/src/DrawerForm.tsx

@@ -216,8 +216,24 @@ export const DrawerForm: React.FC<DrawerFormProps> = ({
     fetchSchema();
   }, [schema.objectName, dataSource]);
 
+  // The record whose data `formData` currently holds. The fetch effect reads it
+  // to tell a genuine record SWAP from a re-run of its own making —
+  // `initialData`/`initialValues` are objects callers commonly rebuild every
+  // render, and flashing the loading state for those would thrash.
+  const loadedRecordIdRef = useRef<string | number | undefined>(undefined);
+
   // Fetch initial data
   useEffect(() => {
+    // A `recordId` change re-enters this effect with the form still MOUNTED on
+    // the previous record, which needs handling on two fronts (pinned by
+    // recordSwapLoading.test.tsx):
+    //  - go back to the loading state, so record A's values are not left on
+    //    screen AND EDITABLE while B is in flight, to be swapped underneath in
+    //    place when it lands. Anything typed there read as A's on screen but
+    //    would have been submitted against B.
+    //  - ignore a response that is no longer the one being awaited, so two
+    //    overlapping reads land in REQUEST order, not completion order.
+    let cancelled = false;
     const fetchData = async () => {
       if (schema.mode === 'create' || !schema.recordId) {
         setFormData(schema.initialData || schema.initialValues || {});
@@ -231,19 +247,33 @@ export const DrawerForm: React.FC<DrawerFormProps> = ({
         return;
       }
 
+      // Only a change of RECORD hides the form. Hiding it UNMOUNTS the inner
+      // renderer, which is the only thing that reports dirtiness — it cannot
+      // emit a final `onDirtyChange(false)` on its way out, so clear the flag
+      // here or the close/unload guards stay armed for input that belongs to a
+      // record no longer on screen.
+      if (loadedRecordIdRef.current !== schema.recordId) {
+        setLoading(true);
+        setIsDirty(false);
+      }
+
       try {
         const data = await dataSource.findOne(schema.objectName, schema.recordId);
+        if (cancelled) return;
+        loadedRecordIdRef.current = schema.recordId;
         setFormData(data || {});
       } catch (err) {
+        if (cancelled) return;
         setError(err as Error);
       } finally {
-        setLoading(false);
+        if (!cancelled) setLoading(false);
       }
     };
 
     if (objectSchema || !dataSource) {
       fetchData();
     }
+    return () => { cancelled = true; };
   }, [objectSchema, schema.mode, schema.recordId, schema.initialData, schema.initialValues, dataSource, schema.objectName]);
 
   // Build form fields from section config

packages/plugin-form/src/ModalForm.tsx

@@ -290,8 +290,24 @@ export const ModalForm: React.FC<ModalFormProps> = ({
     fetchSchema();
   }, [schema.objectName, dataSource]);
 
+  // The record whose data `formData` currently holds. The fetch effect reads it
+  // to tell a genuine record SWAP from a re-run of its own making —
+  // `initialData`/`initialValues` are objects callers commonly rebuild every
+  // render, and flashing the loading state for those would thrash.
+  const loadedRecordIdRef = useRef<string | number | undefined>(undefined);
+
   // Fetch initial data
   useEffect(() => {
+    // A `recordId` change re-enters this effect with the form still MOUNTED on
+    // the previous record, which needs handling on two fronts (pinned by
+    // recordSwapLoading.test.tsx):
+    //  - go back to the loading state, so record A's values are not left on
+    //    screen AND EDITABLE while B is in flight, to be swapped underneath in
+    //    place when it lands. Anything typed there read as A's on screen but
+    //    would have been submitted against B.
+    //  - ignore a response that is no longer the one being awaited, so two
+    //    overlapping reads land in REQUEST order, not completion order.
+    let cancelled = false;
     const fetchData = async () => {
       if (schema.mode === 'create' || !schema.recordId) {
         setFormData(schema.initialData || schema.initialValues || {});
@@ -305,19 +321,33 @@ export const ModalForm: React.FC<ModalFormProps> = ({
         return;
       }
 
+      // Only a change of RECORD hides the form. Hiding it UNMOUNTS the inner
+      // renderer, which is the only thing that reports dirtiness — it cannot
+      // emit a final `onDirtyChange(false)` on its way out, so clear the flag
+      // here or the close/unload guards stay armed for input that belongs to a
+      // record no longer on screen.
+      if (loadedRecordIdRef.current !== schema.recordId) {
+        setLoading(true);
+        setIsDirty(false);
+      }
+
       try {
         const data = await dataSource.findOne(schema.objectName, schema.recordId);
+        if (cancelled) return;
+        loadedRecordIdRef.current = schema.recordId;
         setFormData(data || {});
       } catch (err) {
+        if (cancelled) return;
         setError(err as Error);
       } finally {
-        setLoading(false);
+        if (!cancelled) setLoading(false);
       }
     };
 
     if (objectSchema || !dataSource) {
       fetchData();
     }
+    return () => { cancelled = true; };
   }, [objectSchema, schema.mode, schema.recordId, schema.initialData, schema.initialValues, dataSource, schema.objectName]);
 
   // Build form fields from section config

packages/plugin-form/src/SplitForm.tsx

@@ -14,7 +14,7 @@
  * Aligns with @objectstack/spec FormView type: 'split'
  */
 
-import React, { useState, useCallback, useEffect, useMemo } from 'react';
+import React, { useState, useCallback, useEffect, useMemo, useRef } from 'react';
 import type { FormField, DataSource } from '@object-ui/types';
 import {
   ResizablePanelGroup,
@@ -114,8 +114,24 @@ export const SplitForm: React.FC<SplitFormProps> = ({
     fetchSchema();
   }, [schema.objectName, dataSource]);
 
+  // The record whose data `formData` currently holds. The fetch effect reads it
+  // to tell a genuine record SWAP from a re-run of its own making —
+  // `initialData`/`initialValues` are objects callers commonly rebuild every
+  // render, and flashing the loading state for those would thrash.
+  const loadedRecordIdRef = useRef<string | number | undefined>(undefined);
+
   // Fetch initial data
   useEffect(() => {
+    // A `recordId` change re-enters this effect with the form still MOUNTED on
+    // the previous record, which needs handling on two fronts (pinned by
+    // recordSwapLoading.test.tsx):
+    //  - go back to the loading state, so record A's values are not left on
+    //    screen AND EDITABLE while B is in flight, to be swapped underneath in
+    //    place when it lands. Anything typed there read as A's on screen but
+    //    would have been submitted against B.
+    //  - ignore a response that is no longer the one being awaited, so two
+    //    overlapping reads land in REQUEST order, not completion order.
+    let cancelled = false;
     const fetchData = async () => {
       if (schema.mode === 'create' || !schema.recordId) {
         setFormData(schema.initialData || schema.initialValues || {});
@@ -129,19 +145,26 @@ export const SplitForm: React.FC<SplitFormProps> = ({
         return;
       }
 
+      // Only a change of RECORD hides the form.
+      if (loadedRecordIdRef.current !== schema.recordId) setLoading(true);
+
       try {
         const data = await dataSource.findOne(schema.objectName, schema.recordId);
+        if (cancelled) return;
+        loadedRecordIdRef.current = schema.recordId;
         setFormData(data || {});
       } catch (err) {
+        if (cancelled) return;
         setError(err as Error);
       } finally {
-        setLoading(false);
+        if (!cancelled) setLoading(false);
       }
     };
 
     if (objectSchema || !dataSource) {
       fetchData();
     }
+    return () => { cancelled = true; };
   }, [objectSchema, schema.mode, schema.recordId, schema.initialData, schema.initialValues, dataSource, schema.objectName]);
 
   // Build form fields from section config

packages/plugin-form/src/TabbedForm.tsx

@@ -13,7 +13,7 @@
  * Aligns with @objectstack/spec FormView type: 'tabbed'
  */
 
-import React, { useState, useCallback } from 'react';
+import React, { useState, useCallback, useRef } from 'react';
 import type { FormField, DataSource } from '@object-ui/types';
 import { cn } from '@object-ui/components';
 import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react';
@@ -210,28 +210,51 @@ export const TabbedForm: React.FC<TabbedFormProps> = ({
     fetchSchema();
   }, [schema.objectName, dataSource]);
 
+  // The record whose data `formData` currently holds. The fetch effect reads it
+  // to tell a genuine record SWAP from a re-run of its own making —
+  // `initialData`/`initialValues` are objects callers commonly rebuild every
+  // render, and flashing the loading state for those would thrash.
+  const loadedRecordIdRef = useRef<string | number | undefined>(undefined);
+
   // Fetch initial data for edit/view modes
   React.useEffect(() => {
+    // A `recordId` change re-enters this effect with the form still MOUNTED on
+    // the previous record, which needs handling on two fronts (pinned by
+    // recordSwapLoading.test.tsx):
+    //  - go back to the loading state, so record A's values are not left on
+    //    screen AND EDITABLE while B is in flight, to be swapped underneath in
+    //    place when it lands. Anything typed there read as A's on screen but
+    //    would have been submitted against B.
+    //  - ignore a response that is no longer the one being awaited, so two
+    //    overlapping reads land in REQUEST order, not completion order.
+    let cancelled = false;
     const fetchData = async () => {
       if (schema.mode === 'create' || !schema.recordId || !dataSource) {
         setFormData(schema.initialData || schema.initialValues || {});
         setLoading(false);
         return;
       }
-
+
+      // Only a change of RECORD hides the form.
+      if (loadedRecordIdRef.current !== schema.recordId) setLoading(true);
+
       try {
         const data = await dataSource.findOne(schema.objectName, schema.recordId);
+        if (cancelled) return;
+        loadedRecordIdRef.current = schema.recordId;
         setFormData(data || {});
       } catch (err) {
+        if (cancelled) return;
         setError(err as Error);
       } finally {
-        setLoading(false);
+        if (!cancelled) setLoading(false);
       }
     };
-    
+
     if (objectSchema || !dataSource) {
       fetchData();
     }
+    return () => { cancelled = true; };
   }, [objectSchema, schema.mode, schema.recordId, schema.initialData, schema.initialValues, dataSource, schema.objectName]);
 
   // Build form fields from section config