Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/86: Plugin#destroy() should remove listeners #87

Merged
merged 9 commits into from
Jun 20, 2017
2 changes: 1 addition & 1 deletion src/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export default class Editor {
this.fire( 'destroy' );
this.stopListening();

return Promise.resolve()
return this.plugins.destroy()
.then( () => {
this.document.destroy();
this.data.destroy();
Expand Down
1 change: 1 addition & 0 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default class Plugin {
* @inheritDoc
*/
destroy() {
this.stopListening();
}
}

Expand Down
15 changes: 15 additions & 0 deletions src/plugincollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,21 @@ export default class PluginCollection {
}
}

/**
* Destroys all loaded plugins.
*
* @returns {Promise}
*/
destroy() {
const promises = [];

for ( const [ , pluginInstance ] of this ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all: this could be simplified as: Array.from( this._plugins.values() ).map( plugin => plugin.destroy() )

Second, destroy() is optional in PluginInterface.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use Array.from( this._plugins.values() ) because we will call plugin.destroy() twice for plugins that have a defined name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we can still do Array.from( this ).map( ... ).

promises.push( pluginInstance.destroy() );
}

return Promise.all( promises );
}

/**
* Adds the plugin to the collection. Exposed mainly for testing purposes.
*
Expand Down
2 changes: 2 additions & 0 deletions tests/editor/editor-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ describe( 'Editor', () => {

const spy1 = sinon.spy( editor.data, 'destroy' );
const spy2 = sinon.spy( editor.document, 'destroy' );
const spy3 = sinon.spy( editor.plugins, 'destroy' );

return editor.destroy()
.then( () => {
expect( spy1.calledOnce ).to.be.true;
expect( spy2.calledOnce ).to.be.true;
expect( spy3.calledOnce ).to.be.true;
} );
} );
} );
Expand Down
9 changes: 9 additions & 0 deletions tests/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,14 @@ describe( 'constructor()', () => {

expect( plugin.destroy ).to.be.a( 'function' );
} );

it( 'should stop listening', () => {
const plugin = new Plugin( editor );
const stopListeningSpy = sinon.spy( plugin, 'stopListening' );

plugin.destroy();

expect( stopListeningSpy.calledOnce ).to.equal( true );
} );
} );
} );
20 changes: 20 additions & 0 deletions tests/plugincollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,26 @@ describe( 'PluginCollection', () => {
} );
} );

describe( 'destroy()', () => {
it( 'calls "destroy" method on each loaded plugin', () => {
let destroySpyForPluginA, destroySpyForPluginB;

const plugins = new PluginCollection( editor, [] );

return plugins.load( [ PluginA, PluginB ] )
.then( () => {
destroySpyForPluginA = testUtils.sinon.spy( plugins.get( PluginA ), 'destroy' );
destroySpyForPluginB = testUtils.sinon.spy( plugins.get( PluginB ), 'destroy' );

return plugins.destroy();
} )
.then( () => {
expect( destroySpyForPluginA.calledOnce ).to.equal( true );
expect( destroySpyForPluginB.calledOnce ).to.equal( true );
} );
} );
} );

describe( 'iterator', () => {
it( 'exists', () => {
const plugins = new PluginCollection( editor, availablePlugins );
Expand Down