From 8c5a06bbf85747308541ebed56aca6a204f182eb Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Fri, 26 Apr 2019 18:37:38 +0200 Subject: [PATCH] fix gc when item is deleted in observer call --- README.v13.md | 2 +- examples/prosemirror.html | 10 ++++++++-- examples/prosemirror.js | 11 +++++++++++ package-lock.json | 12 +++++++++--- package.json | 3 ++- src/structs/AbstractItem.js | 5 +++-- src/structs/ItemDeleted.js | 13 +++++++++++++ src/structs/ItemType.js | 9 +++++---- src/utils/Transaction.js | 22 +++++++++++----------- src/utils/Y.js | 5 +++-- tests/testHelper.js | 17 +++++------------ tsconfig.json | 4 ++-- 12 files changed, 73 insertions(+), 40 deletions(-) diff --git a/README.v13.md b/README.v13.md index ae8b1fde..316e660a 100644 --- a/README.v13.md +++ b/README.v13.md @@ -3,7 +3,7 @@ Yjs is a library for automatic conflict resolution on shared state. It implements an operation-based CRDT and exposes its internal CRDT model as shared types. Shared types are common data types like `Map` or `Array` with superpowers - changes are automatically distributed to other peers and merged without merge conflicts. -Yjs is **network agnostic** (p2p!), supports many existing **rich text editors**, **offline editing**, **version snapshots**, **shared cursors**, and encodes update messages using **binary protocol encoding**. +Yjs is **network agnostic** (p2p!), supports many existing **rich text editors**, **offline editing**, **version snapshots**, **shared cursors**, and encodes update messages using **binary protocol encoding**. * Chat: [https://gitter.im/y-js/yjs](https://gitter.im/y-js/yjs) * Demos: [https://yjs.website/tutorial-prosemirror.html](https://yjs.website/tutorial-prosemirror.html) diff --git a/examples/prosemirror.html b/examples/prosemirror.html index 7dc2629e..8647361d 100644 --- a/examples/prosemirror.html +++ b/examples/prosemirror.html @@ -76,11 +76,17 @@ img[ychange_state='removed'] { padding: 2px; } + .y-connect-btn { + position: absolute; + top: 20px; + right: 20px; + } -

This example shows how to bind a YXmlFragment type to a Prosemirror editor.

-

The content of this editor is shared with every client who visits this domain.

+ +

This example shows how to bind a YXmlFragment to a Prosemirror editor using y-prosemirror.

+

The content of this editor is shared with every client that visits this domain.

diff --git a/examples/prosemirror.js b/examples/prosemirror.js index 6edb1b0d..a3196f64 100644 --- a/examples/prosemirror.js +++ b/examples/prosemirror.js @@ -22,4 +22,15 @@ const prosemirrorView = new EditorView(document.querySelector('#editor'), { }) }) +const connectBtn = document.querySelector('.y-connect-btn') +connectBtn.addEventListener('click', () => { + if (ydocument.wsconnected) { + ydocument.disconnect() + connectBtn.textContent = 'Connect' + } else { + ydocument.connect() + connectBtn.textContent = 'Disconnect' + } +}) + window.example = { provider, ydocument, type, prosemirrorView } diff --git a/package-lock.json b/package-lock.json index 4857dc28..41c81f8c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3036,9 +3036,9 @@ } }, "lib0": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/lib0/-/lib0-0.0.1.tgz", - "integrity": "sha512-k2BL//TczDqrl+GzTp8vUkZdyO/tJaMkN3O0OcqmfROvsmmBOYvdEGuwLacLl+c7AB6qFwmdh3cdgqg6YVPn2Q==" + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/lib0/-/lib0-0.0.2.tgz", + "integrity": "sha512-7bJgV2emHGRO5kpj66Gdc9SQKVfhDBLx0UIS/aU5P8R0179nRFHKDTYGvLlNloWbeUUARlqk3ndFIO4JbUy7Sw==" }, "live-server": { "version": "1.2.1", @@ -5578,6 +5578,12 @@ "integrity": "sha1-pcbVMr5lbiPbgg77lDofBJmNY68=", "dev": true }, + "y-protocols": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/y-protocols/-/y-protocols-0.0.2.tgz", + "integrity": "sha512-ixAaywK7USrMX7h6H2PZ59rtNVZcfJCNO0+/gDhAV1Sizwxdt0T6wPm9RCxDJtY3pXNeWA8MgtBysMGkgGA5xA==", + "dev": true + }, "yallist": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/yallist/-/yallist-2.1.2.tgz", diff --git a/package.json b/package.json index 94962458..69472c87 100644 --- a/package.json +++ b/package.json @@ -55,9 +55,10 @@ }, "homepage": "http://y-js.org", "dependencies": { - "lib0": "0.0.1" + "lib0": "0.0.2" }, "devDependencies": { + "y-protocols": "0.0.2", "codemirror": "^5.42.0", "concurrently": "^3.6.1", "jsdoc": "^3.5.5", diff --git a/src/structs/AbstractItem.js b/src/structs/AbstractItem.js index f63a97a4..ace332df 100644 --- a/src/structs/AbstractItem.js +++ b/src/structs/AbstractItem.js @@ -421,13 +421,14 @@ export class AbstractItem extends AbstractStruct { /** * @param {Transaction} transaction * @param {StructStore} store + * @param {boolean} parentGCd * * @private */ - gc (transaction, store) { + gc (transaction, store, parentGCd) { this.delete(transaction) let r - if (this.parent._item !== null && this.parent._item.deleted) { + if (parentGCd) { r = new GC(this.id, this.length) } else { r = new ItemDeleted(this.id, this.left, this.origin, this.right, this.rightOrigin, this.parent, this.parentSub, this.length) diff --git a/src/structs/ItemDeleted.js b/src/structs/ItemDeleted.js index a683c6c7..c2f803e8 100644 --- a/src/structs/ItemDeleted.js +++ b/src/structs/ItemDeleted.js @@ -85,6 +85,19 @@ export class ItemDeleted extends AbstractItem { } return false } + + /** + * @param {Transaction} transaction + * @param {StructStore} store + * @param {boolean} parentGCd + * + * @private + */ + gc (transaction, store, parentGCd) { + if (parentGCd) { + super.gc(transaction, store, parentGCd) + } + } /** * @param {encoding.Encoder} encoder * @param {number} offset diff --git a/src/structs/ItemType.js b/src/structs/ItemType.js index 98503838..57ee6669 100644 --- a/src/structs/ItemType.js +++ b/src/structs/ItemType.js @@ -124,13 +124,13 @@ export class ItemType extends AbstractItem { gcChildren (transaction, store) { let item = this.type._start while (item !== null) { - item.gc(transaction, store) + item.gc(transaction, store, true) item = item.right } this.type._start = null this.type._map.forEach(item => { while (item !== null) { - item.gc(transaction, store) + item.gc(transaction, store, true) // @ts-ignore item = item.left } @@ -141,10 +141,11 @@ export class ItemType extends AbstractItem { /** * @param {Transaction} transaction * @param {StructStore} store + * @param {boolean} parentGCd */ - gc (transaction, store) { + gc (transaction, store, parentGCd) { this.gcChildren(transaction, store) - super.gc(transaction, store) + super.gc(transaction, store, parentGCd) } } diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 8189142e..26d33c7a 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -10,7 +10,6 @@ import { findIndexSS, callEventHandlerListeners, AbstractItem, - ItemDeleted, ID, AbstractType, AbstractStruct, YEvent, Y // eslint-disable-line } from '../internals.js' @@ -45,8 +44,9 @@ import * as math from 'lib0/math.js' export class Transaction { /** * @param {Y} y + * @param {any} origin */ - constructor (y) { + constructor (y, origin) { /** * The Yjs instance. * @type {Y} @@ -90,6 +90,10 @@ export class Transaction { * @private */ this._mergeStructs = new Set() + /** + * @type {any} + */ + this.origin = origin } /** * @type {encoding.Encoder|null} @@ -124,22 +128,24 @@ export const nextID = transaction => { * * @param {Y} y * @param {function(Transaction):void} f + * @param {any} [origin] * * @private * @function */ -export const transact = (y, f) => { +export const transact = (y, f, origin = null) => { const transactionCleanups = y._transactionCleanups let initialCall = false if (y._transaction === null) { initialCall = true - y._transaction = new Transaction(y) + y._transaction = new Transaction(y, origin) transactionCleanups.push(y._transaction) y.emit('beforeTransaction', [y._transaction, y]) } try { f(y._transaction) } finally { + // @todo set after state here if (initialCall && transactionCleanups[0] === y._transaction) { // The first transaction ended, now process observer calls. // Observer call may create new transactions for which we need to call the observers and do cleanup. @@ -185,13 +191,7 @@ export const transact = (y, f) => { break } if (struct.deleted && struct instanceof AbstractItem) { - if (struct.constructor !== ItemDeleted || (struct.parent._item !== null && struct.parent._item.deleted)) { - // check if we can GC - struct.gc(transaction, store) - } else { - // otherwise only gc children (if there are any) - struct.gcChildren(transaction, store) - } + struct.gc(transaction, store, false) } } } diff --git a/src/utils/Y.js b/src/utils/Y.js index df3f28e3..20fea267 100644 --- a/src/utils/Y.js +++ b/src/utils/Y.js @@ -52,11 +52,12 @@ export class Y extends Observable { * other peers. * * @param {function(Transaction):void} f The function that should be executed as a transaction + * @param {any} [origin] Origin of who started the transaction. Will be stored on transaction.origin * * @public */ - transact (f) { - transact(this, f) + transact (f, origin = null) { + transact(this, f, origin) } /** * Define a shared data type. diff --git a/tests/testHelper.js b/tests/testHelper.js index 17d1cffd..54dd1172 100644 --- a/tests/testHelper.js +++ b/tests/testHelper.js @@ -19,14 +19,14 @@ import * as syncProtocol from 'y-protocols/sync.js' * @param {TestYInstance} y */ const afterTransaction = (transaction, y) => { - y.mMux(() => { + if (transaction.origin !== y.tc) { const m = transaction.updateMessage if (m !== null) { const encoder = encoding.createEncoder() syncProtocol.writeUpdate(encoder, m) broadcastMessage(y, encoding.toBuffer(encoder)) } - }) + } } /** @@ -59,11 +59,6 @@ export class TestYInstance extends Y.Y { * @type {Map>} */ this.receiving = new Map() - /** - * Message mutex - * @type {Function} - */ - this.mMux = createMutex() testConnector.allConns.add(this) // set up observe on local model this.on('afterTransactionCleanup', afterTransaction) @@ -165,11 +160,9 @@ export class TestConnector { return this.flushRandomMessage() } const encoder = encoding.createEncoder() - receiver.mMux(() => { - // console.log('receive (' + sender.userID + '->' + receiver.userID + '):\n', syncProtocol.stringifySyncMessage(decoding.createDecoder(m), receiver)) - // do not publish data created when this function is executed (could be ss2 or update message) - syncProtocol.readSyncMessage(decoding.createDecoder(m), encoder, receiver) - }) + // console.log('receive (' + sender.userID + '->' + receiver.userID + '):\n', syncProtocol.stringifySyncMessage(decoding.createDecoder(m), receiver)) + // do not publish data created when this function is executed (could be ss2 or update message) + syncProtocol.readSyncMessage(decoding.createDecoder(m), encoder, receiver, receiver.tc) if (encoding.length(encoder) > 0) { // send reply message sender._receive(encoding.toBuffer(encoder), receiver) diff --git a/tsconfig.json b/tsconfig.json index d7f1fa24..28864acd 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -54,9 +54,9 @@ /* Experimental Options */ // "experimentalDecorators": true, /* Enables experimental support for ES7 decorators. */ // "emitDecoratorMetadata": true, /* Enables experimental support for emitting type metadata for decorators. */ - "maxNodeModuleJsDepth": 5, + "maxNodeModuleJsDepth": 0, // "types": ["./src/utils/typedefs.js"] }, "include": ["./src/**/*", "./tests/**/*"], - "exclude": ["../lib0/**/*", "node_modules"] + "exclude": ["../lib0/**/*", "node_modules/**/*", "dist", "dist/**/*.js"] }