fix(plugin-form): DrawerForm holds the loading state until the first record read lands (#10190)

.changeset/10190-drawerform-first-load-window.md

@@ -0,0 +1,17 @@
+---
+'@object-ui/plugin-form': patch
+---
+
+fix(plugin-form): `DrawerForm` no longer paints an editable form before the record it edits has loaded
+
+Opening an edit drawer showed the form empty and editable while the record read
+was still in flight, and when the record arrived it replaced whatever had been
+typed there — the value was gone from the screen and from the save, with no
+warning. The drawer already refused this when switching from one record to
+another; it now refuses it on the first load too, keeping its loading state
+until the record read lands.
+
+`DrawerForm` builds its fields and reads its record in two effects that both run
+when the object schema arrives. The field-building effect used to end the
+loading state unconditionally; it now leaves that to the record read whenever
+one is outstanding.

packages/plugin-form/src/DrawerForm.tsx

@@ -352,9 +352,28 @@ export const DrawerForm: React.FC<DrawerFormProps> = ({
   useEffect(() => {
     if (!objectSchema && dataSource) return;
 
+    // Ending the loading state here is only this effect's call when no record
+    // read is outstanding — the FIRST load as much as a swap (objectui#10190).
+    // This effect and the fetch effect above both key on `objectSchema`, so the
+    // commit that publishes the schema runs BOTH, fetch first: it enters the
+    // loading state and fires `findOne`, and an unconditional
+    // `setLoading(false)` here then won, painting an empty, EDITABLE form while
+    // the read was in flight — whose landing replaced whatever had been typed.
+    // A load the fetch effect started is ended by the fetch effect. The
+    // condition mirrors that effect's own branches: create mode, no
+    // `recordId`, or no `dataSource` never read a record.
+    const recordReadOutstanding =
+      schema.mode !== 'create' &&
+      !!schema.recordId &&
+      !!dataSource &&
+      loadedRecordIdRef.current !== schema.recordId;
+    const endLoading = () => {
+      if (!recordReadOutstanding) setLoading(false);
+    };
+
     if (schema.sections?.length) {
       // Fields are built per-section in the render
-      setLoading(false);
+      endLoading();
       return;
     }
 
@@ -379,7 +398,7 @@ export const DrawerForm: React.FC<DrawerFormProps> = ({
         customFields: schema.customFields,
       }),
     );
-    setLoading(false);
+    endLoading();
   }, [objectSchema, schema.fields, schema.customFields, schema.sections, schema.readOnly, schema.mode, dataSource]);
 
   // Field-group fallback (object-designer metadata, objectui#4774): when the

packages/plugin-form/src/__tests__/drawerFirstLoadWindow-10190.test.tsx

@@ -0,0 +1,149 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * `DrawerForm` offers no editable form for a record whose FIRST read has not
+ * landed (objectui#10190).
+ *
+ * The drawer already refused this for a record SWAP (`recordSwapLoading.test.tsx`)
+ * but not for the first load: the fetch effect and the fields effect both key
+ * on `objectSchema`, so the commit that publishes the schema runs both — the
+ * fetch effect enters the loading state and fires `findOne`, and the fields
+ * effect, declared after it, used to end the loading state unconditionally.
+ * The form painted empty and editable while the read was in flight, and the
+ * record landing replaced whatever had been typed: the value was gone from the
+ * screen and from the submit, with no warning and no dirty flag.
+ *
+ * `findOne` is held open here by the test, so the window is deterministic
+ * rather than a race against the worker's load — which is how it surfaced, as
+ * an order-dependent red in `fieldSecurityPayload.test.tsx`.
+ *
+ * The last case is the CONTROL that separates the two readings the card
+ * named. A stale submit closure would lose a value typed AFTER the record
+ * landed too; it does not, so the loss is the landing record replacing the
+ * form's values, i.e. the window this file pins shut.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, fireEvent, waitFor, cleanup, act } from '@testing-library/react';
+import { registerAllFields } from '@object-ui/fields';
+import { DrawerForm } from '../DrawerForm';
+
+registerAllFields();
+afterEach(() => cleanup());
+
+const RECORD = { id: 'r1', title: 'Record One', note: 'kept' };
+
+/** A data source whose single `findOne` the test resolves by hand. */
+function heldRead() {
+  let resolveRead: ((v: unknown) => void) | undefined;
+  const update = vi.fn(async (_o: string, _id: string, d: Record<string, unknown>) => ({ id: 'r1', ...d }));
+  const ds = {
+    getObjectSchema: vi.fn().mockResolvedValue({
+      name: 'task',
+      fields: {
+        title: { type: 'text', label: 'Title' },
+        note: { type: 'text', label: 'Note' },
+      },
+    }),
+    find: vi.fn().mockResolvedValue({ data: [] }),
+    create: vi.fn(),
+    update,
+    findOne: vi.fn(() => new Promise((resolve) => { resolveRead = resolve; })),
+  } as any;
+  return {
+    ds,
+    update,
+    /** Resolve the in-flight read once it has actually been issued. */
+    async land() {
+      await waitFor(() => {
+        if (!resolveRead) throw new Error('findOne not issued yet');
+      });
+      await act(async () => {
+        resolveRead!(RECORD);
+      });
+    },
+  };
+}
+
+const SHAPES = [
+  ['flat fields', { fields: [{ name: 'title' }, { name: 'note' }] }],
+  ['sections', { sections: [{ name: 'basics', label: 'Basics', fields: ['title', 'note'] }] }],
+] as const;
+
+const schemaFor = (shape: Record<string, unknown>, mode: 'edit' | 'create' = 'edit') =>
+  ({
+    type: 'object-form',
+    formType: 'drawer',
+    objectName: 'task',
+    mode,
+    ...(mode === 'edit' ? { recordId: 'r1' } : {}),
+    open: true,
+    onOpenChange: vi.fn(),
+    ...shape,
+  }) as any;
+
+const titleInput = () => document.body.querySelector<HTMLInputElement>('input[name="title"]');
+
+/**
+ * Flush the commits that follow the object schema resolving: the one that
+ * publishes it (running both effects) and anything they schedule.
+ */
+async function settleWithReadHeld(ds: any) {
+  await waitFor(() => expect(ds.findOne).toHaveBeenCalledTimes(1));
+  await act(async () => {
+    await new Promise((r) => setTimeout(r, 20));
+  });
+}
+
+describe.each(SHAPES)('DrawerForm first load (%s) — objectui#10190', (_label, shape) => {
+  it('offers no editable input while the first record read is in flight', async () => {
+    const { ds, land } = heldRead();
+    render(<DrawerForm schema={schemaFor(shape)} dataSource={ds} />);
+    await settleWithReadHeld(ds);
+
+    const pending = titleInput();
+    // Either absent or not editable — never a live input for a record the form
+    // has not got.
+    expect(pending === null || pending.disabled || pending.readOnly).toBe(true);
+
+    await land();
+    await waitFor(() => expect(titleInput()?.value).toBe('Record One'));
+    expect(titleInput()!.disabled).toBe(false);
+  });
+
+  it('CONTROL — a value typed after the record lands is the value submitted', async () => {
+    const { ds, update, land } = heldRead();
+    render(<DrawerForm schema={schemaFor(shape)} dataSource={ds} />);
+    await land();
+    const input = await waitFor(() => {
+      const el = titleInput();
+      if (!el || el.value !== 'Record One') throw new Error('record not on screen');
+      return el;
+    });
+
+    fireEvent.change(input, { target: { value: 'typed' } });
+    const form = document.body.querySelector('form');
+    if (!form) throw new Error('no form element');
+    fireEvent.submit(form);
+
+    await waitFor(() => expect(update).toHaveBeenCalled());
+    expect(update.mock.calls[0][2]).toMatchObject({ title: 'typed', note: 'kept' });
+  });
+});
+
+describe('DrawerForm create mode — objectui#10190 boundary', () => {
+  it('reads no record and renders straight to an editable form', async () => {
+    const { ds } = heldRead();
+    render(<DrawerForm schema={schemaFor(SHAPES[0][1], 'create')} dataSource={ds} />);
+    await waitFor(() => expect(titleInput()).not.toBeNull());
+    expect(titleInput()!.disabled).toBe(false);
+    expect(ds.findOne).not.toHaveBeenCalled();
+  });
+});

packages/plugin-form/src/fieldSecurityPayload.test.tsx

@@ -149,11 +149,20 @@ const schemaFor = (formType?: 'modal' | 'drawer') => ({
   fields: FIELDS,
 }) as any;
 
-/** Type into the ONE field the caller is allowed to edit, then submit. */
+/**
+ * Type into the ONE field the caller is allowed to edit, then submit.
+ *
+ * Waits for the RECORD to be on screen, not merely for the input to exist
+ * (objectui#10190): a container that paints its form before `findOne` lands
+ * replaces the typed value when the record arrives, and the payload then
+ * carries `RECORD.actual_value` instead of the edit — an order-dependent red
+ * about load timing, not about field-level security.
+ */
 async function editAllowedFieldAndSubmit(root: HTMLElement, update: any) {
   const input = await waitFor(() => {
     const el = root.querySelector('input[name="actual_value"]') as HTMLInputElement | null;
     if (!el) throw new Error('actual_value not rendered');
+    if (el.value !== String(RECORD.actual_value)) throw new Error('record not on screen yet');
     return el;
   });
   fireEvent.change(input, { target: { value: '5000' } });