fix(collab): replicate document content to joiners

Squashed PR #340 with maintainer follow-up fixes and local validation.
This commit is contained in:
rcoenen 2026-06-30 04:56:58 -04:00 committed by GitHub
parent 3a277c7712
commit d2d816b688
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 447 additions and 94 deletions

View file

@ -1,6 +1,8 @@
import type { Canvas, Paint } from 'canvaskit-wasm'
import type { SceneGraph } from '@open-pencil/scene-graph'
import { getWorldMatrix } from '@open-pencil/scene-graph/coordinate'
import Matrix from '@open-pencil/scene-graph/matrix'
import type { Vector } from '@open-pencil/scene-graph/primitives'
import { PEN_HANDLE_RADIUS, PEN_VERTEX_RADIUS, PEN_CLOSE_RADIUS_BOOST } from '#core/constants'
@ -218,12 +220,28 @@ export function drawRemoteCursors(
for (const nodeId of cursor.selection) {
const node = graph.getNode(nodeId)
if (!node) continue
const abs = graph.getAbsolutePosition(nodeId)
const sx = abs.x * r.zoom + r.panX
const sy = abs.y * r.zoom + r.panY
const sw = node.width * r.zoom
const sh = node.height * r.zoom
canvas.drawRect(r.ck.XYWHRect(sx, sy, sw, sh), r.auxStroke)
// Map the node's four corners through its world matrix (the same transform
// the shape is rendered with) so a remote peer's selection box tracks the
// node exactly, including rotation, flips, and any parent transforms.
const m = getWorldMatrix(node, graph)
const c = Matrix.mapPoints(m, [
0,
0,
node.width,
0,
node.width,
node.height,
0,
node.height
])
const box = new r.ck.Path()
box.moveTo(c[0] * r.zoom + r.panX, c[1] * r.zoom + r.panY)
for (let i = 2; i < c.length; i += 2) {
box.lineTo(c[i] * r.zoom + r.panX, c[i + 1] * r.zoom + r.panY)
}
box.close()
canvas.drawPath(box, r.auxStroke)
box.delete()
}
}

View file

