add react-jsx-dev-runtime to external

[mike-anderson]

PR: Fix React 18 Strict Mode Compatibility & Externalize JSX Dev Runtime

Summary

This PR addresses React 18 development mode compatibility issues and simplifies consuming application configurations by properly externalizing the JSX dev runtime.

Changes

1. Add react/jsx-dev-runtime to External Dependencies

Problem: When consuming applications (like the demo) use this library in development mode, Vite's React plugin uses react/jsx-dev-runtime for JSX transformation instead of react/jsx-runtime. If only react/jsx-runtime is externalized, the dev runtime gets bundled into our library, causing React to be duplicated in the final bundle.

Solution: By adding react/jsx-dev-runtime to our rollup externals:

external: [
  "react",
  "react-dom",
  "react/jsx-runtime",
  "react/jsx-dev-runtime",  // ← new
  "superdoc",
],

This ensures both production and development JSX runtimes are treated as peer dependencies, preventing React duplication regardless of build mode.

Benefit for consumers: The demo app's vite.config.ts no longer needs the resolve.dedupe: ['react', 'react-dom', 'react/jsx-runtime'] workaround. The config is now minimal:

export default defineConfig({
    plugins: [react()],
    base: '/',
});

2. Refactor SuperDoc Initialization Guard for React 18 Strict Mode

Problem: React 18's Strict Mode intentionally double-invokes effects in development to help identify cleanup issues. The previous initialization pattern could cause race conditions where:

• A SuperDoc instance starts initializing
• Cleanup runs (from Strict Mode re-render)
• The first initialization completes and attaches to a stale ref
• A new initialization begins, leading to duplicate instances or orphaned state

Solution: Implemented an abort pattern using a local aborted flag:

useEffect(() => {
  let aborted = false;
  let instance: SuperDoc | null = null;

  const initSuperDoc = async () => {
    const { SuperDoc } = await import("superdoc");
    
    // If cleanup ran while we were importing, abort
    if (aborted) return;

    instance = new SuperDoc({
      // ...
      onReady: () => {
        if (aborted) return;  // Guard callback execution too
        // ...
      },
    });
  };

  initSuperDoc();

  return () => {
    aborted = true;
    if (instance) {
      instance.destroy?.();
    }
    superdocRef.current = null;
  };
}, [document.source, document.mode]);

Key improvements:

• Abort flag: Prevents the async initialization from completing if cleanup has already run
• Local instance tracking: The cleanup now destroys the instance it created, not whatever happens to be in the ref (which could be from a different effect cycle)
• Callback guards: The onReady callback checks the abort flag to prevent state updates after unmount
• Removed discoverAndApplyFields from deps: This function is recreated on every render due to closure dependencies, causing unnecessary re-initialization

Testing

• Verified with yarn and npm package managers
• Tested in React 18 development mode with Strict Mode enabled

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[caio-pizzol]

Thanks for tackling the React 18 Strict Mode issues @mike-anderson!

Overall the core changes look good - the abort pattern is well-implemented and solves a real problem. Just needs some cleanup to keep the PR focused.

[caio-pizzol]

No need to change version - semantic-release will bump patch this automatically (make sure to use fix: ... commit prefix)

[caio-pizzol]

The disable is acceptable here, but a brief comment explaining why would help future readers

[mike-anderson]

@caio-pizzol not sure your stance on this, but we could link a comment or PR for further depth. This was the "conversation" I had with Claude

---

The Problem with discoverAndApplyFields in the Dependency Array

Look at how discoverAndApplyFields is defined:

  const discoverAndApplyFields = useCallback(
    (editor: Editor) => {
      // ... function body ...
    },
    [onFieldsDiscovered, updateFieldInDocument],
  );

This useCallback has dependencies on:

1. onFieldsDiscovered - a prop passed by the consumer
2. updateFieldInDocument - another useCallback (which is stable with [] deps)

Here's the issue: If the parent component passes onFieldsDiscovered like this:

<SuperDocESign 
  onFieldsDiscovered={(fields) => console.log(fields)}
  // ...
/>

Every time the parent re-renders, onFieldsDiscovered is a new function reference. This causes:

