Skip to content

Commit 25f9c9f

Browse files
ntietje1ntietje1
committed
address comments
1 parent 017a469 commit 25f9c9f

8 files changed

Lines changed: 63 additions & 86 deletions

File tree

‎convex/yjsSync/__tests__/makeYjsUpdate.helper.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Y from 'yjs'
2+
3+
export function makeYjsUpdate(): ArrayBuffer {
4+
const doc = new Y.Doc()
5+
doc.getXmlFragment('document')
6+
const update = Y.encodeStateAsUpdate(doc)
7+
const ab = update.buffer.slice(
8+
update.byteOffset,
9+
update.byteOffset + update.byteLength,
10+
) as ArrayBuffer
11+
doc.destroy()
12+
return ab
13+
}

‎convex/yjsSync/__tests__/yjsSyncHelpers.test.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { describe, expect, it } from 'vitest'
2-
import * as Y from 'yjs'
32
import { createTestContext } from '../../_test/setup.helper'
43
import { asDm, setupCampaignContext } from '../../_test/identities.helper'
54
import { createNote } from '../../_test/factories.helper'
@@ -8,18 +7,7 @@ import { shouldCompact } from '../functions/compactUpdates'
87
import { uint8ToArrayBuffer } from '../functions/uint8ToArrayBuffer'
98
import { createYjsDocument } from '../functions/createYjsDocument'
109
import { deleteYjsDocument } from '../functions/deleteYjsDocument'
11-
12-
function makeYjsUpdate(): ArrayBuffer {
13-
const doc = new Y.Doc()
14-
doc.getXmlFragment('document')
15-
const update = Y.encodeStateAsUpdate(doc)
16-
const ab = update.buffer.slice(
17-
update.byteOffset,
18-
update.byteOffset + update.byteLength,
19-
) as ArrayBuffer
20-
doc.destroy()
21-
return ab
22-
}
10+
import { makeYjsUpdate } from './makeYjsUpdate.helper'
2311

2412
describe('shouldCompact', () => {
2513
it('returns false for seq 0', () => {

‎convex/yjsSync/__tests__/yjsSyncInternalMutations.test.ts‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,8 @@ import { createTestContext } from '../../_test/setup.helper'
44
import { setupCampaignContext } from '../../_test/identities.helper'
55
import { createNote } from '../../_test/factories.helper'
66
import { internal } from '../../_generated/api'
7-
8-
function makeYjsUpdate(): ArrayBuffer {
9-
const doc = new Y.Doc()
10-
doc.getXmlFragment('document')
11-
const update = Y.encodeStateAsUpdate(doc)
12-
const ab = update.buffer.slice(
13-
update.byteOffset,
14-
update.byteOffset + update.byteLength,
15-
) as ArrayBuffer
16-
doc.destroy()
17-
return ab
18-
}
7+
import { AWARENESS_TTL_MS } from '../constants'
8+
import { makeYjsUpdate } from './makeYjsUpdate.helper'
199

2010
describe('compact', () => {
2111
const t = createTestContext()
@@ -182,7 +172,7 @@ describe('cleanupStaleAwareness', () => {
182172
clientId: 1,
183173
userId: ctx.dm.profile._id,
184174
state: new ArrayBuffer(4),
185-
updatedAt: Date.now() - 31000,
175+
updatedAt: Date.now() - (AWARENESS_TTL_MS + 1000),
186176
})
187177
})
188178

@@ -210,7 +200,7 @@ describe('cleanupStaleAwareness', () => {
210200
clientId: 1,
211201
userId: ctx.dm.profile._id,
212202
state: new ArrayBuffer(4),
213-
updatedAt: Date.now() - 5000,
203+
updatedAt: Date.now() - Math.floor(AWARENESS_TTL_MS / 2),
214204
})
215205
})
216206

@@ -238,7 +228,7 @@ describe('cleanupStaleAwareness', () => {
238228
clientId: 1,
239229
userId: ctx.dm.profile._id,
240230
state: new ArrayBuffer(4),
241-
updatedAt: Date.now() - 31000,
231+
updatedAt: Date.now() - (AWARENESS_TTL_MS + 1000),
242232
})
243233
})
244234

