fix(react): stabilize user/users/modules props to prevent re-init flicker (SD-2635) (#2876)

* fix(react): stabilize user/users/modules props to prevent re-init flicker (SD-2635)

Consumers passing inline object literals (the idiomatic React pattern) caused
a full SuperDoc destroy + rebuild on every parent re-render because the
main useEffect compared these props by reference identity. When a consumer
stored the SuperDoc instance in state from onReady, the resulting re-render
supplied fresh references and triggered another cycle — observed as 4 full
destroy/re-init cycles in ~100ms with visible display:none/block flicker.

Wrap user, users, and modules in a new useStableValue hook that returns a
reference-stable version only changing identity when the structural content
actually changes (via JSON.stringify compare, run only on reference-change).
Semantics are strictly a superset of the prior behavior — value changes still
rebuild; reference-only changes no longer do.

* fix(react): drop modules from structural memo; harden tests (SD-2635 review round 2)

Review feedback addressed:

- Correctness: `modules` carries function-valued options (`permissionResolver`,
  `popoverResolver`) and live objects (`collaboration.ydoc`/`provider`) that
  JSON.stringify silently drops or collapses. Keeping it on structural
  compare would silently ignore real config changes. Reverted to reference
  identity for `modules`; `user` and `users` stay memoized since they are
  plain data.

- Naming: renamed `useStableValue` → `useStructuralMemo` to match the
  useMemo family and signal non-referential equality.

- JSDoc: expanded the hook's docblock to spell out every JSON.stringify
  footgun consumers need to know about (functions dropped, class instances
  collapsed, undefined values dropped, NaN/Infinity → null, circular refs,
  key-insertion-order sensitivity).

- Tests: replaced the brittle `setTimeout(100)` negative assertion with a
  synchronous `ref.getInstance()` identity check; strengthened the "rebuilds
  on change" test to also assert a second onReady + a fresh instance;
  added a `users`-prop stability test; added a StrictMode + rerender test
  to guard the ref-write-during-render path.

* fix(react): define event types explicitly to unblock CI type-check (SD-2635)

CI type-check failed with `Property 'onEditorUpdate' does not exist on
type 'Config'` even though the JSDoc `Config` typedef in superdoc clearly
declares it. The old approach derived SuperDocEditorUpdateEvent and
SuperDocTransactionEvent via
`Parameters<NonNullable<SuperDocConstructorConfig['onEditorUpdate']>>[0]`,
which walked a chain:

  ConstructorParameters<typeof SuperDoc>[0]
    → @param {Config} in core/SuperDoc.js
    → @typedef Config in core/types/index.js
    → @Property onEditorUpdate with @typedef EditorUpdateEvent

This chain resolves fine locally but breaks on CI — the exact failure
point in JSDoc resolution depends on TS version, moduleResolution mode,
and the `customConditions: ["source"]` in tsconfig.base.json (which
routes imports to the raw .js source instead of the built .d.ts).

Define the two event shapes inline instead, mirroring superdoc's JSDoc.
No behavior change for consumers — same fields, same optionality.

* fix(react): round-4 review feedback — lock modules contract, restore transaction: any (SD-2635)

Addressing consensus findings from the round-3 review:

- Revert `transaction: unknown` back to `any` to match superdoc's upstream
  typedef. `unknown` was a narrowing from `any` that would have broken
  existing consumer code like `event.transaction.docChanged`.

- Re-export `EditorSurface` from the package barrel. It's referenced by
  two public event interfaces but wasn't exported, so consumers couldn't
  name the `surface` field's type.

- Symmetrize per-field JSDoc on `SuperDocTransactionEvent` to match its
  sibling `SuperDocEditorUpdateEvent`.

- Add a regression test asserting that passing a new `modules` object
  with identical content DOES rebuild the editor. This pins the contract
  that `modules` stays on reference identity (it can carry functions and
  live objects that structural compare misses) — a future "cleanup" that
  wraps `modules` in useStructuralMemo would silently re-introduce the
  SD-2635 blocker without this test.

Also trim commentary added during rounds 1-3: the stabilization rationale
was documented twice in SuperDocEditor.tsx (once at the destructure,
once near the dep array), and the types.ts docstrings leaked maintainer
build-tooling rationale into consumer IDE hovers.

* refactor(react): swap JSON.stringify compare for lodash.isequal; drop TransactionEvent (SD-2635)

- Swap the hand-rolled JSON.stringify-based structural compare for
  `lodash.isequal`. +10KB raw / +3.7KB gzipped cost, but removes the
  5-bullet footgun list from JSDoc and gets proper handling of Dates,
  Maps, circular refs, and key ordering for free. For our use (stabilizing
  `user`/`users` plain-data records) the 3.7KB buys nothing practical,
  but it removes ~40 lines of hand-maintained code and a bag of edge
  cases. Trade maintenance cost for bundle cost.

- Rename `useStructuralMemo` → `useMemoByValue`. Plainer, matches the
  useMemo family, says what it does without jargon.

- Drop `SuperDocTransactionEvent` from the public API. It was exported
  but never wired up to a callback prop in `CallbackProps`, so nothing
  fires it. Its shape also leaked ProseMirror's `transaction` object —
  a deprecated surface per superdoc's own notes. Removing it now is
  cheaper than removing it after someone starts relying on it.

- Replace the JSON-stringify-specific unit tests with tests that exercise
  what lodash.isequal actually gives us (key-order insensitivity,
  same-reference function equality). 27/27 tests pass.

* refactor(react): drop lodash.isequal dep, inline JSON.stringify compare (SD-2635)

Round 5 added lodash.isequal for `useMemoByValue` correctness at the
cost of +10KB raw / +3.7KB gzipped. For the two props actually using the
hook (`user` and `users` — small plain-data records) the extra
correctness buys nothing practical: those records contain strings only,
no Dates, no Maps, no circular refs, no functions.

Revert to an inline JSON.stringify compare wrapped in a `try/catch` for
circular refs. The hook is now ~15 lines, zero dependencies, and the
unit test that required lodash (key-order insensitivity) is replaced by
a circular-ref fallback test that matches the implementation.

Bundle is back to 3.69 KB / 1.60 KB gzipped.

* test(react): cover StrictMode + user prop value change (SD-2635)

The existing StrictMode test only proved same-content rerender stays
stable — the positive path (real value change under StrictMode still
triggers destroy + fresh onReady) wasn't exercised. Coverage audit
after rounds 2-6 flagged this as the one gap worth closing before
ship. Mirrors the existing non-StrictMode rebuild test, wrapped in
<StrictMode>.

Author: caio-pizzol

packages/react/src/SuperDocEditor.test.tsx

@@ -175,6 +175,213 @@ describe('SuperDocEditor', () => {
     });
   });
 