1. discoverAndApplyFields to be recreated (new reference)
2. If discoverAndApplyFields was in the effect's deps, the effect would re-run
3. SuperDoc gets destroyed and reinitialized on every parent re-render 💥

Why Removing It Is Safe

The function already uses fieldsRef.current internally:

      fieldsRef.current.document?.forEach((f) => {
        if (f.id) configValues.set(f.id, f.value);
      });

      fieldsRef.current.signer?.forEach((f) => {
        if (f.value !== undefined) {
          configValues.set(f.id, f.value);
        }
      });

Because fieldsRef is a ref (not a dependency), the function always accesses the current field values regardless of when it was created.

What Actually Happens Now

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [document.source, document.mode]);

1. Effect only runs when document.source or document.mode changes
2. discoverAndApplyFields is captured at the time the effect runs (closure)
3. The closure is "stale" in the sense that onFieldsDiscovered points to the version from that render
4. But this is fine because:
   • Field data comes from fieldsRef.current (always fresh)
   • onFieldsDiscovered is only called once during initialization anyway

Alternative Approach (More Complex)

If we wanted to be "correct" by React rules, we'd need to also ref-ify the callbacks:

const onFieldsDiscoveredRef = useRef(onFieldsDiscovered);
onFieldsDiscoveredRef.current = onFieldsDiscovered;

const discoverAndApplyFields = useCallback((editor: Editor) => {
  // ...
  onFieldsDiscoveredRef.current?.(discovered);  // Use ref
}, []);  // Now has no deps, always stable

But this adds complexity for little benefit—the current pattern is a pragmatic trade-off that's common in React codebases with async initialization.

The Timeline Assuming We Actually Change onFieldsDiscovered

1. Component mounts, effect runs
   └─ Captures discoverAndApplyFields (v1) which has onFieldsDiscovered (v1) in its closure

2. async import("superdoc") is happening...

3. Parent re-renders with new onFieldsDiscovered (v2)
   └─ Creates new discoverAndApplyFields (v2)
   └─ BUT effect doesn't re-run, still has discoverAndApplyFields (v1)

4. SuperDoc finishes loading, onReady fires
   └─ Calls discoverAndApplyFields(v1)
   └─ Which calls onFieldsDiscovered(v1) ← THE OLD ONE

So yes, if the parent changes onFieldsDiscovered between mount and onReady firing, the old callback gets called.

Why this is probably acceptable:

1. onReady fires once, early — typically within milliseconds of mount. The window for a parent re-render to happen before this is small.

2. onFieldsDiscovered is a "discovery" event — it's semantically a one-time notification. The parent is saying "tell me what fields you found." It's not an ongoing listener.

3. In practice, parents rarely change these callbacks dynamically. They're usually stable references.

If you wanted to be bulletproof:

You'd ref-ify onFieldsDiscovered:

const onFieldsDiscoveredRef = useRef(onFieldsDiscovered);
useEffect(() => {
  onFieldsDiscoveredRef.current = onFieldsDiscovered;
});

const discoverAndApplyFields = useCallback((editor: Editor) => {
  // ...
  onFieldsDiscoveredRef.current?.(discovered);  // Always calls latest
}, [updateFieldInDocument]);  // No longer depends on onFieldsDiscovered

This would ensure the latest callback is always invoked, even if the parent updates it after mount but before onReady.

Trade-off decision

The current implementation prioritizes:

• Simplicity over theoretical correctness
• Preventing reinitializations (the bigger problem) over perfect callback freshness

Given that onFieldsDiscovered is a one-shot event that fires early, the stale closure risk is low. But if you're concerned about edge cases (e.g., slow network loading the SuperDoc bundle), the ref pattern above would eliminate it entirely.

[caio-pizzol]

This reformats the entire file (quotes, indentation). Could we keep just the meaningful change?
hint: use husky locally for pre-commit hook or run lint/format script commands

[mike-anderson]

Not sure on this one - I just have my vscode set to auto-run the formatter for the project on save

[caio-pizzol]

@mike-anderson closing this one - fixes being addressed here: #7