@@ -248,7 +238,7 @@ describe('cleanupStaleAwareness', () => {
248238
clientId: 2,
249239
userId: ctx.dm.profile._id,
250240
state: new ArrayBuffer(4),
251-
updatedAt: Date.now() - 5000,
241+
updatedAt: Date.now() - Math.floor(AWARENESS_TTL_MS / 2),
252242
})
253243
})
254244

‎convex/yjsSync/__tests__/yjsSyncMutations.test.ts‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { describe, expect, it, vi } from 'vitest'
2-
import * as Y from 'yjs'
32
import { createTestContext } from '../../_test/setup.helper'
43
import {
54
asDm,
@@ -13,18 +12,7 @@ import {
1312
expectPermissionDenied,
1413
} from '../../_test/assertions.helper'
1514
import { api } from '../../_generated/api'
16-
17-
function makeEmptyYjsUpdate(): ArrayBuffer {
18-
const doc = new Y.Doc()
19-
doc.getXmlFragment('document')
20-
const update = Y.encodeStateAsUpdate(doc)
21-
const ab = update.buffer.slice(
22-
update.byteOffset,
23-
update.byteOffset + update.byteLength,
24-
) as ArrayBuffer
25-
doc.destroy()
26-
return ab
27-
}
15+
import { makeYjsUpdate as makeEmptyYjsUpdate } from './makeYjsUpdate.helper'
2816

2917
function makeAwarenessState(): ArrayBuffer {
3018
return new Uint8Array([1, 2, 3, 4]).buffer
@@ -597,7 +585,7 @@ describe('persistBlocks', () => {
597585
.filter((q) => q.eq(q.field('noteId'), noteId))
598586
.collect()
599587

600-
expect(blocks.length).toBeGreaterThanOrEqual(0)
588+
expect(blocks.length).toBe(0)
601589
})
602590
})
603591
})

