authorgravatar for 153842990+yashranaway@users.noreply.github.comAditya Garud <153842990+yashranaway@users.noreply.github.com> 2026-09-02 13:10:45+05:30
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-02 00:40:45-07:00
logd2042d288eaf7aaf7feda85eb7d1a6f546302c12
tree9879e6e7c09f919062ff3c1944a1aa8729e73ab1
parent7a8df3338f910cd18543cd3ea98b8b10385e8dcd
signature Signed by PGP key B5690EEEBB952194

fix(web): avoid stale file writes on close (#8630)

2 files changed, 141 insertions(+), 1 deletions(-)

apps/web/src/components/files/fileSaveCoordinator.test.ts+137
...@@ -73,6 +73,98 @@ describe("FileSaveCoordinator", () => {...@@ -73,6 +73,98 @@ describe("FileSaveCoordinator", () => {
73 expect(onPendingChange.mock.calls.at(-1)).toEqual([false]);73 expect(onPendingChange.mock.calls.at(-1)).toEqual([false]);
74 });74 });
7575
76 it("saves an edit made inside the debounce window when the editor closes", async () => {
77 vi.useFakeTimers();
78 const persist = vi
79 .fn<(contents: string) => Promise<AtomCommandResult<void, never>>>()
80 .mockResolvedValue(AsyncResult.success(undefined));
81 const coordinator = new FileSaveCoordinator({
82 debounceMs: 500,
83 persist,
84 onPendingChange: vi.fn(),
85 onConfirmed: vi.fn(),
86 });
87
88 coordinator.change("unsaved");
89 coordinator.dispose();
90 await vi.runAllTimersAsync();
91
92 expect(persist).toHaveBeenCalledOnce();
93 expect(persist).toHaveBeenCalledWith("unsaved");
94 });
95
96 it("flushes an edit made while a write was in flight when the editor closes", async () => {
97 vi.useFakeTimers();
98 const inFlight = deferred();
99 const persist = vi
100 .fn<(contents: string) => Promise<AtomCommandResult<void, never>>>()
101 .mockReturnValueOnce(inFlight.promise)
102 .mockResolvedValue(AsyncResult.success(undefined));
103 const coordinator = new FileSaveCoordinator({
104 debounceMs: 500,
105 persist,
106 onPendingChange: vi.fn(),
107 onConfirmed: vi.fn(),
108 });
109
110 coordinator.change("first");
111 await vi.advanceTimersByTimeAsync(500);
112 coordinator.change("latest");
113 coordinator.dispose();
114 inFlight.resolve(AsyncResult.success(undefined));
115 await vi.runAllTimersAsync();
116
117 expect(persist).toHaveBeenCalledTimes(2);
118 expect(persist).toHaveBeenLastCalledWith("latest");
119 });
120
121 it("does not rewrite a write that lands while the editor closes", async () => {
122 vi.useFakeTimers();
123 const inFlight = deferred();
124 const persist = vi
125 .fn<(contents: string) => Promise<AtomCommandResult<void, never>>>()
126 .mockReturnValueOnce(inFlight.promise)
127 .mockResolvedValue(AsyncResult.success(undefined));
128 const coordinator = new FileSaveCoordinator({
129 debounceMs: 500,
130 persist,
131 onPendingChange: vi.fn(),
132 onConfirmed: vi.fn(),
133 });
134
135 coordinator.change("only");
136 await vi.advanceTimersByTimeAsync(500);
137 coordinator.dispose();
138 inFlight.resolve(AsyncResult.success(undefined));
139 await vi.runAllTimersAsync();
140
141 expect(persist).toHaveBeenCalledOnce();
142 });
143
144 it("retries a failed write when the editor closes", async () => {
145 vi.useFakeTimers();
146 const persist = vi
147 .fn()
148 .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed"))))
149 .mockResolvedValue(AsyncResult.success(undefined));
150 const coordinator = new FileSaveCoordinator({
151 debounceMs: 500,
152 persist,
153 onPendingChange: vi.fn(),
154 onConfirmed: vi.fn(),
155 });
156
157 coordinator.change("latest");
158 await vi.advanceTimersByTimeAsync(500);
159 expect(persist).toHaveBeenCalledOnce();
160
161 coordinator.dispose();
162 await vi.runAllTimersAsync();
163
164 expect(persist).toHaveBeenCalledTimes(2);
165 expect(persist).toHaveBeenLastCalledWith("latest");
166 });
167
76 it("leaves the file pending when the latest write fails", async () => {168 it("leaves the file pending when the latest write fails", async () => {
77 vi.useFakeTimers();169 vi.useFakeTimers();
78 const onPendingChange = vi.fn();170 const onPendingChange = vi.fn();
...@@ -91,4 +183,49 @@ describe("FileSaveCoordinator", () => {...@@ -91,4 +183,49 @@ describe("FileSaveCoordinator", () => {
91 expect(onPendingChange).toHaveBeenCalledWith(true);183 expect(onPendingChange).toHaveBeenCalledWith(true);
92 expect(onPendingChange).not.toHaveBeenCalledWith(false);184 expect(onPendingChange).not.toHaveBeenCalledWith(false);
93 });185 });
186
187 it("ignores editor changes emitted after disposal", async () => {
188 vi.useFakeTimers();
189 const persist = vi
190 .fn<(contents: string) => Promise<AtomCommandResult<void, never>>>()
191 .mockResolvedValue(AsyncResult.success(undefined));
192 const onPendingChange = vi.fn();
193 const coordinator = new FileSaveCoordinator({
194 debounceMs: 500,
195 persist,
196 onPendingChange,
197 onConfirmed: vi.fn(),
198 });
199
200 coordinator.dispose();
201 coordinator.change("stale contents");
202 await vi.runAllTimersAsync();
203
204 expect(persist).not.toHaveBeenCalled();
205 expect(onPendingChange).not.toHaveBeenCalled();
206 });
207
208 it("does not persist confirmed contents again on disposal", async () => {
209 vi.useFakeTimers();
210 const persist = vi
211 .fn<(contents: string) => Promise<AtomCommandResult<void, never>>>()
212 .mockResolvedValue(AsyncResult.success(undefined));
213 const coordinator = new FileSaveCoordinator({
214 debounceMs: 500,
215 persist,
216 onPendingChange: vi.fn(),
217 onConfirmed: vi.fn(),
218 });
219
220 coordinator.change("temporary edit");
221 await vi.advanceTimersByTimeAsync(500);
222 coordinator.change("original contents");
223 await vi.advanceTimersByTimeAsync(500);
224 expect(persist).toHaveBeenCalledTimes(2);
225
226 coordinator.dispose();
227 await vi.runAllTimersAsync();
228
229 expect(persist).toHaveBeenCalledTimes(2);
230 });
94});231});
apps/web/src/components/files/fileSaveCoordinator.ts+4-1
...@@ -11,6 +11,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {...@@ -11,6 +11,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {
11 private timer: ReturnType<typeof setTimeout> | null = null;11 private timer: ReturnType<typeof setTimeout> | null = null;
12 private latestContents = "";12 private latestContents = "";
13 private latestRevision = 0;13 private latestRevision = 0;
14 private confirmedRevision = 0;
14 private lastChangeAt = 0;15 private lastChangeAt = 0;
15 private saving = false;16 private saving = false;
16 private disposed = false;17 private disposed = false;
...@@ -18,6 +19,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {...@@ -18,6 +19,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {
18 constructor(private readonly options: FileSaveCoordinatorOptions<A, E>) {}19 constructor(private readonly options: FileSaveCoordinatorOptions<A, E>) {}
1920
20 change(contents: string): void {21 change(contents: string): void {
22 if (this.disposed) return;
21 this.latestContents = contents;23 this.latestContents = contents;
22 this.latestRevision += 1;24 this.latestRevision += 1;
23 this.lastChangeAt = Date.now();25 this.lastChangeAt = Date.now();
...@@ -46,7 +48,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {...@@ -46,7 +48,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {
46 }48 }
4749
48 private async persistLatest(): Promise<void> {50 private async persistLatest(): Promise<void> {
49 if (this.saving || this.latestRevision === 0) return;51 if (this.saving || this.latestRevision === this.confirmedRevision) return;
5052
51 this.saving = true;53 this.saving = true;
52 const contents = this.latestContents;54 const contents = this.latestContents;
...@@ -54,6 +56,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {...@@ -54,6 +56,7 @@ export class FileSaveCoordinator<A = unknown, E = unknown> {
54 const result = await this.options.persist(contents);56 const result = await this.options.persist(contents);
55 const succeeded = result._tag === "Success";57 const succeeded = result._tag === "Success";
56 if (succeeded) {58 if (succeeded) {
59 this.confirmedRevision = revision;
57 this.options.onConfirmed(contents);60 this.options.onConfirmed(contents);
58 }61 }
5962