fix(components): apply new form defaultValues in the commit that renders them (#3001)

The form applies a `defaultValues` change by resetting react-hook-form to it.
While that ran in a PASSIVE effect there was a window: the render had already
committed, so the new inputs were mounted and interactive, but the form still
held the old record. Anything typed in that window was destroyed — the pending
`reset()` overwrote the whole record with `defaultValues`, dropping the field
the user had just filled, with no error and nothing in the payload.

It surfaced as the flaky wizard test fixed in #2983: a step transition changes
`defaultValues`, and `note` typed on the new step vanished from the create
body. That fix drained pending effects in the test, which AVOIDS the window;
this one CLOSES it. Measured with the pre-fix test pattern, 1500 replays under
CPU load: 6 failures before, 0 after.

Running the reset as a layout effect is only half of it. The `form_change`
subscription was silent across a reset by accident of ordering, not by design:
`onAction` is usually an inline arrow, so its identity changes every render and
the subscription effect re-runs each commit — and React runs every passive
DESTROY before any passive CREATE, so the watcher was unsubscribed before the
reset fired. Hoisting only the reset to the layout phase puts it AHEAD of that
cleanup, so the still-live previous subscription sees it and a record landing
looks like the user having edited every field it filled. That regressed #2968
deterministically (`changes=[{"category":"not-offered","status":"pending"}]`).
Both `form.watch` subscriptions therefore move to the same phase, restoring the
destroy-then-create order exactly.

The new test pins the window shut without depending on timing: React flushes
passive effects in tree order, so a probe sibling rendered before the form is
guaranteed to run inside the window and types from there. It fails on every run
against the previous code.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

packages/components/src/renderers/form/__tests__/form-defaults-reset-window.test.tsx

@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+
+/**
+ * New `defaultValues` must be applied in the SAME commit that renders them —
+ * never a commit later (#2982).
+ *
+ * The form applies a `defaultValues` change by resetting react-hook-form to it.
+ * While that ran in a PASSIVE effect there was a window: the render had already
+ * committed, so the inputs were mounted and interactive, but the form still
+ * held the old record. A value typed in that window was destroyed — the pending
+ * `reset()` overwrote the whole record with `defaultValues`, dropping the field
+ * the user had just filled, with no error and nothing in the payload.
+ *
+ * It surfaced as a flaky wizard test: a step transition changes `defaultValues`,
+ * and `note` typed on the new step vanished from the create body. In a browser
+ * it needs a busy main thread plus typing on the very first frame of the new
+ * step — rare, but silent data loss when it hits.
+ *
+ * The test below constructs that window deterministically, without relying on
+ * timing. React flushes passive effects in tree order, so a probe sibling
+ * rendered BEFORE the form is guaranteed to run inside the window: its own
+ * passive effect fires on the same commit that delivered the new defaults, but
+ * strictly before the form's. Typing from there is therefore exactly the
+ * hostile interleaving — a keystroke that beat the reset.
+ *
+ * With the reset in a layout effect it has already run by then and the typed
+ * value survives; revert it to `React.useEffect` and this test fails every run.
+ */
+import { describe, it, expect, beforeAll } from 'vitest';
+import { render, waitFor, fireEvent, act } from '@testing-library/react';
+import React from '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' },
+];
+
+type Record_ = Record<string, unknown>;
+const getFormRenderer = () =>
+  ComponentRegistry.get('form') as React.ComponentType<{ schema: Record_ }>;
+
+describe('form — the defaultValues reset leaves no window open', () => {
+  it('keeps a value typed in the same frame the new defaults commit', async () => {
+    const Form = getFormRenderer();
+    const submitted: Record_[] = [];
+    let setDefaults!: (v: Record_) => void;
+
+    // Fires on every commit, but only acts once armed. Rendered BEFORE <Form>,
+    // so on the commit that delivers the new defaults this runs first — inside
+    // the window, before the form's own effect.
+    let armed = false;
+    const TypeInsideTheWindow = () => {
+      React.useEffect(() => {
+        if (!armed) return;
+        armed = false;
+        const note = document.querySelector('input[name="note"]') as HTMLInputElement | null;
+        if (note) fireEvent.change(note, { target: { value: 'hello' } });
+      });
+      return null;
+    };
+
+    const Host = () => {
+      const [defaults, setD] = React.useState<Record<string, unknown>>({});
+      setDefaults = setD;
+      return (
+        <>
+          <TypeInsideTheWindow />
+          <Form
+            schema={{
+              type: 'form',
+              fields: FIELDS,
+              defaultValues: defaults,
+              showSubmit: false,
+              onSubmit: (d: Record_) => { submitted.push(d); },
+            }}
+          />
+        </>
+      );
+    };
+
+    const { container } = render(<Host />);
+    await waitFor(() => {
+      if (!container.querySelector('input[name="note"]')) throw new Error('not ready');
+    });
+    await act(async () => {});
+
+    // Commit a new `defaultValues` — what a wizard step transition does. The
+    // armed probe types into `note` from within the resulting effect flush.
+    armed = true;
+    await act(async () => { setDefaults({ name: 'Alice' }); });
+
+    fireEvent.submit(container.querySelector('form') as HTMLFormElement);
+
+    await waitFor(() => expect(submitted.length).toBe(1));
+    // With a passive reset this is `{ name: 'Alice' }` — `note` silently gone.
+    expect(submitted[0]).toEqual(expect.objectContaining({ name: 'Alice', note: 'hello' }));
+  });
+
+  it('still applies defaults that arrive late (edit-mode record landing)', async () => {
+    const Form = getFormRenderer();
+    let setDefaults!: (v: Record_) => void;
+
+    const Host = () => {
+      const [defaults, setD] = React.useState<Record_>({});
+      setDefaults = setD;
+      return (
+        <Form
+          schema={{ type: 'form', fields: FIELDS, defaultValues: defaults, showSubmit: false }}
+        />
+      );
+    };
+
+    const { container } = render(<Host />);
+    await waitFor(() => {
+      if (!container.querySelector('input[name="name"]')) throw new Error('not ready');
+    });
+
+    // The record finishes loading well after first paint — the form must adopt it.
+    await act(async () => { setDefaults({ name: 'Loaded', note: 'from server' }); });
+
+    await waitFor(() => {
+      const name = container.querySelector('input[name="name"]') as HTMLInputElement;
+      const note = container.querySelector('input[name="note"]') as HTMLInputElement;
+      expect(name.value).toBe('Loaded');
+      expect(note.value).toBe('from server');
+    });
+  });
+});

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

