Przeglądaj źródła

Fix: PluginCollection.destroy() does not crash if a plugin does not have a "destroy()" method.

Kamil Piechaczek 8 lat temu
rodzic
commit
223c6e0071

+ 3 - 1
packages/ckeditor5-core/src/plugincollection.js

@@ -216,7 +216,9 @@ export default class PluginCollection {
 		const promises = [];
 		const promises = [];
 
 
 		for ( const [ , pluginInstance ] of this ) {
 		for ( const [ , pluginInstance ] of this ) {
-			promises.push( pluginInstance.destroy() );
+			if ( typeof pluginInstance.destroy == 'function' ) {
+				promises.push( pluginInstance.destroy() );
+			}
 		}
 		}
 
 
 		return Promise.all( promises );
 		return Promise.all( promises );

+ 57 - 2
packages/ckeditor5-core/tests/plugincollection.js

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md.
  * For licensing, see LICENSE.md.
  */
  */
 
 
+/* globals setTimeout */
+
 import testUtils from '../tests/_utils/utils';
 import testUtils from '../tests/_utils/utils';
 import Editor from '../src/editor/editor';
 import Editor from '../src/editor/editor';
 import PluginCollection from '../src/plugincollection';
 import PluginCollection from '../src/plugincollection';
@@ -389,8 +391,8 @@ describe( 'PluginCollection', () => {
 
 
 			return plugins.load( [ PluginA, PluginB ] )
 			return plugins.load( [ PluginA, PluginB ] )
 				.then( () => {
 				.then( () => {
-					destroySpyForPluginA = testUtils.sinon.spy( plugins.get( PluginA ), 'destroy' );
-					destroySpyForPluginB = testUtils.sinon.spy( plugins.get( PluginB ), 'destroy' );
+					destroySpyForPluginA = sinon.spy( plugins.get( PluginA ), 'destroy' );
+					destroySpyForPluginB = sinon.spy( plugins.get( PluginB ), 'destroy' );
 
 
 					return plugins.destroy();
 					return plugins.destroy();
 				} )
 				} )
@@ -399,6 +401,59 @@ describe( 'PluginCollection', () => {
 					expect( destroySpyForPluginB.calledOnce ).to.equal( true );
 					expect( destroySpyForPluginB.calledOnce ).to.equal( true );
 				} );
 				} );
 		} );
 		} );
+
+		it( 'waits when all plugins are destroyed', () => {
+			const destroyedPlugins = [];
+
+			class AsynchronousPluginA extends Plugin {
+				destroy() {
+					return new Promise( resolve => {
+						setTimeout( () => {
+							super.destroy();
+							destroyedPlugins.push( 'AsynchronousPluginA.destroy()' );
+							resolve();
+						}, 200 );
+					} );
+				}
+			}
+
+			class AsynchronousPluginB extends Plugin {
+				destroy() {
+					return new Promise( resolve => {
+						setTimeout( () => {
+							super.destroy();
+							destroyedPlugins.push( 'AsynchronousPluginB.destroy()' );
+							resolve();
+						}, 100 );
+					} );
+				}
+			}
+
+			const plugins = new PluginCollection( editor, [] );
+
+			return plugins.load( [ AsynchronousPluginA, AsynchronousPluginB ] )
+				.then( () => plugins.destroy() )
+				.then( () => {
+					expect( destroyedPlugins ).to.contain( 'AsynchronousPluginB.destroy()' );
+					expect( destroyedPlugins ).to.contain( 'AsynchronousPluginA.destroy()' );
+				} );
+		} );
+
+		it( 'does not execute "Plugin.destroy()" for plugins which do not have this method', () => {
+			class FooPlugin {
+				constructor( editor ) {
+					this.editor = editor;
+				}
+			}
+
+			const plugins = new PluginCollection( editor, [] );
+
+			return plugins.load( [ PluginA, FooPlugin ] )
+				.then( () => plugins.destroy() )
+				.then( destroyedPlugins => {
+					expect( destroyedPlugins.length ).to.equal( 1 );
+				} );
+		} );
 	} );
 	} );
 
 
 	describe( 'iterator', () => {
 	describe( 'iterator', () => {