Skip to content

Flow editor autosave is cancelled without a replacement on unmount: an edit made in the last 1500 ms before leaving the page is dropped, and lastSavedSerializedRef already counts it as saved #1157

Description

@rawdaymx

Summary

The flow editor's autosave is handleChanges in react-flow-wrapper.tsx:162-171useDebouncedCallback(…, 1500, 4000), the call that actually writes the draft to the server. useDebouncedCallback ends with

// packages/ui/src/hooks/use-debounced-callback.ts:25
React.useEffect(() => () => debounced.cancel(), [debounced])

so on unmount the pending write is cancelled, never flushed. Navigating away from the flow page unmounts the whole tree, and nothing anywhere calls the flush on that path. An edit made in the last 1500 ms before leaving (up to 4000 ms, since the maxWait only bounds when the timer may fire, not whether it survives) is dropped with no error and no indication.

The same component makes that silent: lastSavedSerializedRef is written when a change is detected (:330), not when it is saved (markSaved, :337). By the time the cancel happens, the ref already holds the content that was never sent.

This is not the mechanism in #1154. That one is about pushToFlow in nodes/editor.tsx, which does flush on unmount and loses the value further downstream. This is the wrapper's own debounce, a different timer carrying different data — and notably, the editor's sibling debounce in the same feature does flush on unmount while this one does not.

Environment

Measured against upstream/main @ be7234b93bb47936e9469016ccb82e9b2aa765b7
Also valid for 96032013egit log 96032013e..be7234b93 -- react-flow-wrapper.tsx use-debounced-callback.ts is empty, neither file changed
How reading the repository at that commit, plus a red→green run of a reproduction test on our fork. Not measured against a deployed instance
Node / pnpm / vitest v24.11.0 / 10.33.2 / 4.1.8
Date 2026-09-12

Nobody flushes on the way out

The wrapper hands both controls to its parent (react-flow-wrapper.tsx:347-348), and the flush has exactly one caller, repository-wide:

$ git grep -n 'flushAutosave' be7234b93 -- apps/builder/src
…/react-flow/frame.tsx:32:  const [flushAutosave, setFlushAutosave] = useState<(() => void) | null>(null)
…/react-flow/frame.tsx:69:        flushAutosave?.()
…/react-flow/frame.tsx:74:    [flushAutosave, setOpenNodeDetailSheet],
…/react-flow/react-flow-wrapper.tsx:96:  onAutosaveFlushChange: (flushAutosave: (() => void) | null) => void

and that one caller fires only when the node detail sheet closes:

// frame.tsx:66-74
const handleNodeDetailSheetOpenChange = useCallback(
  (open: boolean) => {
    if (!open) {
      flushAutosave?.()
    }
    setOpenNodeDetailSheet(open)
  },
  [flushAutosave, setOpenNodeDetailSheet],
)

So closing the panel is covered. Leaving the page is not. There is no beforeunload either:

$ git grep -n 'beforeunload' be7234b93 -- apps/builder/src/features/flows
(no matches)

The cleanup at react-flow-wrapper.tsx:352-354 only sets the parent's pointers back to null; it does not call the function.

And a flush after the cancel would not help even if something tried: cancel() in createDebouncedFn runs clearTimer(); resetBurst(), and resetBurst clears lastArgs — while flush() early-returns when there is no timer. The arguments are gone, not just the timer.

Evidence: the harness goes red without the exit write and green with it

We fixed this on our fork, and the reproduction is a test that mounts the real ReactFlowWrapper and the real node editor as siblings, with fake timers and a stubbed draft action, exercising both unmount orders.

To measure upstream's behaviour rather than ours, we disabled our exit write — one line — which leaves exactly upstream's arrangement: the useDebouncedCallback cancel and no flush on unmount.

# with our exit write disabled  (= upstream's behaviour)
$ pnpm exec vitest run …/draft-exit-save.test.tsx
  Tests  34 failed | 12 passed (46)

