Procházet zdrojové kódy

Allowed 2-way binding with additional 1-way chain (3 collections).

Aleksander Nowodzinski před 8 roky
rodič
revize
f89cc6c6fd

+ 28 - 8
packages/ckeditor5-utils/src/collection.js

@@ -57,14 +57,21 @@ export default class Collection {
 		this._idProperty = options && options.idProperty || 'id';
 		this._idProperty = options && options.idProperty || 'id';
 
 
 		/**
 		/**
-		 * A helper mapping external items from bound collection ({@link #bindTo})
-		 * and actual items of the collection.
+		 * A helper mapping external items of a bound collection ({@link #bindTo})
+		 * and actual items of this collection.
 		 *
 		 *
 		 * @protected
 		 * @protected
 		 * @member {Map}
 		 * @member {Map}
 		 */
 		 */
 		this._bindToExternalToInternalMap = new Map();
 		this._bindToExternalToInternalMap = new Map();
-		this._bindToInternalToExternalMap = new Map();
+
+		/**
+		 * A collection instance this collection is bound to as a result
+		 * of calling {@link #bindTo} method.
+		 *
+		 * @protected
+		 * @member {module:utils/collection~Collection} #_bindToCollection
+		 */
 	}
 	}
 
 
 	/**
 	/**
@@ -398,6 +405,12 @@ export default class Collection {
 		};
 		};
 	}
 	}
 
 
+	/**
+	 * Finalizes and activates a binding initiated by {#bindTo}.
+	 *
+	 * @protected
+	 * @param {Function} factory
+	 */
 	_setUpBindToBinding( factory ) {
 	_setUpBindToBinding( factory ) {
 		const externalCollection = this._bindToCollection;
 		const externalCollection = this._bindToCollection;
 
 
@@ -405,14 +418,22 @@ export default class Collection {
 		//
 		//
 		// @private
 		// @private
 		const addItem = ( evt, externalItem, index ) => {
 		const addItem = ( evt, externalItem, index ) => {
-			const item = factory( externalItem );
-
-			if ( externalCollection._bindToInternalToExternalMap.has( externalItem ) ) {
+			const isExternalBoundToThis = externalCollection._bindToCollection == this;
+			const isExternalItemBound = [
+					...externalCollection._bindToExternalToInternalMap.values()
+				].indexOf( externalItem ) > -1;
+
+			// If an external collection is bound to this collection, which makes it a 2–way binding,
+			// and the particular external collection item is already bound, don't add it here.
+			// The external item has been created **out of this collection's item** and (re)adding it will
+			// cause a loop.
+			if ( isExternalBoundToThis && isExternalItemBound ) {
 				return;
 				return;
 			}
 			}
 
 
+			const item = factory( externalItem );
+
 			this._bindToExternalToInternalMap.set( externalItem, item );
 			this._bindToExternalToInternalMap.set( externalItem, item );
-			this._bindToInternalToExternalMap.set( item, externalItem );
 			this.add( item, index );
 			this.add( item, index );
 		};
 		};
 
 
@@ -429,7 +450,6 @@ export default class Collection {
 			const item = this._bindToExternalToInternalMap.get( externalItem );
 			const item = this._bindToExternalToInternalMap.get( externalItem );
 
 
 			this._bindToExternalToInternalMap.delete( externalItem );
 			this._bindToExternalToInternalMap.delete( externalItem );
-			this._bindToInternalToExternalMap.delete( item );
 			this.remove( item );
 			this.remove( item );
 		} );
 		} );
 	}
 	}

+ 53 - 4
packages/ckeditor5-utils/tests/collection.js

