fix(form): a defaultValues change no longer discards the field being filled (#2982)

.changeset/form-defaults-reset-keeps-user-input.md

@@ -0,0 +1,47 @@
+---
+"@object-ui/components": patch
+---
+
+fix(form): a `defaultValues` change no longer discards the field the user is filling
+
+The form renderer adopts a changed `defaultValues` with `form.reset()`, which
+replaces the **whole** react-hook-form record — so it also blanks the fields the
+incoming defaults say nothing about. And it runs in a **passive effect**, one
+commit after those fields have been committed and painted, so input landing in
+that window was silently dropped.
+
+The caught case is the wizard (objectui#2982). It reuses ONE inner form across
+steps and feeds it `defaultValues={formData}` — the merge of the steps submitted
+**so far** — so at every step boundary the incoming defaults are missing exactly
+the fields now on screen:
+
+```
+RESET to {"name":"Alice"}   (values before: {"name":"Alice","note":"hello"})
+-> create POST {"name":"Alice"}   — the last step is gone
+```
+
+In a browser this needs a busy main thread plus typing on the first frame after
+the new defaults arrive — unlikely by hand, but paste and autofill land in a
+single tick. The same shape had already bitten once before, as a `reset()` on
+`defaultValues` **identity** churn wiping input mid-interaction; comparing by
+value fixed that, and this is the residual hole where the value genuinely did
+change.
+
+The reset now carries such a value across instead of dropping it. Deliberately
+narrow: only a field the **caller has never carried** — absent from both the
+outgoing and the incoming defaults — and whose value the user actually changed
+is eligible. Wherever the caller has an opinion it stays authoritative, so the
+load-bearing paths are unchanged:
+
+- an edit-mode record landing after first paint still fills every field it names
+  (a field the user has NOT touched is empty-ish against the baseline, using the
+  same comparison the dirty check uses, so a widget normalizing its own empty
+  value on mount is not mistaken for input);
+- a `recordId` swap still replaces the record outright — drawer/modal/split
+  forms re-fetch without re-entering their loading branch, so record B lands in
+  the still-mounted form and must not inherit an abandoned edit to record A;
+- a field the caller withdraws from its defaults stops being the user's.
+
+A reset that carries input now also reports the form as dirty (it is, against
+the caller's defaults) instead of unconditionally announcing pristine, so a
+host's discard guard keeps hearing the truth.

packages/components/src/renderers/form/__tests__/form-defaults-change-keeps-user-input.test.tsx

@@ -0,0 +1,129 @@
+/**
+ * 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.
+ */
+
+/**
+ * A `defaultValues` change must not throw away what the user typed (#2982).
+ *
+ * The renderer adopts a changed `defaultValues` with `form.reset()`, which
+ * replaces the WHOLE react-hook-form record. Any field the incoming defaults do
+ * not mention is therefore blanked — including one the user just filled in. The
+ * caught case was the wizard: it reuses ONE inner form across steps and feeds it
+ * `defaultValues={formData}`, the merged data of the steps SUBMITTED SO FAR. So
+ * on every step boundary the incoming defaults are missing exactly the fields of
+ * the step now on screen, and the create POST went out without them:
+ *
+ *     RESET to {"name":"Alice"}   (values before: {"name":"Alice","note":"hello"})
+ *     -> create {"name":"Alice"}  — the last step is gone
+ *
+ * (The reset runs in a passive effect, one commit after the new step's inputs
+ * are painted, so a real user can type inside that window too — and paste or
+ * autofill lands there in a single tick.)
+ *
+ * The rule pinned here: the caller's defaults stay authoritative for every field
+ * the caller has an opinion about — a landing record, a swapped record, a
+ * dropped field all overwrite freely, exactly as before. Only a value the caller
+ * has NEVER carried (absent from both the outgoing and the incoming defaults)
+ * and that the user actually changed survives the reset.
+ */
+import { describe, it, expect, beforeAll, vi } from 'vitest';
+import { render, fireEvent, waitFor } from '@testing-library/react';
+import { ComponentRegistry } from '@object-ui/core';
+
+beforeAll(async () => {
+  await import('../../../renderers');
+}, 30000);
+
+const fields = [
+  { name: 'name', label: 'Name', type: 'input' },
+  { name: 'note', label: 'Note', type: 'input' },
+];
+
+function formElement(
+  defaults: Record<string, unknown>,
+  onSubmit?: (data: Record<string, unknown>) => void,
+) {
+  const Form = ComponentRegistry.get('form')!;
+  return (
+    <Form
+      schema={{
+        type: 'form',
+        fields,
+        defaultValues: defaults,
+        showSubmit: false,
+        showCancel: false,
+        onSubmit,
+      }}
+    />
+  );
+}
+
+const input = (c: HTMLElement, name: string) =>
+  c.querySelector(`input[name="${name}"]`) as HTMLInputElement;
+
+describe('form renderer — a defaultValues change keeps user input (#2982)', () => {
+  it('keeps a field the incoming defaults never mention, and submits it', async () => {
+    const onSubmit = vi.fn();
+    const { rerender, container } = render(formElement({}, onSubmit));
+
+    // The user fills the field that is on screen now. The caller does not model
+    // it yet — for the wizard, `note` belongs to the step being filled.
+    fireEvent.change(input(container, 'note'), { target: { value: 'hello' } });
+
+    // The caller's defaults change to what it HAS collected (step 1's `name`).
+    rerender(formElement({ name: 'Alice' }, onSubmit));
+
+    expect(input(container, 'note').value).toBe('hello');
+    expect(input(container, 'name').value).toBe('Alice');
+
+    // The harm was on the wire, not just on screen: the create body lost a step.
+    fireEvent.submit(container.querySelector('form') as HTMLFormElement);
+    await waitFor(() => expect(onSubmit).toHaveBeenCalled());
+    expect(onSubmit.mock.calls[0][0]).toEqual(
+      expect.objectContaining({ name: 'Alice', note: 'hello' }),
+    );
+  });
+
+  it('still adopts a record that lands after first paint (edit mode)', () => {
+    // The load-bearing case the reset exists for: an edit form paints before
+    // `findOne` resolves, and every field must take the record's values.
+    const { rerender, container } = render(formElement({}));
+
+    rerender(formElement({ name: 'Alice', note: 'from the record' }));
+
+    expect(input(container, 'name').value).toBe('Alice');
+    expect(input(container, 'note').value).toBe('from the record');
+  });
+
+  it('does not leak an unsaved edit into a different record swapped in place', () => {
+    // Split/drawer/modal forms re-fetch on a `recordId` change WITHOUT going
+    // back through their loading branch, so record B lands in the still-mounted
+    // form. B's values must win outright — carrying A's abandoned edit across
+    // would silently write it to the wrong record.
+    const { rerender, container } = render(formElement({ name: 'A', note: 'note A' }));
+
+    fireEvent.change(input(container, 'note'), { target: { value: 'unsaved edit' } });
+    rerender(formElement({ name: 'B', note: 'note B' }));
+
+    expect(input(container, 'name').value).toBe('B');
+    expect(input(container, 'note').value).toBe('note B');
+  });
+
+  it('still lets the caller take back a field it drops from its defaults', () => {
+    // The caller previously carried `note` and no longer does: that is the
+    // caller withdrawing the value, so the user's edit does not survive it.
+    // (react-hook-form reverts a dropped key to the default it last had, rather
+    // than blanking it — the point asserted here is only that the caller, not
+    // the abandoned edit, decides.)
+    const { rerender, container } = render(formElement({ name: 'Alice', note: 'note A' }));
+
+    fireEvent.change(input(container, 'note'), { target: { value: 'unsaved edit' } });
+    rerender(formElement({ name: 'Alice' }));
+
+    expect(input(container, 'note').value).toBe('note A');
+  });
+});

packages/components/src/renderers/form/form.tsx

@@ -119,6 +119,10 @@ const valuesEqualForDirty = (a: unknown, b: unknown): boolean => {
   }
 };
 
+/** Own-property test — a field can legitimately be named `constructor`. */
+const hasOwn = (o: Record<string, unknown>, k: string): boolean =>
+  Object.prototype.hasOwnProperty.call(o ?? {}, k);
+
 const computeDirty = (
   baseline: Record<string, unknown>,
   values: Record<string, unknown>,
@@ -561,13 +565,52 @@ ComponentRegistry.register('form',
       try { key = JSON.stringify(defaultValues ?? {}); } catch { key = String(Date.now()); }
       if (lastDefaultsKey.current === key) return;
       lastDefaultsKey.current = key;
+      const incoming = (defaultValues ?? {}) as Record<string, unknown>;
+      const outgoing = baselineRef.current;
+      // `reset()` replaces the WHOLE record, so it also blanks fields the
+      // incoming defaults say nothing about — and it lands here in a PASSIVE
+      // effect, one commit after those fields were painted. Input arriving in
+      // that window was silently discarded. The caught case is the wizard: it
+      // reuses one inner form across steps and feeds back `formData`, the merge
+      // of the steps submitted SO FAR — so at every step boundary the incoming
+      // defaults are missing exactly the fields now on screen, and the create
+      // POST went out without the step just filled (#2982):
+      //
+      //   RESET to {"name":"Alice"}  (values before: {"name":"Alice","note":"hello"})
+      //
+      // Carry such a value across the reset instead of dropping it.
+      const carried = Object.entries(form.getValues() as Record<string, unknown>).filter(
+        ([name, value]) =>
+          // Only a field the CALLER HAS NEVER CARRIED is eligible — absent from
+          // both the outgoing and the incoming defaults. Everywhere the caller
+          // has an opinion it stays authoritative, so the three load-bearing
+          // paths keep today's behavior exactly: a record landing after first
+          // paint still fills every field it names; a `recordId` swap still
+          // replaces the record outright (drawer/modal/split forms re-fetch
+          // without re-entering their loading branch, so record B lands in the
+          // still-mounted form and must not inherit an abandoned edit to A);
+          // and a field the caller withdraws stops being the user's.
+          !hasOwn(incoming, name) &&
+          !hasOwn(outgoing, name) &&
+          // The same empty-ish comparison the dirty check uses, so a widget
+          // normalizing its own empty value on mount ('' -> null, undefined ->
+          // '') is not mistaken for input and does not keep a field alive.
+          !valuesEqualForDirty(outgoing[name], value),
+      );
+      // Set the baseline before resetting: `reset`/`setValue` notify the watcher
+      // below synchronously, and it reads this to compute dirtiness.
+      baselineRef.current = incoming;
       form.reset(defaultValues);
-      baselineRef.current = (defaultValues ?? {}) as Record<string, unknown>;
+      for (const [name, value] of carried) {
+        form.setValue(name, value, { shouldValidate: false, shouldDirty: true });
+      }
       // Fresh values, so last attempt's rejected-field markers no longer apply.
       setRejectedFieldNames([]);
-      // A fresh reset is by definition pristine — clear any stale dirty signal
-      // (e.g. an edit-mode record that just finished loading).
-      onDirtyChangeProp?.(false);
+      // A reset that carried nothing is by definition pristine — clear any stale
+      // dirty signal (e.g. an edit-mode record that just finished loading). One
+      // that carried input is genuinely dirty against the caller's defaults, and
+      // the host's discard guard has to keep hearing so.
+      onDirtyChangeProp?.(carried.length > 0);
       // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [defaultValues]);