Explorar o código

Made View and ViewCollection classes synchronous.

Aleksander Nowodzinski %!s(int64=8) %!d(string=hai) anos
pai
achega
b7332235c8

+ 23 - 32
packages/ckeditor5-ui/src/view.js

@@ -43,10 +43,10 @@ import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
  *
  *		const view = new SampleView( locale );
  *
- *		view.init().then( () => {
- *			// Will append <p class="foo">Hello<b>world</b></p>
- *			document.body.appendChild( view.element );
- *		} );
+ *		view.init();
+ *
+ *		// Will append <p class="foo">Hello<b>world</b></p>
+ *		document.body.appendChild( view.element );
  *
  * @mixes module:utils/observablemixin~ObservableMixin
  */
@@ -190,14 +190,14 @@ export default class View {
 	 *		const view = new SampleView( locale );
 	 *		const anotherView = new AnotherSampleView( locale );
 	 *
-	 *		view.init().then( () => {
-	 *			// Will append <p></p>
-	 *			document.body.appendChild( view.element );
+	 *		view.init();
+	 *
+	 *		// Will append <p></p>
+	 *		document.body.appendChild( view.element );
 	 *
-	 *			// `anotherView` becomes a child of the view, which is reflected in DOM
-	 *			// <p><anotherView#element></p>
-	 *			view.items.add( anotherView );
-	 *		} );
+	 *		// `anotherView` becomes a child of the view, which is reflected in DOM
+	 *		// <p><anotherView#element></p>
+	 *		view.items.add( anotherView );
 	 *
 	 * @returns {module:ui/viewcollection~ViewCollection} A new collection of view instances.
 	 */
@@ -237,10 +237,10 @@ export default class View {
 	 *
 	 *		const view = new SampleView( locale );
 	 *
-	 *		view.init().then( () => {
-	 *			// Will append <p><childA#element><b></b><childB#element></p>
-	 *			document.body.appendChild( view.element );
-	 *		} );
+	 *		view.init();
+	 *
+	 *		// Will append <p><childA#element><b></b><childB#element></p>
+	 *		document.body.appendChild( view.element );
 	 *
 	 * **Note**: There's no need to add child views if they're used in the
 	 * {@link #template} explicitly:
@@ -265,20 +265,17 @@ export default class View {
 	 *		}
 	 *
 	 * @param {module:ui/view~View|Iterable.<module:ui/view~View>} children Children views to be registered.
-	 * @returns {Promise}
 	 */
 	addChildren( children ) {
 		if ( !isIterable( children ) ) {
 			children = [ children ];
 		}
 
-		return Promise.all( children.map( c => this._unboundChildren.add( c ) ) );
+		children.map( c => this._unboundChildren.add( c ) );
 	}
 
 	/**
 	 * Initializes the view and child views located in {@link #_viewCollections}.
-	 *
-	 * @returns {Promise} A Promise resolved when the initialization process is finished.
 	 */
 	init() {
 		if ( this.ready ) {
@@ -290,26 +287,20 @@ export default class View {
 			throw new CKEditorError( 'ui-view-init-reinit: This View has already been initialized.' );
 		}
 
-		return Promise.resolve()
-			// Initialize collections in #_viewCollections.
-			.then( () => {
-				return Promise.all( this._viewCollections.map( c => c.init() ) );
-			} )
-			// Spread the word that this view is ready!
-			.then( () => {
-				this.ready = true;
-			} );
+		// Initialize collections in #_viewCollections.
+		this._viewCollections.map( c => c.init() );
+
+		// Spread the word that this view is ready!
+		this.ready = true;
 	}
 
 	/**
-	 * Destroys the view instance and child views located in {@link #_viewCollections}.
-	 *
-	 * @returns {Promise} A Promise resolved when the destruction process is finished.
+f	 * Destroys the view instance and child views located in {@link #_viewCollections}.
 	 */
 	destroy() {
 		this.stopListening();
 
-		return Promise.all( this._viewCollections.map( c => c.destroy() ) );
+		this._viewCollections.map( c => c.destroy() );
 	}
 
 	/**

+ 5 - 40
packages/ckeditor5-ui/src/viewcollection.js

@@ -69,23 +69,11 @@ export default class ViewCollection extends Collection {
 		 * @member {HTMLElement}
 		 */
 		this._parentElement = null;
-
-		/**
-		 * A set containing promises created by {@link #add}. If the {@link #destroy} is called
-		 * before the child views' {@link module:ui/view~View#init} are completed,
-		 * {@link #destroy} will wait until all the promises are resolved.
-		 *
-		 * @private
-		 * @member {Set}
-		 */
-		this._addPromises = new Set();
 	}
 
 	/**
 	 * Initializes child views by injecting {@link module:ui/view~View#element} into DOM
 	 * and calling {@link module:ui/view~View#init}.
-	 *
-	 * @returns {Promise} A Promise resolved when the initialization process is finished.
 	 */
 	init() {
 		if ( this.ready ) {
@@ -97,25 +85,16 @@ export default class ViewCollection extends Collection {
 			throw new CKEditorError( 'ui-viewcollection-init-reinit: This ViewCollection has already been initialized.' );
 		}
 
-		return Promise.all( this.map( v => v.init() ) )
-			.then( () => {
-				this.ready = true;
-			} );
+		this.map( v => v.init() );
+
+		this.ready = true;
 	}
 
 	/**
 	 * Destroys the view collection along with child views.
-	 *
-	 * @returns {Promise} A Promise resolved when the destruction process is finished.
 	 */
 	destroy() {
-		// Wait for all #add() promises to resolve before destroying the children.
-		// https://github.com/ckeditor/ckeditor5-ui/issues/203
-		return Promise.all( this._addPromises )
-			// Then begin the process of destroying the children.
-			.then( () => {
-				return Promise.all( this.map( v => v.destroy() ) );
-			} );
+		this.map( v => v.destroy() );
 	}
 
 	/**
@@ -124,27 +103,13 @@ export default class ViewCollection extends Collection {
 	 *
 	 * @param {module:ui/view~View} view A child view.
 	 * @param {Number} [index] Index at which the child will be added to the collection.
-	 * @returns {Promise} A Promise resolved when the child {@link module:ui/view~View#init} is done.
 	 */
 	add( view, index ) {
 		super.add( view, index );
 
-		// {@link module:ui/view~View#init} returns `Promise`.
-		let promise = Promise.resolve();
-
 		if ( this.ready && !view.ready ) {
-			promise = promise
-				.then( () => view.init() )
-				// The view is ready. There's no point in storing the promise any longer.
-				.then( () => this._addPromises.delete( promise ) );
-
-			// Store the promise so it can be respected (and resolved) before #destroy()
-			// starts destroying the child view.
-			// https://github.com/ckeditor/ckeditor5-ui/issues/203
-			this._addPromises.add( promise );
+			view.init();
 		}
-
-		return promise;
 	}
 
 	/**

+ 47 - 176
packages/ckeditor5-ui/tests/view.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document, setTimeout, HTMLElement */
+/* global document, HTMLElement */
 
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import View from '../src/view';
@@ -87,47 +87,21 @@ describe( 'View', () => {
 			setTestViewInstance();
 		} );
 
-		it( 'should return a promise', () => {
-			const spy = sinon.spy();
-			const child = {
-				init: () => {
-					return new Promise( resolve => {
-						setTimeout( () => resolve(), 100 );
-					} )
-					.then( () => spy() );
-				}
-			};
-
-			return view.init()
-				.then( () => {
-					const returned = view.addChildren( child );
-					expect( returned ).to.be.instanceof( Promise );
-
-					return returned.then( () => {
-						sinon.assert.calledOnce( spy );
-					} );
-				} );
-		} );
-
 		it( 'should add a single view to #_unboundChildren', () => {
 			expect( view._unboundChildren ).to.have.length( 0 );
 
 			const child = {};
 
-			return view.addChildren( child )
-				.then( () => {
-					expect( view._unboundChildren ).to.have.length( 1 );
-					expect( view._unboundChildren.get( 0 ) ).to.equal( child );
-				} );
+			view.addChildren( child );
+			expect( view._unboundChildren ).to.have.length( 1 );
+			expect( view._unboundChildren.get( 0 ) ).to.equal( child );
 		} );
 
 		it( 'should support iterables', () => {
 			expect( view._unboundChildren ).to.have.length( 0 );
 
-			return view.addChildren( [ {}, {}, {} ] )
-				.then( () => {
-					expect( view._unboundChildren ).to.have.length( 3 );
-				} );
+			view.addChildren( [ {}, {}, {} ] );
+			expect( view._unboundChildren ).to.have.length( 3 );
 		} );
 	} );
 
@@ -135,28 +109,22 @@ describe( 'View', () => {
 		beforeEach( createViewWithChildren );
 
 		it( 'should throw if already initialized', () => {
-			return view.init()
-				.then( () => {
-					view.init();
-
-					throw new Error( 'This should not be executed.' );
-				} )
-				.catch( err => {
-					expect( err ).to.be.instanceof( CKEditorError );
-					expect( err.message ).to.match( /ui-view-init-re/ );
-				} );
-		} );
-
-		it( 'returns a promise', () => {
-			expect( view.init() ).to.be.instanceof( Promise );
+			view.init();
+
+			try {
+				view.init();
+				throw new Error( 'This should not be executed.' );
+			} catch ( err ) {
+				expect( err ).to.be.instanceof( CKEditorError );
+				expect( err.message ).to.match( /ui-view-init-re/ );
+			}
 		} );
 
 		it( 'should set view#ready', () => {
 			expect( view.ready ).to.be.false;
 
-			return view.init().then( () => {
-				expect( view.ready ).to.be.true;
-			} );
+			view.init();
+			expect( view.ready ).to.be.true;
 		} );
 
 		it( 'calls init() on all view#_viewCollections', () => {
@@ -166,11 +134,10 @@ describe( 'View', () => {
 			const spyA = testUtils.sinon.spy( collectionA, 'init' );
 			const spyB = testUtils.sinon.spy( collectionB, 'init' );
 
-			return view.init().then( () => {
-				sinon.assert.calledOnce( spyA );
-				sinon.assert.calledOnce( spyB );
-				sinon.assert.callOrder( spyA, spyB );
-			} );
+			view.init();
+			sinon.assert.calledOnce( spyA );
+			sinon.assert.calledOnce( spyB );
+			sinon.assert.callOrder( spyA, spyB );
 		} );
 	} );
 
@@ -261,47 +228,32 @@ describe( 'View', () => {
 	describe( 'destroy()', () => {
 		beforeEach( createViewWithChildren );
 
-		it( 'should return a promise', () => {
-			const promise = view.destroy();
-
-			expect( promise ).to.be.instanceof( Promise );
-
-			return promise;
-		} );
-
-		it( 'can be called multiple times', done => {
+		it( 'can be called multiple times', () => {
 			expect( () => {
-				view.destroy().then( () => {
-					return view.destroy().then( () => {
-						done();
-					} );
-				} );
+				view.destroy();
 			} ).to.not.throw();
 		} );
 
 		it( 'should not touch the basic properties', () => {
-			return view.destroy().then( () => {
-				expect( view.element ).to.be.an.instanceof( HTMLElement );
-				expect( view.template ).to.be.an.instanceof( Template );
-				expect( view.locale ).to.be.an( 'object' );
-				expect( view.locale.t ).to.be.a( 'function' );
-
-				expect( view._viewCollections ).to.be.instanceOf( Collection );
-				expect( view._unboundChildren ).to.be.instanceOf( ViewCollection );
-			} );
+			view.destroy();
+
+			expect( view.element ).to.be.an.instanceof( HTMLElement );
+			expect( view.template ).to.be.an.instanceof( Template );
+			expect( view.locale ).to.be.an( 'object' );
+			expect( view.locale.t ).to.be.a( 'function' );
+
+			expect( view._viewCollections ).to.be.instanceOf( Collection );
+			expect( view._unboundChildren ).to.be.instanceOf( ViewCollection );
 		} );
 
 		it( 'should not clear the #_unboundChildren', () => {
 			const cached = view._unboundChildren;
 
-			return view.addChildren( [ new View(), new View() ] )
-				.then( () => {
-					expect( cached ).to.have.length( 4 );
+			view.addChildren( [ new View(), new View() ] );
+			expect( cached ).to.have.length( 4 );
 
-					return view.destroy().then( () => {
-						expect( cached ).to.have.length( 4 );
-					} );
-				} );
+			view.destroy();
+			expect( cached ).to.have.length( 4 );
 		} );
 
 		it( 'should not clear the #_viewCollections', () => {
@@ -309,9 +261,8 @@ describe( 'View', () => {
 
 			expect( cached ).to.have.length( 1 );
 
-			return view.destroy().then( () => {
-				expect( cached ).to.have.length( 1 );
-			} );
+			view.destroy();
+			expect( cached ).to.have.length( 1 );
 		} );
 
 		it( 'leaves the #element in DOM', () => {
@@ -320,9 +271,8 @@ describe( 'View', () => {
 
 			parentEl.appendChild( view.element );
 
-			return view.destroy().then( () => {
-				expect( elRef.parentNode ).to.equal( parentEl );
-			} );
+			view.destroy();
+			expect( elRef.parentNode ).to.equal( parentEl );
 		} );
 
 		it( 'calls destroy() on all view#_viewCollections', () => {
@@ -332,61 +282,18 @@ describe( 'View', () => {
 			const spyA = testUtils.sinon.spy( collectionA, 'destroy' );
 			const spyB = testUtils.sinon.spy( collectionB, 'destroy' );
 
-			return view.destroy().then( () => {
-				sinon.assert.calledOnce( spyA );
-				sinon.assert.calledOnce( spyB );
-				sinon.assert.callOrder( spyA, spyB );
-			} );
+			view.destroy();
+			sinon.assert.calledOnce( spyA );
+			sinon.assert.calledOnce( spyB );
+			sinon.assert.callOrder( spyA, spyB );
 		} );
 
 		it( 'destroy a template–less view', () => {
-			let promise;
-
 			view = new View();
 
 			expect( () => {
-				promise = view.destroy();
+				view.destroy();
 			} ).to.not.throw();
-
-			return promise;
-		} );
-
-		// https://github.com/ckeditor/ckeditor5-ui/issues/203
-		it( 'waits for all #addChildren promises to resolve', () => {
-			const spyA = sinon.spy();
-			const spyB = sinon.spy();
-
-			class DelayedInitView extends View {
-				constructor( delay, spy ) {
-					super();
-
-					this.delay = delay;
-					this.spy = spy;
-				}
-
-				init() {
-					return new Promise( resolve => {
-						setTimeout( () => resolve(), this.delay );
-					} )
-						.then( () => super.init() )
-						.then( () => {
-							this.spy();
-						} );
-				}
-			}
-
-			const viewA = new DelayedInitView( 200, spyA );
-			const viewB = new DelayedInitView( 100, spyB );
-
-			return view.init().then( () => {
-				view.addChildren( [ viewA, viewB ] );
-
-				return view.destroy().then( () => {
-					expect( viewA.ready ).to.be.true;
-					expect( viewB.ready ).to.be.true;
-					sinon.assert.callOrder( spyB, spyA );
-				} );
-			} );
 		} );
 	} );
 } );
@@ -429,44 +336,8 @@ function createViewWithChildren() {
 		}
 	}
 
-	class ChildViewA extends ChildView {
-		init() {
-			const promise = new Promise( resolve => {
-				setTimeout( resolve, 50 );
-			} );
-
-			return super.init().then( promise );
-		}
-
-		destroy() {
-			const promise = new Promise( resolve => {
-				setTimeout( resolve, 10 );
-			} );
-
-			return super.destroy().then( promise );
-		}
-	}
-
-	class ChildViewB extends ChildView {
-		init() {
-			const promise = new Promise( resolve => {
-				setTimeout( resolve, 10 );
-			} );
-
-			return super.init().then( promise );
-		}
-
-		destroy() {
-			const promise = new Promise( resolve => {
-				setTimeout( resolve, 50 );
-			} );
-
-			return super.destroy().then( promise );
-		}
-	}
-
-	childA = new ChildViewA();
-	childB = new ChildViewB();
+	childA = new ChildView();
+	childB = new ChildView();
 
 	setTestViewClass( {
 		tag: 'p',

+ 39 - 146
packages/ckeditor5-ui/tests/viewcollection.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document, setTimeout */
+/* global document */
 
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
@@ -111,79 +111,26 @@ describe( 'ViewCollection', () => {
 				collection.remove( 1 );
 
 				// Finally init the view. Check what's in there.
-				return view.init().then( () => {
-					expect( view.element.childNodes ).to.have.length( 2 );
-					expect( view.element.childNodes[ 0 ] ).to.equal( viewA.element );
-					expect( view.element.childNodes[ 1 ] ).to.equal( viewC.element );
-				} );
+				view.init();
+
+				expect( view.element.childNodes ).to.have.length( 2 );
+				expect( view.element.childNodes[ 0 ] ).to.equal( viewA.element );
+				expect( view.element.childNodes[ 1 ] ).to.equal( viewC.element );
 			} );
 		} );
 	} );
 
 	describe( 'init()', () => {
-		it( 'should return a promise', () => {
-			expect( collection.init() ).to.be.instanceof( Promise );
-		} );
-
-		it( 'should return a composition of child init() promises', () => {
-			const spyA = sinon.spy().named( 'A' );
-			const spyB = sinon.spy().named( 'B' );
-			const spyC = sinon.spy().named( 'C' );
-
-			const childA = {
-				init: () => {
-					return new Promise( resolve => {
-						setTimeout( () => {
-							spyA();
-							resolve();
-						}, 20 );
-					} );
-				}
-			};
-
-			const childB = {
-				init: () => {
-					return new Promise( resolve => {
-						setTimeout( () => {
-							spyB();
-							resolve();
-						}, 10 );
-					} );
-				}
-			};
-
-			const childC = {
-				init: () => {
-					return new Promise( resolve => {
-						setTimeout( () => {
-							spyC();
-							resolve();
-						}, 0 );
-					} );
-				}
-			};
-
-			collection.add( childA );
-			collection.add( childB );
-			collection.add( childC );
-
-			return collection.init()
-				.then( () => {
-					sinon.assert.callOrder( spyC, spyB, spyA );
-				} );
-		} );
-
 		it( 'should throw if already initialized', () => {
-			return collection.init()
-				.then( () => {
-					collection.init();
-
-					throw new Error( 'This should not be executed.' );
-				} )
-				.catch( err => {
-					expect( err ).to.be.instanceof( CKEditorError );
-					expect( err.message ).to.match( /ui-viewcollection-init-reinit/ );
-				} );
+			collection.init();
+
+			try {
+				collection.init();
+				throw new Error( 'This should not be executed.' );
+			} catch ( err ) {
+				expect( err ).to.be.instanceof( CKEditorError );
+				expect( err.message ).to.match( /ui-viewcollection-init-reinit/ );
+			}
 		} );
 
 		it( 'calls #init on all views in the collection', () => {
@@ -201,23 +148,18 @@ describe( 'ViewCollection', () => {
 			collection.add( viewA );
 			collection.add( viewB );
 
-			return collection.init().then( () => {
-				sinon.assert.calledOnce( spyA );
-				sinon.assert.calledOnce( spyB );
-				sinon.assert.callOrder( spyA, spyB );
+			collection.init();
+			sinon.assert.calledOnce( spyA );
+			sinon.assert.calledOnce( spyB );
+			sinon.assert.callOrder( spyA, spyB );
 
-				expect( viewA.element.parentNode ).to.equal( collection._parentElement );
-				expect( viewA.element.nextSibling ).to.equal( viewB.element );
-				expect( collection.ready ).to.be.true;
-			} );
+			expect( viewA.element.parentNode ).to.equal( collection._parentElement );
+			expect( viewA.element.nextSibling ).to.equal( viewB.element );
+			expect( collection.ready ).to.be.true;
 		} );
 	} );
 
 	describe( 'destroy()', () => {
-		it( 'should return a promise', () => {
-			expect( collection.destroy() ).to.be.instanceof( Promise );
-		} );
-
 		it( 'calls #destroy on all views in the collection', () => {
 			const viewA = new View();
 			const viewB = new View();
@@ -228,59 +170,14 @@ describe( 'ViewCollection', () => {
 			collection.add( viewA );
 			collection.add( viewB );
 
-			return collection.destroy().then( () => {
-				sinon.assert.calledOnce( spyA );
-				sinon.assert.calledOnce( spyB );
-				sinon.assert.callOrder( spyA, spyB );
-			} );
-		} );
-
-		// https://github.com/ckeditor/ckeditor5-ui/issues/203
-		it( 'waits for all #add promises to resolve', () => {
-			const spyA = sinon.spy();
-			const spyB = sinon.spy();
-
-			class DelayedInitView extends View {
-				constructor( delay, spy ) {
-					super();
-
-					this.delay = delay;
-					this.spy = spy;
-				}
-
-				init() {
-					return new Promise( resolve => {
-						setTimeout( () => resolve(), this.delay );
-					} )
-						.then( () => super.init() )
-						.then( () => {
-							this.spy();
-						} );
-				}
-			}
-
-			const viewA = new DelayedInitView( 200, spyA );
-			const viewB = new DelayedInitView( 100, spyB );
-
-			return collection.init().then( () => {
-				collection.add( viewA );
-				collection.add( viewB );
-			} )
-				.then( () => {
-					return collection.destroy().then( () => {
-						expect( viewA.ready ).to.be.true;
-						expect( viewB.ready ).to.be.true;
-						sinon.assert.callOrder( spyB, spyA );
-					} );
-				} );
+			collection.destroy();
+			sinon.assert.calledOnce( spyA );
+			sinon.assert.calledOnce( spyB );
+			sinon.assert.callOrder( spyA, spyB );
 		} );
 	} );
 
 	describe( 'add()', () => {
-		it( 'returns a promise', () => {
-			expect( collection.add( {} ) ).to.be.instanceof( Promise );
-		} );
-
 		it( 'initializes the new view in the collection', () => {
 			let view = new View();
 			let spy = testUtils.sinon.spy( view, 'init' );
@@ -288,23 +185,20 @@ describe( 'ViewCollection', () => {
 			expect( collection.ready ).to.be.false;
 			expect( view.ready ).to.be.false;
 
-			return collection.add( view ).then( () => {
-				expect( collection.ready ).to.be.false;
-				expect( view.ready ).to.be.false;
-
-				sinon.assert.notCalled( spy );
+			collection.add( view );
+			expect( collection.ready ).to.be.false;
+			expect( view.ready ).to.be.false;
 
-				view = new View();
-				spy = testUtils.sinon.spy( view, 'init' );
+			sinon.assert.notCalled( spy );
 
-				collection.ready = true;
+			view = new View();
+			spy = testUtils.sinon.spy( view, 'init' );
 
-				return collection.add( view ).then( () => {
-					expect( view.ready ).to.be.true;
+			collection.ready = true;
 
-					sinon.assert.calledOnce( spy );
-				} );
-			} );
+			collection.add( view );
+			expect( view.ready ).to.be.true;
+			sinon.assert.calledOnce( spy );
 		} );
 
 		it( 'works for a view with a Number view#id attribute', () => {
@@ -312,10 +206,9 @@ describe( 'ViewCollection', () => {
 
 			view.set( 'id', 1 );
 
-			return collection.add( view ).then( () => {
-				expect( view.id ).to.equal( 1 );
-				expect( view.viewUid ).to.be.a( 'string' );
-			} );
+			collection.add( view );
+			expect( view.id ).to.equal( 1 );
+			expect( view.viewUid ).to.be.a( 'string' );
 		} );
 	} );