@@ -560,7 +560,18 @@ ComponentRegistry.register('form',
     const baselineRef = React.useRef<Record<string, unknown>>(
       (defaultValues ?? {}) as Record<string, unknown>,
     );
-    React.useEffect(() => {
+    // LAYOUT effect, deliberately — not a passive one. A passive effect runs a
+    // commit LATER than the render that produced the new values, so there is a
+    // window in which the new inputs are already mounted and interactive but the
+    // form still holds the old record. Anything typed in that window is
+    // destroyed: the pending `reset()` overwrites the whole record with
+    // `defaultValues`, silently dropping the field the user just filled. A
+    // layout effect runs synchronously inside the same commit, before the
+    // browser can paint or deliver a keystroke to those inputs, so the window
+    // does not exist. This surfaced as a flaky wizard test (#2982): a step
+    // transition changes `defaultValues`, and a value entered on the new step
+    // before the deferred reset landed vanished from the create payload.
+    React.useLayoutEffect(() => {
       let key: string;
       try { key = JSON.stringify(defaultValues ?? {}); } catch { key = String(Date.now()); }
       if (lastDefaultsKey.current === key) return;
@@ -614,8 +625,17 @@ ComponentRegistry.register('form',
       // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [defaultValues]);
 
-    // Watch for form changes - only track changes when onAction is available
-    React.useEffect(() => {
+    // Watch for form changes - only track changes when onAction is available.
+    // LAYOUT effect to stay in the same phase as the `defaultValues` reset
+    // above. React runs every layout DESTROY (mutation phase) before any layout
+    // CREATE, so a caller passing a fresh `onAction` each render — the common
+    // case, it is usually an inline arrow — has this subscription torn down
+    // before the reset runs and re-established after. That is what keeps a
+    // reset from being reported as a user edit. Leaving this passive while the
+    // reset is layout-phase inverts the order: the reset fires into the still
+    // live previous subscription and a record landing looks like the user
+    // editing every field it filled (#2968).
+    React.useLayoutEffect(() => {
       if (onAction) {
         const subscription = form.watch((data) => {
           onAction({
@@ -632,8 +652,9 @@ ComponentRegistry.register('form',
     // accidental discard of unsaved input). We compute it via a normalized
     // comparison against the pristine baseline (see computeDirty) rather than
     // react-hook-form's `isDirty`, which false-positives on fields that
-    // self-normalize their empty value on mount.
-    React.useEffect(() => {
+    // self-normalize their empty value on mount. Layout-phase for the same
+    // ordering reason as the subscription above.
+    React.useLayoutEffect(() => {
       const subscription = form.watch((values) => {
         onDirtyChangeProp?.(
           computeDirty(baselineRef.current, values as Record<string, unknown>),