diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index b9a2f0980b..997ab3ae6d 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -417,6 +417,24 @@ class Entry { return result } + /** Take over the edits of a deployed entry this one replaced at its key: each field its value + * changed from its own deployed side, so they stay a draft over what this entry deployed. */ + carryEditsOf(old: Entry): void { + if (old.origin !== 'deployed' || !old.dirty || this.value === undefined) return + const edited = old.value as Record + const base = old.deployed as Record + const next = snapshot(this.value) as Record + let changed = false + for (const k of new Set([...Object.keys(edited), ...Object.keys(base)])) { + if (deepEqual(edited[k], base[k])) continue + next[k] = snapshot(edited[k]) + changed = true + } + if (!changed) return + this.pristine = false + this.replaceValue(next as V) + } + /** Give the value each field the server set otherwise than `sent` asked, unless the user has * changed that field since: left as sent, it would read as a draft of a change nobody made. */ private adopt(sent: V, held: V): void { @@ -811,10 +829,11 @@ export function createItemStore(ports: ItemRowPort) { /** * An entry moved onto a key another live entry holds. One key has one row writer, so the - * old one stops writing and every handle on it moves to the entry that now is the item — - * the same outcome as when nobody had it open: what was there is superseded by the write. + * old one stops writing and every handle on it moves to the entry that now is the item. Its + * deployed side is superseded by the write; the edits on screen over it are not. */ function retire(old: Entry, into: Entry): void { + into.carryEditsOf(old) old.retired = true old.stopWatch?.() for (const handle of old.handles) { @@ -996,6 +1015,8 @@ export function useItems( ): (ItemHandle | undefined)[] { const handles = $state<(ItemHandle | undefined)[]>([]) const held = new Map>() + const sessionOf = new Map, string>() + const idOf = (key: ItemKey, session: unknown) => `${keyString(key)}#${String(session ?? '')}` function reconcile() { const specs = getSpecs() @@ -1006,14 +1027,26 @@ export function useItems( continue } const key: ItemKey = { workspace: spec.workspace, kind, path: spec.path } - wanted.push({ id: `${keyString(key)}#${String(spec.session ?? '')}`, key, spec }) + wanted.push({ id: idOf(key, spec.session), key, spec }) } const ids = new Set(wanted.map((w) => w.id)) + // A spec that caught up with where a save moved its item keeps that item, rather than + // releasing it and reading it afresh over edits whose row may not have landed yet. + for (const { id } of wanted) { + if (!id || held.has(id)) continue + for (const [heldId, acq] of held) { + if (ids.has(heldId) || idOf(acq.handle.key, sessionOf.get(acq)) !== id) continue + held.delete(heldId) + held.set(id, acq) + break + } + } // Release first, so an item reopened under a new session is read afresh when idle. for (const [id, acq] of [...held]) { if (!ids.has(id)) { acq.release() held.delete(id) + sessionOf.delete(acq) } } const next = wanted.map(({ id, key, spec }) => { @@ -1022,6 +1055,7 @@ export function useItems( if (!acq) { acq = itemStore.acquire(key, spec, adapter) held.set(id, acq) + sessionOf.set(acq, String(spec.session ?? '')) } return acq.handle }) @@ -1038,6 +1072,7 @@ export function useItems( onDestroy(() => { for (const acq of held.values()) acq.release() held.clear() + sessionOf.clear() }) return handles } diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index cbf24eb7d4..75e4660479 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -500,6 +500,36 @@ describe('item store: one entry per key', () => { ]) }) + it('keeps what was typed in an editor a move replaces, as a draft over what it wrote', async () => { + const rows = fakeRows() + const store = createItemStore(rows.port) + const gate = deferred() + const b = { ...deployedRes, path: 'u/me/b' } + const { handle: open } = store.acquire( + { workspace: 'w', kind: 'resource', path: 'u/me/b' }, + { workspace: 'w', path: 'u/me/b' }, + adapter({ deployed: b }) + ) + const temporary = newItemPath() + const { handle: moving } = store.acquire( + { workspace: 'w', kind: 'resource', path: temporary }, + { workspace: 'w', path: temporary, template: b }, + adapter({}, () => gate.promise) + ) + await settle() + moving.value = { ...b, args: { a: 2 } } + const moved = moving.save() + await settle() + open.value = { ...b, description: 'typed while it moved' } + + gate.resolve() + expect(await moved).toMatchObject({ ok: true, moved: true }) + const kept = { ...b, args: { a: 2 }, description: 'typed while it moved' } + expect(open.deployed).toEqual({ ...b, args: { a: 2 } }) + expect(open.value).toEqual(kept) + expect(rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: kept }) + }) + it('writes an item it moves onto after the saves queued there, and supersedes later ones', async () => { const rows = fakeRows() const store = createItemStore(rows.port) @@ -617,7 +647,9 @@ describe('item store: one entry per key', () => { expect(await secondMove).toMatchObject({ ok: true, moved: true }) expect(await later).toMatchObject({ ok: false }) expect(order).toEqual(['first', 'second']) - expect(first.value?.description).toBe('second') + expect(first.deployed?.description).toBe('second') + // Not written, but not lost either: a draft over what the later move wrote. + expect(first.value?.description).toBe('saved again through the first') }) })