Commit a3487da
authored
improvement(files): remove Save UI in favor of silent autosave with local-first draft recovery (#5549)
* improvement(files): remove Save UI in favor of silent autosave with local-first draft recovery
Files editor no longer shows a Save/Saving/Save failed button - autosave
already ran in the background, the button was vestigial. Cmd+S still works.
Adds local-first draft persistence to the shared useAutosave hook (opt-in via
draftKey): edits mirror into IndexedDB on a 400ms debounce, independent of the
1.5s network save, and flush best-effort on visibilitychange/pagehide. On
reopen, a newer local draft is silently recovered and resynced. This replaces
the beforeunload "leave site?" warning, which only blocked navigation - it
never actually saved anything.
A toast (with a Retry action) surfaces a save failure, since there's no more
persistent status indicator to show it.
* improvement(files): simplify autosave draft persistence and dedupe editor resync
Structural cleanup after the local-draft feature: draftKey is now ANDed with
enabled inside useAutosave itself (rather than trusting every caller to
replicate that gating), the combined dirty-transition/debounce effect is
split into two single-purpose effects, redundant back-to-back IndexedDB
writes on visibilitychange+pagehide are deduped, a dead identity wrapper
around setDraftContent is removed, and the three near-identical
"resync editor body if changed" blocks in rich-markdown-editor.tsx collapse
into one local helper.
* fix(files): flush pending local draft on unmount, fix stale Retry target
Two real bugs from Greptile's first review pass:
- Unmounting before the 400ms local-draft debounce fired cancelled the
pending timer without ever writing the draft, so if the network flush
also failed on the way out, the edit had no backup anywhere. The unmount
cleanup now calls persistLocalDraft() synchronously before attempting
the network flush.
- The save-failure toast's Retry action read saveRef.current lazily at
click time, so navigating to a different file before clicking Retry
would retry-save the wrong file. It now captures the failing file's
save function at the moment the toast is created.
* fix(files): Discard Changes now actually resets editor content
Discard previously only cleared the parent's mirrored isDirty/saveStatus
state; the editor's own content was never reset to match the server
baseline. On unmount, useAutosave's flush logic saw content still
diverged and (a) re-saved the "discarded" edit to the server, and (b)
after this PR's local-draft addition, also persisted it to IndexedDB —
so even a future fix to (a) would still have the draft resurrect the
discarded text on next open.
Adds a discardRef bridge (mirrors the existing saveRef pattern) so
Discard resets the editor's draft content back to savedContent before
navigating away, closing both paths at the root.
* fix(files): make Discard deterministic, independent of state-update timing
The previous discard fix (setDraftContent(savedContent) before navigating)
relied on that dispatch landing before the FileViewer unmounts. If unmount
raced ahead of it, the autosave cleanup would still see stale dirty content
and could resurrect the discarded edit via the local draft.
useAutosave now exposes discard(): it flags the instance as discarded,
cancels any pending timers, and clears the local draft immediately. Every
write path (persistLocalDraft, save, the unmount flush) checks that flag
first, so nothing written after discard() can bring the edit back,
regardless of whether the content-reset render has committed yet.
* fix(files): correct in-flight save after discard, fix IndexedDB write/delete ordering
Two more real races from round 4 of review:
- discard() couldn't stop a save that had already started (discardedRef
only blocks saves not yet begun). Once that in-flight save lands, it
now schedules a corrective save to push the reverted content, rather
than leaving the discarded edit on the server permanently. Only fires
when a save was genuinely in flight at discard time.
- persistLocalDraft's set() and clearLocalDraft's del() were independent
promises with no ordering guarantee. A slow write starting before
discard could resolve after discard's delete and resurrect the draft.
Both now go through a single serialized queue per hook instance, so a
delete queued after a write always runs after it completes.
* fix(files): make discard's corrective save use an explicit baseline
The corrective save (from the previous fix) relied on the caller's
setDraftContent(savedContent) having landed by the time it ran — a real
race, not a guarantee: React commits that render on its own schedule,
and if the correction's continuation runs first, onSave still reads the
ambient (still-dirty) content ref and re-persists the discarded edit.
onSave now accepts an optional override content; discard() captures
savedContentRef.current as an explicit target at the moment it's called
and passes it through, so the correction always pushes the true reverted
baseline regardless of render timing. Widened the shared onSave type is
backward compatible — the other useAutosave caller (chunk-editor) ignores
the extra optional param.
* fix(files): retry no longer depends on a remount-able shared ref, purge stale drafts
Two more from round 6:
- The failure toast's Retry action captured saveRef.current inside the
effect reacting to saveStatus='error' — but if the user switched files
between the failure occurring and that effect committing, the keyed
remount could have already repointed saveRef at the new file's save
function first. onSaveStatusChange now passes the failing instance's
own saveImmediately alongside the 'error' status directly from the
hook that owns it, so retry can never be sourced from the wrong file
regardless of remount timing.
- A local draft with a stale (mismatched) baseline was left in IndexedDB
after being correctly skipped for recovery, so it could resurrect later
if the server baseline ever coincidentally matched it again. It's now
purged as soon as it's identified as stale.
* fix(files): clear inFlightRef once a save settles
inFlightRef.current was never reset after a save resolved or rejected —
it stayed pointing at the (now-fulfilled) promise indefinitely. discard()
reads it to decide whether a save is genuinely in flight; since a
resolved promise is still truthy, discard() treated any prior completed
save as still pending, captured savedContentRef.current as the
"corrective" target, and could push a stale baseline if that capture
happened before the save's own dispatch had updated it.
Now cleared to null as soon as the save settles, so discard()'s
in-flight check reflects reality regardless of how long ago the last
save finished.
* fix(files): surface a failed discard correction, resume autosave after discard if editing continues
Two more from round 8:
- If discard()'s corrective save failed, it was only logged — the server
could permanently keep the discarded edit with zero user-facing signal.
Now surfaced via a dedicated onDiscardCorrectionFailed callback, which
closes over the specific file's own name rather than routing through
the shared onSaveStatusChange path (that path reads whichever file is
currently selected, which by the time this fires is already the file
the user navigated to, not the discarded one).
- discardedRef never cleared once set, so if the editor stayed mounted
briefly after discard (before navigation completes) and the user typed
again, every save path silently no-op'd forever for that new edit too.
It now clears itself as soon as a genuinely new edit (content diverging
from savedContent again) is observed.
* fix(files): serialize local drafts by key across mounts, not just within one
idbQueueRef was a per-instance ref, so it only ordered IndexedDB ops issued
by the same hook instance. A slow write queued by an unmount's flush lived
on as a bare promise after that instance was gone, with nothing sequencing
it against a freshly-mounted instance for the same file — its del() or
recovery read could run first, and the late write would land afterward and
resurrect a draft that was supposed to be gone.
Replaced the per-instance ref with a module-level queue keyed by draft key,
shared by every useAutosave instance (past or present) touching that key,
so ordering holds across a fast unmount+remount of the same file.
* improvement(files): consolidate autosave hook after 9 rounds of incremental fixes
Cosmetic-only pass, no behavior change:
- Hoisted MIN_SAVING_DISPLAY_MS to module scope alongside LOCAL_DRAFT_DELAY_MS
(was declared inside the hook body, re-allocated every render, inconsistent
with its sibling constant).
- Grouped the ~15 refs by concern (save/network, content mirrors, draft-key +
callbacks, local-draft persistence, discard) instead of the chronological
order they were added across nine review rounds.
- Removed a provably-dead re-check in the unmount cleanup: content/savedContent
can't change between the outer guard and the inner one (no renders happen
post-unmount), so only the discardedRef half of the inner check was live.
- Removed an unnecessary useCallback around onDiscardCorrectionFailed — its
reference is never observed by anything (useAutosave copies it into a ref
every render regardless of identity), unlike handleSaveStatusChange in
files.tsx, which is correctly memoized because it flows through
React.memo-wrapped TextEditor/RichMarkdownEditor.
Validated against external research: the two-tier debounce (network + local
IndexedDB draft) matches how Tiptap/Notion describe their own local-first
persistence; the discardedRef+corrective-save approach over AbortController
is a deliberate, justified choice (onSave has no signal parameter, and an
abort can't undo a write that's already landed server-side); the module-level
per-key promise queue is a recognized idiomatic pattern. Splitting this hook
into three smaller ones (useDebouncedSave/useLocalDraft/useDiscard) is a
legitimate future refactor, deliberately deferred given the risk of touching
this heavily-interdependent, already-hardened state this late in review.
* fix(files): serialize discard correction against newer saves, recover local drafts only once per mount
Two more real races, both interactions between earlier fixes:
- discard()'s corrective save and a genuinely new edit made right after
could race independently: if the user typed again before the correction
fired, and that correction landed after the new edit's own save, the
server would end up with the discarded baseline instead of the user's
latest content. The correction now shares the same inFlightRef/savingRef
mutual exclusion normal saves use, and skips entirely once content has
moved on to something that's neither the discarded baseline nor what it
was at the moment discard() was called.
- The local-draft recovery effect re-ran every time draftKey toggled
through enabled (e.g. autosave turning off during agent streaming and
back on once it settles), re-scanning IndexedDB as if freshly mounted.
If the settled content coincidentally matched the stale draft's stored
baseline, a pre-stream local edit could silently overwrite the agent's
work. Recovery now attempts exactly once per mount.1 parent cbe5e0f commit a3487da
7 files changed
Lines changed: 865 additions & 70 deletions
File tree
- apps/sim
- app/workspace/[workspaceId]/files
- components/file-viewer
- rich-markdown-editor
- hooks
Lines changed: 8 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
102 | | - | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
103 | 106 | | |
| 107 | + | |
104 | 108 | | |
105 | 109 | | |
106 | 110 | | |
| |||
131 | 135 | | |
132 | 136 | | |
133 | 137 | | |
| 138 | + | |
134 | 139 | | |
135 | 140 | | |
136 | 141 | | |
| |||
174 | 179 | | |
175 | 180 | | |
176 | 181 | | |
| 182 | + | |
177 | 183 | | |
178 | 184 | | |
179 | 185 | | |
| |||
193 | 199 | | |
194 | 200 | | |
195 | 201 | | |
| 202 | + | |
196 | 203 | | |
197 | 204 | | |
198 | 205 | | |
| |||
Lines changed: 14 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
48 | | - | |
| 48 | + | |
49 | 49 | | |
| 50 | + | |
50 | 51 | | |
51 | 52 | | |
52 | 53 | | |
| |||
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
| 74 | + | |
73 | 75 | | |
74 | 76 | | |
75 | 77 | | |
| |||
93 | 95 | | |
94 | 96 | | |
95 | 97 | | |
| 98 | + | |
96 | 99 | | |
97 | 100 | | |
98 | 101 | | |
| |||
356 | 359 | | |
357 | 360 | | |
358 | 361 | | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
359 | 370 | | |
360 | 371 | | |
361 | 372 | | |
| |||
407 | 418 | | |
408 | 419 | | |
409 | 420 | | |
410 | | - | |
411 | | - | |
412 | | - | |
413 | | - | |
414 | | - | |
415 | | - | |
416 | | - | |
417 | | - | |
| 421 | + | |
418 | 422 | | |
419 | 423 | | |
420 | 424 | | |
| |||
428 | 432 | | |
429 | 433 | | |
430 | 434 | | |
| 435 | + | |
431 | 436 | | |
432 | 437 | | |
433 | 438 | | |
| |||
Lines changed: 7 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
328 | 328 | | |
329 | 329 | | |
330 | 330 | | |
331 | | - | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
332 | 335 | | |
| 336 | + | |
333 | 337 | | |
334 | 338 | | |
335 | 339 | | |
| |||
345 | 349 | | |
346 | 350 | | |
347 | 351 | | |
| 352 | + | |
348 | 353 | | |
349 | 354 | | |
350 | 355 | | |
| |||
385 | 390 | | |
386 | 391 | | |
387 | 392 | | |
| 393 | + | |
388 | 394 | | |
389 | 395 | | |
390 | 396 | | |
| |||
Lines changed: 44 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
4 | 5 | | |
5 | 6 | | |
6 | 7 | | |
| |||
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
37 | | - | |
| 38 | + | |
| 39 | + | |
38 | 40 | | |
| 41 | + | |
| 42 | + | |
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
| |||
115 | 119 | | |
116 | 120 | | |
117 | 121 | | |
| 122 | + | |
118 | 123 | | |
119 | 124 | | |
120 | 125 | | |
| |||
182 | 187 | | |
183 | 188 | | |
184 | 189 | | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
190 | 200 | | |
191 | | - | |
| 201 | + | |
192 | 202 | | |
193 | 203 | | |
194 | 204 | | |
195 | | - | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
196 | 212 | | |
197 | 213 | | |
198 | 214 | | |
199 | 215 | | |
200 | 216 | | |
201 | 217 | | |
202 | 218 | | |
203 | | - | |
204 | | - | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
205 | 224 | | |
206 | 225 | | |
207 | 226 | | |
| |||
213 | 232 | | |
214 | 233 | | |
215 | 234 | | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
216 | 250 | | |
217 | 251 | | |
218 | 252 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
171 | 171 | | |
172 | 172 | | |
173 | 173 | | |
| 174 | + | |
174 | 175 | | |
175 | 176 | | |
176 | 177 | | |
| |||
970 | 971 | | |
971 | 972 | | |
972 | 973 | | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
973 | 983 | | |
974 | 984 | | |
975 | 985 | | |
| |||
1106 | 1116 | | |
1107 | 1117 | | |
1108 | 1118 | | |
| 1119 | + | |
1109 | 1120 | | |
1110 | 1121 | | |
1111 | 1122 | | |
| |||
1358 | 1369 | | |
1359 | 1370 | | |
1360 | 1371 | | |
1361 | | - | |
1362 | | - | |
1363 | | - | |
1364 | | - | |
1365 | 1372 | | |
1366 | | - | |
1367 | | - | |
1368 | | - | |
1369 | | - | |
1370 | | - | |
| 1373 | + | |
1371 | 1374 | | |
1372 | 1375 | | |
1373 | 1376 | | |
| |||
1428 | 1431 | | |
1429 | 1432 | | |
1430 | 1433 | | |
1431 | | - | |
| 1434 | + | |
1432 | 1435 | | |
1433 | 1436 | | |
1434 | 1437 | | |
1435 | | - | |
1436 | | - | |
| 1438 | + | |
1437 | 1439 | | |
1438 | 1440 | | |
1439 | 1441 | | |
1440 | 1442 | | |
1441 | | - | |
1442 | | - | |
1443 | | - | |
1444 | | - | |
1445 | | - | |
1446 | | - | |
1447 | | - | |
1448 | | - | |
1449 | | - | |
1450 | 1443 | | |
1451 | 1444 | | |
1452 | 1445 | | |
1453 | 1446 | | |
1454 | 1447 | | |
1455 | 1448 | | |
1456 | | - | |
1457 | | - | |
1458 | | - | |
1459 | | - | |
1460 | | - | |
1461 | | - | |
1462 | | - | |
1463 | | - | |
1464 | | - | |
1465 | | - | |
1466 | | - | |
1467 | | - | |
1468 | 1449 | | |
1469 | 1450 | | |
1470 | 1451 | | |
| |||
1505 | 1486 | | |
1506 | 1487 | | |
1507 | 1488 | | |
1508 | | - | |
1509 | 1489 | | |
1510 | | - | |
1511 | 1490 | | |
1512 | 1491 | | |
1513 | | - | |
1514 | 1492 | | |
1515 | 1493 | | |
1516 | 1494 | | |
| |||
1897 | 1875 | | |
1898 | 1876 | | |
1899 | 1877 | | |
1900 | | - | |
| 1878 | + | |
1901 | 1879 | | |
| 1880 | + | |
1902 | 1881 | | |
1903 | 1882 | | |
1904 | 1883 | | |
| |||
0 commit comments