‎convex/yjsSync/__tests__/yjsSyncQueries.test.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { describe, expect, it } from 'vitest'
2-
import * as Y from 'yjs'
32
import { createTestContext } from '../../_test/setup.helper'
43
import {
54
asDm,
@@ -12,18 +11,7 @@ import {
1211
expectPermissionDenied,
1312
} from '../../_test/assertions.helper'
1413
import { api } from '../../_generated/api'
15-
16-
function makeEmptyYjsUpdate(): ArrayBuffer {
17-
const doc = new Y.Doc()
18-
doc.getXmlFragment('document')
19-
const update = Y.encodeStateAsUpdate(doc)
20-
const ab = update.buffer.slice(
21-
update.byteOffset,
22-
update.byteOffset + update.byteLength,
23-
) as ArrayBuffer
24-
doc.destroy()
25-
return ab
26-
}
14+
import { makeYjsUpdate as makeEmptyYjsUpdate } from './makeYjsUpdate.helper'
2715

2816
describe('getUpdates', () => {
2917
const t = createTestContext()

‎e2e/yjs-collaboration.spec.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ test.describe.serial('yjs collaboration', () => {
8080
await page1.keyboard.press('Home')
8181
await page1.keyboard.type(textA)
8282

83+
await page1.waitForTimeout(100)
84+
8385
await editor2.click()
84-
await page2.keyboard.press('Home')
86+
await page2.keyboard.press('End')
8587
await page2.keyboard.type(textB)
8688

8789
await expect(editor1).toContainText(textA, { timeout: 15000 })
@@ -251,7 +253,7 @@ test.describe.serial('yjs collaboration', () => {
251253
await page1.keyboard.press('Enter')
252254
await page1.keyboard.type(duringOffline)
253255

254-
await page1.waitForTimeout(2000)
256+
await expect(editor1).toContainText(duringOffline, { timeout: 5000 })
255257

256258
await context2.setOffline(false)
257259

‎src/features/editor/hooks/useConvexYjsCollaboration.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function useConvexYjsCollaboration(
6565
useEffect(() => {
6666
if (updatesResult.data && state) {
6767
state.provider.applyRemoteUpdates(updatesResult.data)
68-
if (afterSeq === undefined && updatesResult.data.length > 0) {
68+
if (updatesResult.data.length > 0) {
6969
setAfterSeq(state.provider.lastAppliedSeq)
7070
}
7171
if (isLoading) setIsLoading(false)

‎src/features/editor/providers/convex-yjs-provider.ts‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -147,35 +147,40 @@ export class ConvexYjsProvider extends ObservableV2<ProviderEvents> {
147147

148148
destroy() {
149149
if (this.destroyed) return
150+
this.destroyed = true
150151

151152
this.stopPersistInterval()
152153
this.clearAwarenessTimer()
153154

154-
if (this._writable) {
155-
this.flushUpdates()
156-
this.persist()
157-
}
158-
159-
this.flushAwareness()
155+
const teardown = async () => {
156+
if (this._writable) {
157+
await this.flushUpdates()
158+
this.persist()
159+
}
160160

161-
this.destroyed = true
161+
await this.flushAwareness()
162162

163-
this.config.removeAwareness({
164-
documentId: this.documentId,
165-
clientId: this.doc.clientID,
166-
})
163+
this.config
164+
.removeAwareness({
165+
documentId: this.documentId,
166+
clientId: this.doc.clientID,
167+
})
168+
.catch(() => {})
169+
}
167170

168-
removeAwarenessStates(
169-
this.awareness,
170-
[this.doc.clientID],
171-
'local-disconnect',
172-
)
171+
teardown().finally(() => {
172+
removeAwarenessStates(
173+
this.awareness,
174+
[this.doc.clientID],
175+
'local-disconnect',
176+
)
173177

174-
this.doc.off('update', this.handleDocUpdate)
175-
this.awareness.off('update', this.handleAwarenessUpdate)
176-
this.awareness.destroy()
178+
this.doc.off('update', this.handleDocUpdate)
179+
this.awareness.off('update', this.handleAwarenessUpdate)
180+
this.awareness.destroy()
177181

178-
super.destroy()
182+
super.destroy()
183+
})
179184
}
180185

181186
private persist() {
@@ -214,10 +219,10 @@ export class ConvexYjsProvider extends ObservableV2<ProviderEvents> {
214219
}
215220
}
216221

217-
private flushUpdates() {
222+
private flushUpdates(): Promise<void> {
218223
this.clearUpdateTimers()
219-
if (this.pushInFlight) return
220-
if (this.pendingUpdates.length === 0) return
224+
if (this.pushInFlight) return Promise.resolve()
225+
if (this.pendingUpdates.length === 0) return Promise.resolve()
221226

222227
const merged =
223228
this.pendingUpdates.length === 1
@@ -226,7 +231,7 @@ export class ConvexYjsProvider extends ObservableV2<ProviderEvents> {
226231
this.pendingUpdates = []
227232

228233
this.pushInFlight = true
229-
this.config
234+
return this.config
230235
.pushUpdate({
231236
documentId: this.documentId,
232237
update: uint8ToArrayBuffer(merged),
@@ -236,6 +241,8 @@ export class ConvexYjsProvider extends ObservableV2<ProviderEvents> {
236241
})
237242
.catch((err: unknown) => {
238243
console.error('[YJS] push failed for', this.documentId, err)
244+
this.pendingUpdates.unshift(merged)
245+
this.scheduleFlush()
239246
})
240247
.finally(() => {
241248
this.pushInFlight = false
@@ -301,20 +308,21 @@ export class ConvexYjsProvider extends ObservableV2<ProviderEvents> {
301308
}
302309
}
303310

304-
private flushAwareness() {
311+
private flushAwareness(): Promise<void> {
305312
this.clearAwarenessTimer()
306-
if (this.awarenessInFlight || this.destroyed) return
313+
if (this.awarenessInFlight) return Promise.resolve()
307314

308315
const myClientId = this.doc.clientID
309316
const encoded = encodeAwarenessUpdate(this.awareness, [myClientId])
310317

311318
this.awarenessInFlight = true
312-
this.config
319+
return this.config
313320
.pushAwareness({
314321
documentId: this.documentId,
315322
clientId: myClientId,
316323
state: uint8ToArrayBuffer(encoded),
317324
})
325+
.then(() => {})
318326
.catch((err: unknown) => {
319327
console.error('[YJS] awareness push failed for', this.documentId, err)
320328
})

0 commit comments

Comments
 (0)
⚡