fix(sync): no conflicts with one's own text, one copy per conflict
- Leaving the tab sent pending text with keepalive but kept it queued; typing on folded it with newer keystrokes under the old base version, which then conflicted with the user's own text. The flushed batch now becomes the batch in flight. - After a conflict, the focused field still showed the losing text and every further batch made another copy. Typing on goes into the one copy until the cursor leaves; on leaving, the field shows the block's current text, and entering a block bases edits on what it shows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
08d35a4a81
commit
c021946c7f
3 files changed
+118
-12
No files matched your search
@@ -358,6 +358,58 @@ describe('DocSyncEngine', () => {
|
||||
expect(contents(engine)).toEqual(['Schreibe']);
|
||||
});
|
||||
|
||||
it('typing on after leaving the tab causes no conflict with oneself', async () => {
|
||||
const server = new FakeServer();
|
||||
const engine = await startEngine(new FakeNet(server));
|
||||
engine.focusBlock('b1');
|
||||
edit(engine, (b) => [{ ...b[0], content: 'Start und' }]);
|
||||
// Tab hidden (window switch, app switch on the phone): pending text goes out with keepalive
|
||||
(engine as any).flushOnExit();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
expect(serverContents(server)).toEqual(['Start und']);
|
||||
|
||||
// Back in the tab, typing on in the same block
|
||||
edit(engine, (b) => [{ ...b[0], content: 'Start und weiter' }]);
|
||||
await engine.syncNow();
|
||||
edit(engine, (b) => [{ ...b[0], content: 'Start und weiter geht es' }]);
|
||||
await engine.syncNow();
|
||||
|
||||
expect(engine.getState().conflicts).toEqual([]);
|
||||
expect(serverContents(server)).toEqual(['Start und weiter geht es']);
|
||||
});
|
||||
|
||||
it('typing on in a block after a conflict updates the one conflict copy', async () => {
|
||||
const server = new FakeServer();
|
||||
const a = await startEngine(new FakeNet(server));
|
||||
const b = await startEngine(new FakeNet(server));
|
||||
|
||||
a.focusBlock('b1');
|
||||
edit(b, (blocks) => [{ ...blocks[0], content: 'B hat geändert' }]);
|
||||
await b.syncNow();
|
||||
await a.syncNow(); // the focused field keeps showing A's own text
|
||||
|
||||
a.applyFlat([{ id: 'b1', type: 'bullet', content: 'Start + A', level: 0 }]);
|
||||
await a.syncNow(); // conflict: A's text becomes a copy
|
||||
// A keeps typing in the same field, which still shows A's text
|
||||
a.applyFlat([{ ...flatOf(a)[0], content: 'Start + A tippt' }, flatOf(a)[1]]);
|
||||
await a.syncNow();
|
||||
a.applyFlat([{ ...flatOf(a)[0], content: 'Start + A tippt weiter' }, flatOf(a)[1]]);
|
||||
await a.syncNow();
|
||||
await b.syncNow();
|
||||
|
||||
expect(a.getState().conflicts).toHaveLength(1);
|
||||
expect(serverContents(server)).toEqual(['B hat geändert', 'Start + A tippt weiter']);
|
||||
expect(contents(b)).toEqual(['B hat geändert', 'Start + A tippt weiter']);
|
||||
|
||||
// Once the cursor left, the field shows B's version again: editing it changes the block itself
|
||||
a.focusBlock(null);
|
||||
a.focusBlock('b1');
|
||||
edit(a, (blocks) => [{ ...blocks[0], content: 'B hat geändert, A ergänzt' }, blocks[1]]);
|
||||
await a.syncNow();
|
||||
expect(a.getState().conflicts).toHaveLength(1);
|
||||
expect(serverContents(server)).toEqual(['B hat geändert, A ergänzt', 'Start + A tippt weiter']);
|
||||
});
|
||||
|
||||
it('viewers cannot change anything', async () => {
|
||||
const server = new FakeServer();
|
||||
const viewer = await startEngine(new FakeNet(server, 'viewer'));
|
||||
|
||||
@@ -140,6 +140,10 @@ export class DocSyncEngine {
|
||||
// Version of each block the editor last showed in agreement with the engine. The focused text field does
|
||||
// not take over remote changes while typing, so its edits are based on this version, not the newest one.
|
||||
private uiSeen = new Map<string, number>();
|
||||
// Focused block -> its conflict copy: the field still shows the user's losing text, so typing on there
|
||||
// goes into that copy instead of making a new copy per keystroke batch. Ends when the field shows the
|
||||
// engine's text again or the cursor leaves the block.
|
||||
private conflictRedirects = new Map<string, string>();
|
||||
private undoStack: UndoStep[] = [];
|
||||
private redoStack: UndoStep[] = [];
|
||||
|
||||
@@ -267,15 +271,19 @@ export class DocSyncEngine {
|
||||
}
|
||||
for (const b of blocks) {
|
||||
const cur = this.local.blocks.get(b.id);
|
||||
if (cur && !cur.deleted && cur.content === (b.content || '')) this.uiSeen.set(b.id, cur.version);
|
||||
if (cur && !cur.deleted && cur.content === (b.content || '')) {
|
||||
this.uiSeen.set(b.id, cur.version);
|
||||
this.conflictRedirects.delete(b.id);
|
||||
}
|
||||
}
|
||||
const ops = diffFlat(this.local, blocks, () => this.newOpId());
|
||||
if (ops.length === 0) return;
|
||||
const before = this.local;
|
||||
const t = this.now();
|
||||
for (const op of ops) {
|
||||
if ((op.kind === 'insert' || op.kind === 'set_content') && op.blockId) this.activity.set(op.blockId, t);
|
||||
if (op.kind === 'set_content' && this.uiSeen.has(op.blockId!)) op.baseVersion = this.uiSeen.get(op.blockId!);
|
||||
if (op.kind === 'set_content') this.redirectToConflictCopy(op, this.local);
|
||||
if ((op.kind === 'insert' || op.kind === 'set_content') && op.blockId) this.activity.set(op.blockId, t);
|
||||
}
|
||||
this.enqueue(ops);
|
||||
this.record(before, ops);
|
||||
@@ -350,7 +358,12 @@ export class DocSyncEngine {
|
||||
const t = this.now();
|
||||
const changed = blockId !== this.focused;
|
||||
if (this.focused) this.activity.set(this.focused, t);
|
||||
if (changed && this.focused) this.conflictRedirects.delete(this.focused);
|
||||
this.focused = blockId;
|
||||
// Entering a block, its field shows the engine's text (unfocused fields take it over), so edits from
|
||||
// here on build on that version, not on one an earlier visit of the field saw
|
||||
const entered = changed && blockId ? this.local.blocks.get(blockId) : undefined;
|
||||
if (entered && !entered.deleted) this.uiSeen.set(entered.id, entered.version);
|
||||
if (blockId) this.activity.set(blockId, t);
|
||||
// Announce the lease right away: with push the next regular sync may be up to 30 s away
|
||||
if (changed && blockId && this.canEdit && this.collaborative()) {
|
||||
@@ -387,7 +400,7 @@ export class DocSyncEngine {
|
||||
? this.status === 'error'
|
||||
? this.pollDelay()
|
||||
: this.nextBackoff()
|
||||
: this.syncAgain || this.pending.length > 0
|
||||
: this.syncAgain || this.pending.length > 0 || this.inflight
|
||||
? 0
|
||||
: this.pollDelay();
|
||||
this.syncAgain = false;
|
||||
@@ -462,6 +475,19 @@ export class DocSyncEngine {
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
/** Sends a text change into the conflict copy of its block (see conflictRedirects). */
|
||||
private redirectToConflictCopy(op: Op, doc: Doc, copyId = this.conflictRedirects.get(op.blockId!)): void {
|
||||
if (!copyId) return;
|
||||
const copy = doc.blocks.get(copyId);
|
||||
if (!copy || copy.deleted || copy.props.conflictOf !== op.blockId) {
|
||||
// Resolved or discarded meanwhile: a new change is a new conflict
|
||||
this.conflictRedirects.delete(op.blockId!);
|
||||
return;
|
||||
}
|
||||
op.blockId = copyId;
|
||||
op.baseVersion = copy.version;
|
||||
}
|
||||
|
||||
private recompute(): void {
|
||||
const v = cloneDoc(this.confirmed);
|
||||
// Pending ops count the way they will be sent: formBatch folds all text changes of a block into one,
|
||||
@@ -537,20 +563,27 @@ export class DocSyncEngine {
|
||||
this.pending = [];
|
||||
await this.persist();
|
||||
}
|
||||
const ops = this.canEdit ? this.inflight || [] : [];
|
||||
if (ops.length > 0) this.status = 'saving';
|
||||
// flushOnExit may start a new batch while this request is on the wire; only this one is answered here
|
||||
let batch = this.canEdit ? this.inflight : null;
|
||||
if (batch && batch.length > 0) this.status = 'saving';
|
||||
|
||||
for (;;) {
|
||||
const res = await this.transport.sync(this.docId, {
|
||||
clientId: this.clientId,
|
||||
lastSeq: this.lastSeq,
|
||||
ops: this.inflight ? ops : [],
|
||||
ops: batch || [],
|
||||
leases: this.canEdit && this.collaborative() ? { hold: this.holds(), release: [] } : undefined,
|
||||
});
|
||||
if (this.inflight) {
|
||||
const sent = new Map(this.inflight.map((op) => [op.opId, op]));
|
||||
const lostTo = new Map<string, string>(); // block -> conflict copy of its losing text
|
||||
if (batch) {
|
||||
const sent = new Map(batch.map((op) => [op.opId, op]));
|
||||
for (const r of res.results) {
|
||||
if (r.status === 'conflict') this.conflicts = [...this.conflicts, r];
|
||||
const conflicted = sent.get(r.opId);
|
||||
if (r.status === 'conflict' && r.conflictBlockId && conflicted?.kind === 'set_content') {
|
||||
lostTo.set(conflicted.blockId!, r.conflictBlockId);
|
||||
if (this.focused === conflicted.blockId) this.conflictRedirects.set(conflicted.blockId!, r.conflictBlockId);
|
||||
}
|
||||
// Someone else created a block with the same ID first (e.g. the first block of an empty note):
|
||||
// keep the typed text by sending it as a regular text change of that block
|
||||
const op = sent.get(r.opId);
|
||||
@@ -559,7 +592,8 @@ export class DocSyncEngine {
|
||||
this.pending.unshift({ opId: this.newOpId(), kind: 'set_content', blockId: op.blockId, content: op.content, baseVersion: 0 });
|
||||
}
|
||||
}
|
||||
this.inflight = null;
|
||||
if (this.inflight === batch) this.inflight = null;
|
||||
batch = null;
|
||||
}
|
||||
this.role = res.role;
|
||||
this.shared = res.shared;
|
||||
@@ -581,6 +615,11 @@ export class DocSyncEngine {
|
||||
this.lastSeq = entry.seq;
|
||||
}
|
||||
if (gap) await this.loadSnapshot();
|
||||
// Text typed into the same field while the losing batch was on the wire builds on the losing text
|
||||
// and belongs into the copy too
|
||||
for (const op of this.pending) {
|
||||
if (op.kind === 'set_content' && lostTo.has(op.blockId!)) this.redirectToConflictCopy(op, this.confirmed, lostTo.get(op.blockId!));
|
||||
}
|
||||
await this.persist();
|
||||
if (!res.hasMore || gap) break;
|
||||
}
|
||||
@@ -690,7 +729,16 @@ export class DocSyncEngine {
|
||||
/** Sends what is pending with keepalive (survives page unload) and releases leases. */
|
||||
private flushOnExit(): void {
|
||||
if (this.canEdit && (this.inflight || this.pending.length > 0)) {
|
||||
const ops = this.inflight || formBatch(this.pending);
|
||||
// The batch becomes the one in flight: the next sync resends exactly these ops (same IDs, applied
|
||||
// once), and text typed afterwards starts a new batch on top instead of being folded into this one
|
||||
// with its older base version, which would conflict with the user's own text
|
||||
if (!this.inflight) {
|
||||
this.inflight = formBatch(this.pending);
|
||||
this.pending = [];
|
||||
this.recompute();
|
||||
void this.persist();
|
||||
}
|
||||
const ops = this.inflight;
|
||||
this.transport
|
||||
.sync(this.docId, { clientId: this.clientId, lastSeq: this.lastSeq, ops }, { keepalive: true })
|
||||
.catch(() => {
|
||||
|
||||
Reference in new issue
Block a user