@ -262,23 +262,14 @@ export class SceneGraph {
height: node?.height ?? 0
}
}
private generateNodeId(): string {
let id = generateId()
while (this.nodes.has(id)) id = generateId()
return id
}
createNode(type: NodeType, parentId: string, overrides: Partial<SceneNode> = {}): SceneNode {
const node = createDefaultNode(() => this.generateNodeId(), type, overrides)
private registerNode(node: SceneNode, parentId: string | null): SceneNode {
node.parentId = parentId
this.nodes.set(node.id, node)
const parent = this.nodes.get(parentId)
if (parent) {
parent.childIds.push(node.id)
}
if (node.type === 'INSTANCE' && node.componentId) {
let set = this.instanceIndex.get(node.componentId)
if (!set) {
@ -287,10 +278,26 @@ export class SceneGraph {
}
set.add(node.id)
}
this.emitter.emit('node:created', node)
return node
}
createNode(type: NodeType, parentId: string, overrides: Partial<SceneNode> = {}): SceneNode {
const node = createDefaultNode(() => this.generateNodeId(), type, overrides)
this.nodes.get(parentId)?.childIds.push(node.id)
return this.registerNode(node, parentId)
}
createNodeWithId(
id: string,
type: NodeType,
parentId: string | null,
overrides: Partial<SceneNode> = {}
): SceneNode {
const node = createDefaultNode(() => id, type, overrides)
node.id = id
const parent = parentId ? this.nodes.get(parentId) : undefined
if (parent && !parent.childIds.includes(id)) parent.childIds.push(id)
return this.registerNode(node, parentId)
}
static TEXT_PICTURE_KEYS: ReadonlySet<string> = TEXT_PICTURE_KEYS

View file

@ -3,7 +3,6 @@ import * as Y from 'yjs'
import type { SceneNode } from '@open-pencil/scene-graph'
import type { EditorStore } from '@/app/editor/active-store'
import { YJS_JSON_FIELDS } from '@/constants'
type YNodes = Y.Map<Y.Map<unknown>>
type YImages = Y.Map<Uint8Array>
@ -34,31 +33,22 @@ type YjsGraphSyncOptions = {
setSuppressYjsEvents: (value: boolean) => void
}
function logCollabSyncError(context: string, error: unknown) {
console.error(`[Collab] ${context}:`, error)
}
// Clone across the graph/Yjs boundary to avoid shared mutable nested data.
export function syncNodePropsToYMap(node: SceneNode, ynode: Y.Map<unknown>) {
for (const [key, value] of Object.entries(node)) {
if (typeof value === 'object' && value !== null) {
ynode.set(key, JSON.stringify(value))
} else {
ynode.set(key, value)
}
ynode.set(key, structuredClone(value))
}
}
export function yNodeToProps(ynode: Y.Map<unknown>): Record<string, unknown> {
const props: Record<string, unknown> = {}
for (const [key, value] of ynode.entries()) {
if (YJS_JSON_FIELDS.has(key)) {
try {
props[key] = typeof value === 'string' ? JSON.parse(value) : value
} catch {
props[key] = value
}
} else {
props[key] = value
}
props[key] = structuredClone(value)
}
return props
}
@ -86,10 +76,15 @@ export function bindCollabGraphEvents({
const ynodes = getYnodes()
if (!getSuppressGraphSync() && ydoc && ynodes) {
setSuppressYjsEvents(true)
ydoc.transact(() => {
ynodes.delete(id)
})
setSuppressYjsEvents(false)
try {
ydoc.transact(() => {
ynodes.delete(id)
})
} catch (error) {
logCollabSyncError('Failed to delete synced node', error)
} finally {
setSuppressYjsEvents(false)
}
}
})
]
@ -111,23 +106,29 @@ export function registerYjsObservers({
setSuppressGraphSync(true)
try {
applyYjsToGraph(events)
store.requestRender()
} catch (error) {
logCollabSyncError('Failed to apply remote graph changes', error)
} finally {
setSuppressGraphSync(false)
}
store.requestRender()
})
yimages.observe((event) => {
if (getSuppressYjsEvents()) return
for (const [key, change] of event.changes.keys) {
if (change.action === 'add' || change.action === 'update') {
const data = yimages.get(key)
if (data) store.graph.images.set(key, new Uint8Array(data))
} else {
store.graph.images.delete(key)
try {
for (const [key, change] of event.changes.keys) {
if (change.action === 'add' || change.action === 'update') {
const data = yimages.get(key)
if (data) store.graph.images.set(key, new Uint8Array(data))
} else {
store.graph.images.delete(key)
}
}
store.requestRender()
} catch (error) {
logCollabSyncError('Failed to apply remote image changes', error)
}
store.requestRender()
})
}
@ -148,24 +149,29 @@ export function createYjsGraphSync({
const localYimages = getYimages()
setSuppressYjsEvents(true)
ydoc.transact(() => {
let ynode = ynodes.get(nodeId)
if (!ynode) {
ynode = new Y.Map()
ynodes.set(nodeId, ynode)
}
syncNodePropsToYMap(node, ynode)
try {
ydoc.transact(() => {
let ynode = ynodes.get(nodeId)
if (!ynode) {
ynode = new Y.Map()
ynodes.set(nodeId, ynode)
}
syncNodePropsToYMap(node, ynode)
if (localYimages) {
for (const fill of node.fills) {
if (fill.imageHash && !localYimages.has(fill.imageHash)) {
const data = store.graph.images.get(fill.imageHash)
if (data) localYimages.set(fill.imageHash, data)
if (localYimages) {
for (const fill of node.fills) {
if (fill.imageHash && !localYimages.has(fill.imageHash)) {
const data = store.graph.images.get(fill.imageHash)
if (data) localYimages.set(fill.imageHash, data)
}
}
}
}
})
setSuppressYjsEvents(false)
})
} catch (error) {
logCollabSyncError(`Failed to sync node ${nodeId}`, error)
} finally {
setSuppressYjsEvents(false)
}
}
function syncAllNodesToYjs() {
@ -175,26 +181,31 @@ export function createYjsGraphSync({
if (!ydoc || !ynodes) return
const localYimages = getYimages()
setSuppressYjsEvents(true)
ydoc.transact(() => {
for (const node of store.graph.getAllNodes()) {
let ynode = ynodes.get(node.id)
if (!ynode) {
ynode = new Y.Map()
ynodes.set(node.id, ynode)
}
syncNodePropsToYMap(node, ynode)
}
})
if (localYimages) {
try {
ydoc.transact(() => {
for (const [hash, data] of store.graph.images) {
if (!localYimages.has(hash)) {
localYimages.set(hash, data)
for (const node of store.graph.getAllNodes()) {
let ynode = ynodes.get(node.id)
if (!ynode) {
ynode = new Y.Map()
ynodes.set(node.id, ynode)
}
syncNodePropsToYMap(node, ynode)
}
})
if (localYimages) {
ydoc.transact(() => {
for (const [hash, data] of store.graph.images) {
if (!localYimages.has(hash)) {
localYimages.set(hash, data)
}
}
})
}
} catch (error) {
logCollabSyncError('Failed to sync document', error)
} finally {
setSuppressYjsEvents(false)
}
setSuppressYjsEvents(false)
}
function applyYjsToGraph(events: Y.YEvent<Y.Map<unknown>>[]) {
@ -234,20 +245,28 @@ export function createYjsGraphSync({
const store = getStore()
const existing = store.graph.getNode(nodeId)
const props = yNodeToProps(ynode)
const parentId = typeof props.parentId === 'string' ? props.parentId : null
if (existing) {
store.graph.updateNode(nodeId, props as Partial<SceneNode>)
if (parentId === null) store.graph.rootId = nodeId
ensureCurrentPageExists(store)
return
}
const parentId = props.parentId as string
if (parentId && store.graph.getNode(parentId)) {
const type = props.type as SceneNode['type']
const node = store.graph.createNode(type, parentId, props as Partial<SceneNode>)
store.graph.nodes.delete(node.id)
node.id = nodeId
store.graph.nodes.set(nodeId, node)
}
const type = props.type as SceneNode['type'] | undefined
if (!type) return
// Parent childIds may arrive before or after the child node.
store.graph.createNodeWithId(nodeId, type, parentId, props as Partial<SceneNode>)
if (parentId === null) store.graph.rootId = nodeId
ensureCurrentPageExists(store)
}
function ensureCurrentPageExists(store: EditorStore) {
const pages = store.graph.getPages()
if (pages.some((page) => page.id === store.state.currentPageId)) return
if (pages.length === 0) return
void store.switchPage(pages[0].id)
}
return { syncNodeToYjs, syncAllNodesToYjs, applyYjsToGraph }

View file

@ -76,16 +76,6 @@ export const PEER_COLORS: Color[] = [
{ r: 0.91, g: 0.12, b: 0.39, a: 1 }
]
export const YJS_JSON_FIELDS = new Set([
'childIds',
'fills',
'strokes',
'effects',
'vectorNetwork',
'boundVariables',
'styleRuns'
])
export {
DEFAULT_SHAPE_FILL,
DEFAULT_FRAME_FILL,

View file

@ -0,0 +1,302 @@
import { describe, test, expect } from 'bun:test'
import * as Y from 'yjs'
import type { Fill, GeometryPath, SceneNode } from '@open-pencil/scene-graph'
import { SceneGraph } from '@open-pencil/scene-graph'
import { nodeVisualBounds } from '@open-pencil/scene-graph/geometry'
import {
createYjsGraphSync,
registerYjsObservers,
syncNodePropsToYMap,
yNodeToProps
} from '@/app/collab/yjs-sync'
import { createEditorStore } from '@/app/editor/session'
import { expectDefined, getNodeOrThrow } from '#tests/helpers/assert'
import { connectYDocs } from '#tests/helpers/yjs'
// Test copy of the private apply path.
function applyYnodeToGraph(peer: SceneGraph, nodeId: string, ynode: Y.Map<unknown>) {
const props = yNodeToProps(ynode)
if (peer.getNode(nodeId)) {
peer.updateNode(nodeId, props as Partial<SceneNode>)
return
}
const type = props.type as SceneNode['type'] | undefined
if (!type) return
const parentId = typeof props.parentId === 'string' ? props.parentId : null
peer.createNodeWithId(nodeId, type, parentId, props as Partial<SceneNode>)
if (parentId === null) peer.rootId = nodeId
}
function seedHostIntoYjs(host: SceneGraph): Y.Map<Y.Map<unknown>> {
const doc = new Y.Doc()
const ynodes = doc.getMap<Y.Map<unknown>>('nodes')
doc.transact(() => {
for (const node of host.getAllNodes()) {
const ynode = new Y.Map<unknown>()
ynodes.set(node.id, ynode)
syncNodePropsToYMap(node, ynode)
}
})
return ynodes
}
function firstPage(graph: SceneGraph): SceneNode {
return expectDefined(graph.getPages()[0], 'first page')
}
type SyncedStores = ReturnType<typeof createSyncedStores>
function createSyncedStores() {
const hostStore = createEditorStore(new SceneGraph())
const peerStore = createEditorStore(new SceneGraph())
const hostDoc = new Y.Doc()
const peerDoc = new Y.Doc()
const hostNodes = hostDoc.getMap<Y.Map<unknown>>('nodes')
const peerNodes = peerDoc.getMap<Y.Map<unknown>>('nodes')
const hostImages = hostDoc.getMap<Uint8Array>('images')
const peerImages = peerDoc.getMap<Uint8Array>('images')
let hostSuppressYjsEvents = false
let peerSuppressYjsEvents = false
let hostSuppressGraphSync = false
let peerSuppressGraphSync = false
const hostSync = createYjsGraphSync({
getStore: () => hostStore,
getYdoc: () => hostDoc,
getYnodes: () => hostNodes,
getYimages: () => hostImages,
setSuppressYjsEvents: (value) => {
hostSuppressYjsEvents = value
}
})
const peerSync = createYjsGraphSync({
getStore: () => peerStore,
getYdoc: () => peerDoc,
getYnodes: () => peerNodes,
getYimages: () => peerImages,
setSuppressYjsEvents: (value) => {
peerSuppressYjsEvents = value
}
})
registerYjsObservers({
store: hostStore,
ynodes: hostNodes,
yimages: hostImages,
getSuppressYjsEvents: () => hostSuppressYjsEvents,
setSuppressGraphSync: (value) => {
hostSuppressGraphSync = value
},
applyYjsToGraph: hostSync.applyYjsToGraph
})
registerYjsObservers({
store: peerStore,
ynodes: peerNodes,
yimages: peerImages,
getSuppressYjsEvents: () => peerSuppressYjsEvents,
setSuppressGraphSync: (value) => {
peerSuppressGraphSync = value
},
applyYjsToGraph: peerSync.applyYjsToGraph
})
const disconnectYDocs = connectYDocs(hostDoc, peerDoc)
return {
hostStore,
peerStore,
hostSync,
peerSync,
get hostSuppressGraphSync() {
return hostSuppressGraphSync
},
get peerSuppressGraphSync() {
return peerSuppressGraphSync
},
cleanup: () => {
disconnectYDocs()
hostDoc.destroy()
peerDoc.destroy()
}
}
}
function withSyncedStores(run: (stores: SyncedStores) => void) {
const stores = createSyncedStores()
try {
run(stores)
} finally {
stores.cleanup()
}
}
describe('collab yjs-sync', () => {
test('createNodeWithId forces the requested id even if synced props contain a stale id', () => {
const graph = new SceneGraph()
const page = firstPage(graph)
const node = graph.createNodeWithId('remote-id', 'RECTANGLE', page.id, {
id: 'stale-local-id',
width: 50
})
expect(node.id).toBe('remote-id')
expect(graph.getNode('remote-id')).toBe(node)
expect(graph.getNode('stale-local-id')).toBeUndefined()
expect(page.childIds).toContain('remote-id')
})
test('binary geometry fields round-trip as Uint8Array, not strings', () => {
const host = new SceneGraph()
const page = firstPage(host)
const blob = new Uint8Array([1, 2, 3, 250])
const geometry: GeometryPath[] = [{ windingRule: 'NONZERO', commandsBlob: blob }]
const ellipse = host.createNode('ELLIPSE', page.id, {
width: 100,
height: 100,
fillGeometry: geometry
})
const doc = new Y.Doc()
const ynode = new Y.Map<unknown>()
doc.getMap<Y.Map<unknown>>('nodes').set(ellipse.id, ynode)
syncNodePropsToYMap(ellipse, ynode)
blob[0] = 99
const props = yNodeToProps(ynode)
expect(typeof ynode.get('fillGeometry')).not.toBe('string')
const decoded = props.fillGeometry as GeometryPath[]
expect(Array.isArray(decoded)).toBe(true)
const commandsBlob = expectDefined(decoded[0], 'decoded geometry path').commandsBlob
expect(commandsBlob).toBeInstanceOf(Uint8Array)
expect(Array.from(commandsBlob)).toEqual([1, 2, 3, 250])
expect(Array.isArray(props.strokeGeometry)).toBe(true)
})
test('a fresh peer reconstructs one ellipse, no duplicate childIds, order-independent', () => {
const host = new SceneGraph()
const hostPage = firstPage(host)
const ellipse = host.createNode('ELLIPSE', hostPage.id, { width: 80, height: 60 })
const ynodes = seedHostIntoYjs(host)
const peer = new SceneGraph()
const ids = [...ynodes.keys()].reverse()
for (const id of ids) applyYnodeToGraph(peer, id, expectDefined(ynodes.get(id), `ynode ${id}`))
const peerEllipse = getNodeOrThrow(peer, ellipse.id)
expect(peerEllipse.type).toBe('ELLIPSE')
expect(peerEllipse.parentId).toBe(hostPage.id)
const peerPage = getNodeOrThrow(peer, hostPage.id)
const refs = peerPage.childIds.filter((c) => c === ellipse.id)
expect(refs).toHaveLength(1)
expect(peer.getPages().map((page) => page.id)).toContain(hostPage.id)
})
test('a live-created node links into its parent even when the parent childIds was not re-synced', () => {
const host = new SceneGraph()
const hostPage = firstPage(host)
const rect = host.createNode('RECTANGLE', hostPage.id, { width: 50, height: 50 })
const doc = new Y.Doc()
const ynodes = doc.getMap<Y.Map<unknown>>('nodes')
doc.transact(() => {
const pageYnode = new Y.Map<unknown>()
ynodes.set(hostPage.id, pageYnode)
syncNodePropsToYMap({ ...hostPage, childIds: [] } as SceneNode, pageYnode)
const rectYnode = new Y.Map<unknown>()
ynodes.set(rect.id, rectYnode)
syncNodePropsToYMap(rect, rectYnode)
})
const peer = new SceneGraph()
applyYnodeToGraph(peer, hostPage.id, expectDefined(ynodes.get(hostPage.id), 'page ynode'))
applyYnodeToGraph(peer, rect.id, expectDefined(ynodes.get(rect.id), 'rect ynode'))
const peerPage = getNodeOrThrow(peer, hostPage.id)
expect(peerPage.childIds).toEqual([rect.id])
expect(getNodeOrThrow(peer, rect.id).type).toBe('RECTANGLE')
})
test('syncAllNodesToYjs populates peer graph and current page', () => {
withSyncedStores(({ hostStore, peerStore, hostSync }) => {
const hostPage = firstPage(hostStore.graph)
const rect = hostStore.graph.createNode('RECTANGLE', hostPage.id, { width: 80, height: 60 })
hostSync.syncAllNodesToYjs()
expect(peerStore.graph.rootId).toBe(hostStore.graph.rootId)
expect(peerStore.state.currentPageId).toBe(hostPage.id)
expect(peerStore.graph.getPages().map((page) => page.id)).toContain(hostPage.id)
expect(getNodeOrThrow(peerStore.graph, rect.id).type).toBe('RECTANGLE')
})
})
test('live-created and edited nodes sync in both directions', () => {
withSyncedStores(({ hostStore, peerStore, hostSync, peerSync }) => {
const hostPage = firstPage(hostStore.graph)
hostSync.syncAllNodesToYjs()
const rect = hostStore.graph.createNode('RECTANGLE', hostPage.id, { width: 50, height: 50 })
hostSync.syncNodeToYjs(rect.id)
const peerRect = getNodeOrThrow(peerStore.graph, rect.id)
expect(peerRect.parentId).toBe(hostPage.id)
expect(getNodeOrThrow(peerStore.graph, hostPage.id).childIds).toContain(rect.id)
peerStore.graph.updateNode(rect.id, { x: 42, y: 24 })
peerSync.syncNodeToYjs(rect.id)
expect(getNodeOrThrow(hostStore.graph, rect.id).x).toBe(42)
expect(getNodeOrThrow(hostStore.graph, rect.id).y).toBe(24)
})
})
test('image fills sync image bytes', () => {
withSyncedStores(({ hostStore, peerStore, hostSync }) => {
const hostPage = firstPage(hostStore.graph)
const imageHash = 'image-hash'
const imageFill: Fill = {
type: 'IMAGE',
color: { r: 0, g: 0, b: 0, a: 1 },
opacity: 1,
visible: true,
imageHash,
imageScaleMode: 'FILL'
}
const rect = hostStore.graph.createNode('RECTANGLE', hostPage.id, { fills: [imageFill] })
hostStore.graph.images.set(imageHash, new Uint8Array([9, 8, 7]))
hostSync.syncNodeToYjs(rect.id)
expect(
Array.from(expectDefined(peerStore.graph.images.get(imageHash), 'peer image'))
).toEqual([9, 8, 7])
})
})
test('synced node does not crash the visual-bounds helper', () => {
const host = new SceneGraph()
const hostPage = firstPage(host)
const ellipse = host.createNode('ELLIPSE', hostPage.id, { width: 120, height: 90 })
const ynodes = seedHostIntoYjs(host)
const peer = new SceneGraph()
for (const id of ynodes.keys()) {
applyYnodeToGraph(peer, id, expectDefined(ynodes.get(id), `ynode ${id}`))
}
const peerEllipse = getNodeOrThrow(peer, ellipse.id)
expect(() =>
nodeVisualBounds(peerEllipse, (id) => {
const n = peer.getNode(id)
return { x: n?.x ?? 0, y: n?.y ?? 0 }
})
).not.toThrow()
})
})

17
tests/helpers/yjs.ts Normal file
View file

@ -0,0 +1,17 @@
import * as Y from 'yjs'
export function connectYDocs(left: Y.Doc, right: Y.Doc): () => void {
const syncLeftToRight = (update: Uint8Array) => Y.applyUpdate(right, update)
const syncRightToLeft = (update: Uint8Array) => Y.applyUpdate(left, update)
left.on('update', syncLeftToRight)
right.on('update', syncRightToLeft)
Y.applyUpdate(right, Y.encodeStateAsUpdate(left))
Y.applyUpdate(left, Y.encodeStateAsUpdate(right))
return () => {
left.off('update', syncLeftToRight)
right.off('update', syncRightToLeft)
}
}