+  describe('prop stability (SD-2635)', () => {
+    it('does not destroy/re-init when user prop is a new object literal with identical content', async () => {
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <SuperDocEditor
+          ref={ref}
+          user={{ name: 'Alex', email: 'alex@example.com' }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+      expect(instanceBefore).toBeTruthy();
+
+      // Re-render with a *new* object literal carrying the same content —
+      // this is the idiomatic React pattern that used to trigger a full
+      // destroy + re-init loop before SD-2635.
+      rerender(
+        <SuperDocEditor
+          ref={ref}
+          user={{ name: 'Alex', email: 'alex@example.com' }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      // Same underlying instance proves no destroy+rebuild happened.
+      expect(ref.current?.getInstance()).toBe(instanceBefore);
+      expect(onEditorDestroy).not.toHaveBeenCalled();
+    });
+
+    it('does not destroy/re-init when users prop is a new array literal with identical content', async () => {
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <SuperDocEditor
+          ref={ref}
+          users={[{ name: 'Alex', email: 'alex@example.com' }]}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+
+      rerender(
+        <SuperDocEditor
+          ref={ref}
+          users={[{ name: 'Alex', email: 'alex@example.com' }]}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      expect(ref.current?.getInstance()).toBe(instanceBefore);
+      expect(onEditorDestroy).not.toHaveBeenCalled();
+    });
+
+    it('rebuilds and remounts a new instance when user prop value actually changes', async () => {
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <SuperDocEditor
+          ref={ref}
+          user={{ name: 'Alex', email: 'alex@example.com' }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+
+      rerender(
+        <SuperDocEditor
+          ref={ref}
+          user={{ name: 'Jamie', email: 'jamie@example.com' }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      // Old instance torn down, new instance ready.
+      await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 });
+      await waitFor(() => expect(onReady).toHaveBeenCalledTimes(2), { timeout: 5000 });
+      expect(ref.current?.getInstance()).not.toBe(instanceBefore);
+    });
+
+    it('stays stable under StrictMode double-invocation on rerender', async () => {
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <StrictMode>
+          <SuperDocEditor
+            ref={ref}
+            user={{ name: 'Alex', email: 'alex@example.com' }}
+            onReady={onReady}
+            onEditorDestroy={onEditorDestroy}
+          />
+        </StrictMode>,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+      const destroysBefore = onEditorDestroy.mock.calls.length;
+
+      rerender(
+        <StrictMode>
+          <SuperDocEditor
+            ref={ref}
+            user={{ name: 'Alex', email: 'alex@example.com' }}
+            onReady={onReady}
+            onEditorDestroy={onEditorDestroy}
+          />
+        </StrictMode>,
+      );
+
+      expect(ref.current?.getInstance()).toBe(instanceBefore);
+      expect(onEditorDestroy.mock.calls.length).toBe(destroysBefore);
+    });
+
+    it('still rebuilds under StrictMode when user prop value actually changes', async () => {
+      // The same-content StrictMode test above proves memoization survives
+      // double-invocation. This test proves the positive path — a real
+      // value change under StrictMode still tears down and remounts.
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <StrictMode>
+          <SuperDocEditor
+            ref={ref}
+            user={{ name: 'Alex', email: 'alex@example.com' }}
+            onReady={onReady}
+            onEditorDestroy={onEditorDestroy}
+          />
+        </StrictMode>,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+
+      rerender(
+        <StrictMode>
+          <SuperDocEditor
+            ref={ref}
+            user={{ name: 'Jamie', email: 'jamie@example.com' }}
+            onReady={onReady}
+            onEditorDestroy={onEditorDestroy}
+          />
+        </StrictMode>,
+      );
+
+      await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 });
+      await waitFor(() => expect(ref.current?.getInstance()).not.toBe(instanceBefore), { timeout: 5000 });
+    });
+
+    it('rebuilds when a new modules object is passed, even if content looks equal', async () => {
+      // `modules` is intentionally kept on reference identity in the dep
+      // array because it can carry functions and live objects that a
+      // structural compare would miss. This test pins that contract —
+      // if a future refactor wraps `modules` in useStructuralMemo, this
+      // test will fail and flag the regression.
+      const ref = createRef<SuperDocRef>();
+      const onReady = vi.fn();
+      const onEditorDestroy = vi.fn();
+
+      const { rerender } = render(
+        <SuperDocEditor
+          ref={ref}
+          modules={{ comments: { visible: true } }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 });
+      const instanceBefore = ref.current?.getInstance();
+
+      rerender(
+        <SuperDocEditor
+          ref={ref}
+          modules={{ comments: { visible: true } }}
+          onReady={onReady}
+          onEditorDestroy={onEditorDestroy}
+        />,
+      );
+
+      await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 });
+      await waitFor(() => expect(onReady).toHaveBeenCalledTimes(2), { timeout: 5000 });
+      expect(ref.current?.getInstance()).not.toBe(instanceBefore);
+    });
+  });
+
   describe('unique IDs', () => {
     it('should generate unique container IDs for multiple instances', () => {
       const { container: container1 } = render(<SuperDocEditor />);

packages/react/src/SuperDocEditor.tsx

@@ -7,7 +7,7 @@ import {
   type CSSProperties,
   type ForwardedRef,
 } from 'react';
-import { useStableId } from './utils';
+import { useStableId, useMemoByValue } from './utils';
 import type {
   CallbackProps,
   DocumentMode,
@@ -50,8 +50,8 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef<Super
     onException,
     // Key props that trigger rebuild when changed
     document: documentProp,
-    user,
-    users,
+    user: userProp,
+    users: usersProp,
     modules,
     // All other props passed through
     ...restProps
@@ -61,6 +61,13 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef<Super
   const documentMode = props.documentMode ?? 'editing';
   const role = props.role ?? 'editor';
 
+  // `user` and `users` are memoized by value so inline literals don't
+  // trigger a rebuild. `modules` stays on reference identity — it can
+  // carry functions and live objects (e.g. `collaboration.provider`)
+  // that a consumer may intentionally swap. See SD-2635.
+  const user = useMemoByValue(userProp);
+  const users = useMemoByValue(usersProp);
+
   const instanceRef = useRef<SuperDocInstance | null>(null);
   const toolbarContainerRef = useRef<HTMLDivElement | null>(null);
 
@@ -223,8 +230,8 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef<Super
       destroyed = true;
     };
     // Only these props trigger a full rebuild. Other props (rulers, etc.) are
-    // initial values - use getInstance() methods to change them at runtime.
-    // Note: restProps is intentionally excluded to avoid rebuilds on every render.
+    // initial values — use getInstance() methods to change them at runtime.
+    // restProps is intentionally excluded to avoid rebuilds on every render.
     // documentMode is handled separately via setDocumentMode() for efficiency.
   }, [documentProp, user, users, modules, role, hideToolbar, contained, containerId, toolbarId]);
 

packages/react/src/index.ts

@@ -20,10 +20,10 @@ export type {
 
   // Callback event types
   Editor,
+  EditorSurface,
   SuperDocReadyEvent,
   SuperDocEditorCreateEvent,
   SuperDocEditorUpdateEvent,
-  SuperDocTransactionEvent,
   SuperDocContentErrorEvent,
   SuperDocExceptionEvent,
 } from './types';

packages/react/src/types.ts

@@ -50,11 +50,22 @@ export interface SuperDocEditorCreateEvent {
   editor: Editor;
 }
 
-/** Event passed to onEditorUpdate callback */
-export type SuperDocEditorUpdateEvent = Parameters<NonNullable<SuperDocConstructorConfig['onEditorUpdate']>>[0];
+/** Surface where an editor event originated. */
+export type EditorSurface = 'body' | 'header' | 'footer';
 
-/** Event passed to onTransaction callback */
-export type SuperDocTransactionEvent = Parameters<NonNullable<SuperDocConstructorConfig['onTransaction']>>[0];
+/** Event passed to onEditorUpdate callback. Mirrors superdoc's EditorUpdateEvent. */
+export interface SuperDocEditorUpdateEvent {
+  /** The primary editor associated with the update. For header/footer edits, this is the main body editor. */
+  editor: Editor;
+  /** The editor instance that emitted the update. For body edits, this matches `editor`. */
+  sourceEditor: Editor;
+  /** The surface where the edit originated. */
+  surface: EditorSurface;
+  /** Relationship ID for header/footer edits. */
+  headerId?: string | null;
+  /** Header/footer variant (`default`, `first`, `even`, `odd`) when available. */
+  sectionType?: string | null;
+}
 
 /** Event passed to onContentError callback */
 export interface SuperDocContentErrorEvent {

packages/react/src/utils.test.ts

@@ -0,0 +1,75 @@
+import { describe, it, expect } from 'vitest';
+import { renderHook } from '@testing-library/react';
+import { useMemoByValue } from './utils';
+
+describe('useMemoByValue', () => {
+  it('returns the same reference across renders when content is unchanged', () => {
+    const initial = { name: 'Alex', email: 'alex@example.com' };
+    const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), {
+      initialProps: { value: initial },
+    });
+
+    const first = result.current;
+    expect(first).toBe(initial);
+
+    // Parent passes a fresh object literal with identical content
+    rerender({ value: { name: 'Alex', email: 'alex@example.com' } });
+    expect(result.current).toBe(first); // same reference — critical for effect deps
+
+    // And again, still stable
+    rerender({ value: { name: 'Alex', email: 'alex@example.com' } });
+    expect(result.current).toBe(first);
+  });
+
+  it('returns a new reference when the content actually changes', () => {
+    const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), {
+      initialProps: { value: { name: 'Alex' } },
+    });
+
+    const first = result.current;
+    rerender({ value: { name: 'Jamie' } });
+    expect(result.current).not.toBe(first);
+    expect(result.current.name).toBe('Jamie');
+  });
+
+  it('handles undefined and null stably', () => {
+    const { result, rerender } = renderHook(({ value }) => useMemoByValue(value as unknown), {
+      initialProps: { value: undefined },
+    });
+
+    const first = result.current;
+    rerender({ value: undefined });
+    expect(result.current).toBe(first);
+
+    rerender({ value: null });
+    expect(result.current).toBe(null);
+  });
+
+  it('stabilizes arrays the same way as objects', () => {
+    const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), {
+      initialProps: { value: [{ id: 1 }, { id: 2 }] },
+    });
+
+    const first = result.current;
+    rerender({ value: [{ id: 1 }, { id: 2 }] });
+    expect(result.current).toBe(first);
+
+    rerender({ value: [{ id: 1 }, { id: 3 }] });
+    expect(result.current).not.toBe(first);
+  });
+
+  it('adopts a new reference on circular input (JSON.stringify throws)', () => {
+    const circularA: { self?: unknown; name: string } = { name: 'a' };
+    circularA.self = circularA;
+
+    const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), {
+      initialProps: { value: circularA },
+    });
+
+    const circularB: { self?: unknown; name: string } = { name: 'a' };
+    circularB.self = circularB;
+    rerender({ value: circularB });
+    // The compare throws; the hook falls back to adopting the new reference.
+    expect(result.current).toBe(circularB);
+  });
+});