@@ -727,7 +727,7 @@ describe( 'Collection', () => {
 		} );
 		} );
 
 
 		describe( 'two–way data binding', () => {
 		describe( 'two–way data binding', () => {
-			it( 'works #1', () => {
+			it( 'works (custom factories)', () => {
 				const collectionA = new Collection();
 				const collectionA = new Collection();
 				const collectionB = new Collection();
 				const collectionB = new Collection();
 
 
@@ -738,7 +738,7 @@ describe( 'Collection', () => {
 				const spyB = sinon.spy();
 				const spyB = sinon.spy();
 
 
 				collectionA.on( 'add', spyA );
 				collectionA.on( 'add', spyA );
-				collectionA.on( 'add', spyB );
+				collectionB.on( 'add', spyB );
 
 
 				collectionA.bindTo( collectionB ).using( 'data' );
 				collectionA.bindTo( collectionB ).using( 'data' );
 				collectionB.bindTo( collectionA ).as( FactoryClass );
 				collectionB.bindTo( collectionA ).as( FactoryClass );
@@ -760,7 +760,7 @@ describe( 'Collection', () => {
 				sinon.assert.callCount( spyB, 3 );
 				sinon.assert.callCount( spyB, 3 );
 			} );
 			} );
 
 
-			it( 'works #2', () => {
+			it( 'works (1:1)', () => {
 				const collectionA = new Collection();
 				const collectionA = new Collection();
 				const collectionB = new Collection();
 				const collectionB = new Collection();
 
 
@@ -771,7 +771,7 @@ describe( 'Collection', () => {
 				const spyB = sinon.spy();
 				const spyB = sinon.spy();
 
 
 				collectionA.on( 'add', spyA );
 				collectionA.on( 'add', spyA );
-				collectionA.on( 'add', spyB );
+				collectionB.on( 'add', spyB );
 
 
 				collectionA.bindTo( collectionB ).using( i => i );
 				collectionA.bindTo( collectionB ).using( i => i );
 				collectionB.bindTo( collectionA ).using( i => i );
 				collectionB.bindTo( collectionA ).using( i => i );
@@ -792,6 +792,55 @@ describe( 'Collection', () => {
 				sinon.assert.callCount( spyA, 3 );
 				sinon.assert.callCount( spyA, 3 );
 				sinon.assert.callCount( spyB, 3 );
 				sinon.assert.callCount( spyB, 3 );
 			} );
 			} );
+
+			it( 'works (double chain)', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+				const collectionC = new Collection();
+
+				collectionA.name = 'A';
+				collectionB.name = 'B';
+				collectionC.name = 'C';
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+				const spyC = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+				collectionC.on( 'add', spyC );
+
+				// A<--->B--->C
+				collectionA.bindTo( collectionB ).using( i => i );
+				collectionB.bindTo( collectionA ).using( i => i );
+				collectionC.bindTo( collectionB ).using( i => i );
+
+				collectionA.add( new FactoryClass( 'foo' ) );
+				collectionA.add( new FactoryClass( 'bar' ) );
+
+				expect( collectionA.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar' ], 'CollectionA' );
+				expect( collectionB.map( i => i ).every( i => i instanceof FactoryClass ) ).to.be.true;
+				expect( collectionB.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar' ], 'CollectionB' );
+				expect( collectionC.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar' ], 'CollectionC' );
+
+				collectionB.add( new FactoryClass( 'baz' ) );
+
+				expect( collectionA.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz' ], 'CollectionA' );
+				expect( collectionB.map( i => i ).every( i => i instanceof FactoryClass ) ).to.be.true;
+				expect( collectionB.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz' ], 'CollectionB' );
+				expect( collectionC.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz' ], 'CollectionC' );
+
+				collectionC.add( new FactoryClass( 'qux' ) );
+
+				expect( collectionA.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz' ], 'CollectionA' );
+				expect( collectionB.map( i => i ).every( i => i instanceof FactoryClass ) ).to.be.true;
+				expect( collectionB.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz' ], 'CollectionB' );
+				expect( collectionC.map( i => i.data ) ).to.deep.equal( [ 'foo', 'bar', 'baz', 'qux' ], 'CollectionC' );
+
+				sinon.assert.callCount( spyA, 3 );
+				sinon.assert.callCount( spyB, 3 );
+				sinon.assert.callCount( spyC, 4 );
+			} );
 		} );
 		} );
 	} );
 	} );