New types dont fire events - fixes #155

This commit is contained in:
Kevin Jahns 2019-06-18 17:41:19 +02:00
parent 1faff323c1
commit 1aac245b93
4 changed files with 55 additions and 14 deletions

View File

@ -120,7 +120,6 @@ export class ContentType {
} }
}) })
transaction.changed.delete(this.type) transaction.changed.delete(this.type)
transaction.changedParentTypes.delete(this.type)
} }
/** /**
* @param {StructStore} store * @param {StructStore} store

View File

@ -22,6 +22,7 @@ import {
readContentEmbed, readContentEmbed,
readContentFormat, readContentFormat,
readContentType, readContentType,
addChangedTypeToTransaction,
ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line ContentType, ContentDeleted, StructStore, ID, AbstractType, Transaction // eslint-disable-line
} from '../internals.js' } from '../internals.js'
@ -237,7 +238,8 @@ export class Item extends AbstractStruct {
} }
addStruct(store, this) addStruct(store, this)
this.content.integrate(transaction, this) this.content.integrate(transaction, this)
maplib.setIfUndefined(transaction.changed, parent, set.create).add(parentSub) // add parent to transaction.changed
addChangedTypeToTransaction(transaction, parent, parentSub)
if ((parent._item !== null && parent._item.deleted) || (this.right !== null && parentSub !== null)) { if ((parent._item !== null && parent._item.deleted) || (this.right !== null && parentSub !== null)) {
// delete if parent is deleted or if this is not the current attribute value of parent // delete if parent is deleted or if this is not the current attribute value of parent
this.delete(transaction) this.delete(transaction)

View File

@ -16,6 +16,7 @@ import {
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
import * as map from 'lib0/map.js' import * as map from 'lib0/map.js'
import * as math from 'lib0/math.js' import * as math from 'lib0/math.js'
import * as set from 'lib0/set.js'
/** /**
* A transaction is created for every change on the Yjs model. It is possible * A transaction is created for every change on the Yjs model. It is possible
@ -117,6 +118,21 @@ export const nextID = transaction => {
return createID(y.clientID, getState(y.store, y.clientID)) return createID(y.clientID, getState(y.store, y.clientID))
} }
/**
* If `type.parent` was added in current transaction, `type` technically
* did not change, it was just added and we should not fire events for `type`.
*
* @param {Transaction} transaction
* @param {AbstractType<YEvent>} type
* @param {string|null} parentSub
*/
export const addChangedTypeToTransaction = (transaction, type, parentSub) => {
const item = type._item
if (item === null || (item.id.clock < (transaction.beforeState.get(item.id.client) || 0) && !item.deleted)) {
map.setIfUndefined(transaction.changed, type, set.create).add(parentSub)
}
}
/** /**
* Implements the functionality of `y.transact(()=>{..})` * Implements the functionality of `y.transact(()=>{..})`
* *
@ -153,20 +169,26 @@ export const transact = (doc, f, origin = null) => {
doc.emit('beforeObserverCalls', [transaction, doc]) doc.emit('beforeObserverCalls', [transaction, doc])
// emit change events on changed types // emit change events on changed types
transaction.changed.forEach((subs, itemtype) => { transaction.changed.forEach((subs, itemtype) => {
itemtype._callObserver(transaction, subs) if (itemtype._item === null || !itemtype._item.deleted) {
itemtype._callObserver(transaction, subs)
}
}) })
transaction.changedParentTypes.forEach((events, type) => { transaction.changedParentTypes.forEach((events, type) => {
events = events // We need to think about the possibility that the user transforms the
.filter(event => // Y.Doc in the event.
event.target._item === null || !event.target._item.deleted if (type._item === null || !type._item.deleted) {
) events = events
events .filter(event =>
.forEach(event => { event.target._item === null || !event.target._item.deleted
event.currentTarget = type )
}) events
// we don't need to check for events.length .forEach(event => {
// because we know it has at least one element event.currentTarget = type
callEventHandlerListeners(type._dEH, events, transaction) })
// We don't need to check for events.length
// because we know it has at least one element
callEventHandlerListeners(type._dEH, events, transaction)
}
}) })
doc.emit('afterTransaction', [transaction, doc]) doc.emit('afterTransaction', [transaction, doc])
/** /**

View File

@ -210,6 +210,24 @@ export const testInsertAndDeleteEventsForTypes2 = tc => {
compare(users) compare(users)
} }
/**
* This issue has been reported here https://github.com/y-js/yjs/issues/155
* @param {t.TestCase} tc
*/
export const testNewChildDoesNotEmitEventInTransaction = tc => {
const { array0, users } = init(tc, { users: 2 })
let fired = false
users[0].transact(() => {
const newMap = new Y.Map()
newMap.observe(() => {
fired = true
})
array0.insert(0, [newMap])
newMap.set('tst', 42)
})
t.assert(!fired, 'Event does not trigger')
}
/** /**
* @param {t.TestCase} tc * @param {t.TestCase} tc
*/ */