# with it restored  (= our fork)
$ pnpm exec vitest run …/draft-exit-save.test.tsx
  Tests  46 passed (46)

34 of 46 scenes depend on this. The harness discriminates: it is not a test that has only ever been seen green.

Why we think this is a defect rather than the intended design

Cancelling a pending autosave is deliberate in this codebase, and upstream documents why. But every intentional cancel is immediately followed by a replacement write:

  • delete-node-orchestrator.ts:22-27 — the comment is explicit: "Cancels the pending autosave debounce. Load-bearing: a pending pre-delete save still holds the node, and if it fired after our save it would write the old node list back and resurrect the deleted node." It cancels, then performs its own save with the post-delete state (:60).
  • duplicate-node-orchestrator.ts:22,47 — same pattern.
  • flow-edit-toolbar.tsx:185 — revert to published: cancels, then replaces the canvas with the published version. Discarding the draft is the point there.

The unmount cancel in use-debounced-callback.ts:25 is the only one that cancels with nothing taking its place. That is an asymmetry with the file's own established contract, not a fourth instance of a deliberate pattern.

Two more things point the same way. The generic hook cleanup is right for the other thirteen call sites — a search box, a receiver count, a template filter — where dropping a pending call on unmount is exactly what you want; it is wrong only for the one caller whose pending call is a durability guarantee. And in this same feature, nodes/editor.tsx explicitly flushes its own debounce on unmount (useEffect(() => () => pushToFlow.flush(), [pushToFlow])), which shows the authors did reach for flush-on-unmount where they thought about it.

On the lastSavedSerializedRef half. On its own it is harmless: for every other failure mode the canvas state is never rolled back, so the next edit produces a different serialization, the comparison at :325-327 fails, and handleChanges goes out carrying the whole current canvas — the content self-heals. What the cancel-without-replacement removes is precisely that next edit. With the ref already holding the unsent content and no further change coming, if (serialized === lastSavedSerializedRef.current) return blocks the only recovery path this component has. So we are reporting it as one defect with two parts rather than two issues: the ref is why the loss is silent and permanent, not a separate bug.

Suggested fix

The pending write has to survive the unmount instead of being cancelled. Two shapes, both reasonable:

  1. Narrow — give useDebouncedCallback an opt-in so a caller whose pending call is durable flushes instead of cancelling (useDebouncedCallback(fn, 1500, { maxWait: 4000, flushOnUnmount: true })), and set it on handleChanges only. The other thirteen call sites keep today's behaviour, which is correct for them.
  2. Local — leave the hook alone and add an unmount effect in react-flow-wrapper.tsx that writes the pending draft directly, the way nodes/editor.tsx already does for its own debounce.

We took the second on our fork, because a flush during unmount still has to reach the save path without depending on a render — which is the subject of #1154 and the reason a bare flush() is not sufficient on its own. Our version keeps the last detected draft in a ref alongside lastSavedSerializedRef and writes from that ref in the unmount effect.

Happy to send it as a PR, and the reproduction test with it — that test is self-contained and would pin this against regression regardless of which shape you pick.

What we did not verify

  • We did not reproduce this against a running instance. No browser lost an operator's text in front of us. The evidence is the code path plus the red→green harness described above, which runs against real components under fake timers, not against a real server or a real navigation.
  • The 34/46 figure comes from our fork's test with our own fix disabled, which we believe reproduces upstream's arrangement for this mechanism. It is not a measurement of stock upstream/main, which does not contain that test file.
  • We did not check the exact React cleanup ordering between the hook's internal effect and the wrapper's registration effect. It does not change the outcome — neither cleanup flushes, so the cancel wins in any order — but we did not establish the order itself.
  • We did not audit the other thirteen useDebouncedCallback call sites for the same durability concern. We believe they are fine, but that is a reading, not a check.

Related issues

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions