Przeglądaj źródła

Merge pull request #84 from ckeditor/t/78b

Other: Introduced `PluginInterface`. A plugin doesn't need to inherit directly from the `Plugin` class, as long as it implements some minimal interface. See #78.
Piotrek Koszuliński 8 lat temu
rodzic
commit
af061d9422

+ 4 - 0
packages/ckeditor5-core/src/editor/editor.js

@@ -128,6 +128,10 @@ export default class Editor {
 
 		function initPlugins( loadedPlugins, method ) {
 			return loadedPlugins.reduce( ( promise, plugin ) => {
+				if ( !plugin[ method ] ) {
+					return promise;
+				}
+
 				return promise.then( plugin[ method ].bind( plugin ) );
 			}, Promise.resolve() );
 		}

+ 121 - 75
packages/ckeditor5-core/src/plugin.js

@@ -13,98 +13,144 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
 /**
  * The base class for CKEditor plugin classes.
  *
- * @mixes module:utils/observablemixin~ObservaleMixin
+ * @implements module:core/plugin~PluginInterface
+ * @mixes module:utils/observablemixin~ObservableMixin
  */
 export default class Plugin {
 	/**
-	 * Creates a new Plugin instance. This is the first step of a plugin initialization.
-	 * See also {@link #init} and {@link #afterInit}.
-	 *
-	 * A plugin is always instantiated after its {@link module:core/plugin~Plugin.requires dependencies} and the
-	 * {@link #init} and {@link #afterInit} methods are called in the same order.
-	 *
-	 * Usually, you'll want to put your plugin's initialization code in the {@link #init} method.
-	 * The constructor can be understood as "before init" and used in special cases, just like
-	 * {@link #afterInit} servers for the special "after init" scenarios (e.g. code which depends on other
-	 * plugins, but which doesn't {@link module:core/plugin~Plugin.requires explicitly require} them).
-	 *
-	 * @param {module:core/editor/editor~Editor} editor
+	 * @inheritDoc
 	 */
 	constructor( editor ) {
 		/**
 		 * The editor instance.
 		 *
 		 * @readonly
-		 * @member {module:core/editor/editor~Editor} module:core/plugin~Plugin#editor
+		 * @member {module:core/editor/editor~Editor} #editor
 		 */
 		this.editor = editor;
 	}
 
 	/**
-	 * An array of plugins required by this plugin.
-	 *
-	 * To keep a plugin class definition tight it's recommended to define this property as a static getter:
-	 *
-	 *		import Image from './image.js';
-	 *
-	 *		export default class ImageCaption extends Plugin {
-     *			static get requires() {
-     *				return [ Image ];
-     *			}
-	 *		}
-	 *
-	 * @static
-	 * @readonly
-	 * @member {Array.<Function>|undefined} module:core/plugin~Plugin.requires
+	 * @inheritDoc
 	 */
+	destroy() {
+	}
+}
 
-	/**
-	 * Optional name of the plugin. If set, the plugin will be available in
-	 * {@link module:core/plugincollection~PluginCollection#get} by its
-	 * name and its constructor. If not, then only by its constructor.
-	 *
-	 * The name should reflect the package name + the plugin module name. E.g. `ckeditor5-image/src/image.js` plugin
-	 * should be named `image/image` (the `ckeditor5-` prefix is stripped during compilation). If plugin is kept
-	 * deeper in the directory structure, it's recommended to only use the module file name, not the whole path.
-	 * So, e.g. a plugin defined in `ckeditor5-ui/src/notification/notification.js` file may be named `ui/notification`.
-	 *
-	 * To keep a plugin class definition tight it's recommended to define this property as a static getter:
-	 *
-	 *		export default class ImageCaption {
-     *			static get pluginName() {
-     *				return 'image/imagecaption';
-     *			}
-	 *		}
-	 *
-	 * @static
-	 * @readonly
-	 * @member {String|undefined} module:core/plugin~Plugin.pluginName
-	 */
+mix( Plugin, ObservableMixin );
 
-	/**
-	 * The second stage (after plugin {@link #constructor}) of plugin initialization.
-	 * Unlike the plugin constructor this method can perform asynchronous.
-	 *
-	 * A plugin's `init()` method is called after its {@link module:core/plugin~Plugin.requires dependencies} are initialized,
-	 * so in the same order as constructors of these plugins.
-	 *
-	 * @returns {null|Promise}
-	 */
-	init() {}
+/**
+ * The base interface for CKEditor plugins.
+ *
+ * In its minimal form it can be a simple function (it will be used as a constructor) which accepts
+ * {@link module:core/editor/editor~Editor the editor} as a parm.
+ * It can also implement a few methods which, when present, will be used to properly initialize and destroy the plugin.
+ *
+ *		// A simple plugin which enables a data processor.
+ *		function MyPlugin( editor ) {
+ *			editor.data.processor = new MyDataProcessor();
+ *		}
+ *
+ * In most cases, however, you'll want to inherit from the {@link module:core/plugin~Plugin} class which implements the
+ * {@link module:utils/observablemixin~ObservableMixin} and is, therefore, more convenient:
+ *
+ *		class MyPlugin extends Plugin {
+ *			init() {
+ *				// `listenTo()` and `editor` are available thanks to `Plugin`.
+ *				// By using `listenTo()` you'll ensure that the listener will be removed when
+ *				// the plugin is destroyed.
+ *				this.listenTo( this.editor, 'dataReady', () => {
+ *					// Do something when data is ready.
+ *				} );
+ *			}
+ *		}
+ *
+ * @interface PluginInterface
+ */
 
-	/**
-	 * The third (and last) stage of plugin initialization. See also {@link #constructor} and {@link #init}.
-	 *
-	 * @returns {null|Promise}
-	 */
-	afterInit() {}
+/**
+ * Creates a new plugin instance. This is the first step of a plugin initialization.
+ * See also {@link #init} and {@link #afterInit}.
+ *
+ * A plugin is always instantiated after its {@link module:core/plugin~PluginInterface.requires dependencies} and the
+ * {@link #init} and {@link #afterInit} methods are called in the same order.
+ *
+ * Usually, you'll want to put your plugin's initialization code in the {@link #init} method.
+ * The constructor can be understood as "before init" and used in special cases, just like
+ * {@link #afterInit} servers for the special "after init" scenarios (e.g. code which depends on other
+ * plugins, but which doesn't {@link module:core/plugin~PluginInterface.requires explicitly require} them).
+ *
+ * @method #constructor
+ * @param {module:core/editor/editor~Editor} editor
+ */
 
-	/**
-	 * Destroys the plugin.
-	 *
-	 * @returns {null|Promise}
-	 */
-	destroy() {}
-}
+/**
+ * An array of plugins required by this plugin.
+ *
+ * To keep a plugin class definition tight it's recommended to define this property as a static getter:
+ *
+ *		import Image from './image.js';
+ *
+ *		export default class ImageCaption {
+ *			static get requires() {
+ *				return [ Image ];
+ *			}
+ *      }
+ *
+ * @static
+ * @readonly
+ * @member {Array.<Function>|undefined} module:core/plugin~PluginInterface.requires
+ */
 
-mix( Plugin, ObservableMixin );
+/**
+ * Optional name of the plugin. If set, the plugin will be available in
+ * {@link module:core/plugincollection~PluginCollection#get} by its
+ * name and its constructor. If not, then only by its constructor.
+ *
+ * The name should reflect the package name + the plugin module name. E.g. `ckeditor5-image/src/image.js` plugin
+ * should be named `image/image`. If plugin is kept deeper in the directory structure, it's recommended to only use the module file name,
+ * not the whole path. So, e.g. a plugin defined in `ckeditor5-ui/src/notification/notification.js` file may be named `ui/notification`.
+ *
+ * To keep a plugin class definition tight it's recommended to define this property as a static getter:
+ *
+ *		export default class ImageCaption {
+ *			static get pluginName() {
+ *				return 'image/imagecaption';
+ *			}
+ *		}
+ *
+ * @static
+ * @readonly
+ * @member {String|undefined} module:core/plugin~PluginInterface.pluginName
+ */
+
+/**
+ * The second stage (after plugin {@link #constructor}) of plugin initialization.
+ * Unlike the plugin constructor this method can be asynchronous.
+ *
+ * A plugin's `init()` method is called after its {@link module:core/plugin~PluginInterface.requires dependencies} are initialized,
+ * so in the same order as constructors of these plugins.
+ *
+ * **Note:** This method is optional. A plugin instance does not need to have to have it defined.
+ *
+ * @method #init
+ * @returns {null|Promise}
+ */
+
+/**
+ * The third (and last) stage of plugin initialization. See also {@link #constructor} and {@link #init}.
+ *
+ * **Note:** This method is optional. A plugin instance does not need to have to have it defined.
+ *
+ * @method #afterInit
+ * @returns {null|Promise}
+ */
+
+/**
+ * Destroys the plugin.
+ *
+ * **Note:** This method is optional. A plugin instance does not need to have to have it defined.
+ *
+ * @method #destroy
+ * @returns {null|Promise}
+ */

+ 7 - 25
packages/ckeditor5-core/src/plugincollection.js

@@ -7,7 +7,6 @@
  * @module core/plugincollection
  */
 
-import Plugin from './plugin';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 
@@ -21,7 +20,7 @@ export default class PluginCollection {
 	 *
 	 * @param {module:core/editor/editor~Editor} editor
 	 * @param {Array.<Function>} [availablePlugins] Plugins (constructors) which the collection will be able to use
-	 * when {@link module:core/plugin~PluginCollection#load} is used with plugin names (strings, instead of constructors).
+	 * when {@link module:core/plugincollection~PluginCollection#load} is used with plugin names (strings, instead of constructors).
 	 * Usually, the editor will pass its built-in plugins to the collection so they can later be
 	 * used in `config.plugins` or `config.removePlugins` by names.
 	 */
@@ -69,8 +68,8 @@ export default class PluginCollection {
 	/**
 	 * Gets the plugin instance by its constructor or name.
 	 *
-	 * @param {Function|String} key The plugin constructor or {@link module:core/plugin~Plugin.pluginName name}.
-	 * @returns {module:core/plugin~Plugin}
+	 * @param {Function|String} key The plugin constructor or {@link module:core/plugin~PluginInterface.pluginName name}.
+	 * @returns {module:core/plugin~PluginInterface}
 	 */
 	get( key ) {
 		return this._plugins.get( key );
@@ -79,14 +78,14 @@ export default class PluginCollection {
 	/**
 	 * Loads a set of plugins and adds them to the collection.
 	 *
-	 * @param {Array.<Function|String>} plugins An array of {@link module:core/plugin~Plugin plugin constructors}
-	 * or {@link module:core/plugin~Plugin.pluginName plugin names}. The second option (names) work only if
+	 * @param {Array.<Function|String>} plugins An array of {@link module:core/plugin~PluginInterface plugin constructors}
+	 * or {@link module:core/plugin~PluginInterface.pluginName plugin names}. The second option (names) work only if
 	 * `availablePlugins` were passed to the {@link #constructor}.
 	 * @param {Array.<String|Function>} [removePlugins] Names of plugins or plugin constructors
 	 * which should not be loaded (despite being specified in the `plugins` array).
 	 * @returns {Promise} A promise which gets resolved once all plugins are loaded and available into the
 	 * collection.
-	 * @returns {Promise.<Array.<module:core/plugin~Plugin>>} returns.loadedPlugins The array of loaded plugins.
+	 * @returns {Promise.<Array.<module:core/plugin~PluginInterface>>} returns.loadedPlugins The array of loaded plugins.
 	 */
 	load( plugins, removePlugins = [] ) {
 		const that = this;
@@ -150,8 +149,6 @@ export default class PluginCollection {
 			return new Promise( resolve => {
 				loading.add( PluginConstructor );
 
-				assertIsPlugin( PluginConstructor );
-
 				if ( PluginConstructor.requires ) {
 					PluginConstructor.requires.forEach( RequiredPluginConstructorOrName => {
 						const RequiredPluginConstructor = getPluginConstructor( RequiredPluginConstructorOrName );
@@ -191,21 +188,6 @@ export default class PluginCollection {
 			return that._availablePlugins.get( PluginConstructorOrName );
 		}
 
-		function assertIsPlugin( PluginConstructor ) {
-			if ( !( PluginConstructor.prototype instanceof Plugin ) ) {
-				/**
-				 * The loaded plugin module is not an instance of {@link module:core/plugin~Plugin}.
-				 *
-				 * @error plugincollection-instance
-				 * @param {*} plugin The constructor which is meant to be loaded as a plugin.
-				 */
-				throw new CKEditorError(
-					'plugincollection-instance: The loaded plugin module is not an instance of Plugin.',
-					{ plugin: PluginConstructor }
-				);
-			}
-		}
-
 		function getMissingPluginNames( plugins ) {
 			const missingPlugins = [];
 
@@ -230,7 +212,7 @@ export default class PluginCollection {
 	 *
 	 * @protected
 	 * @param {Function} PluginConstructor The plugin constructor.
-	 * @param {module:core/plugin~Plugin} plugin The instance of the plugin.
+	 * @param {module:core/plugin~PluginInterface} plugin The instance of the plugin.
 	 */
 	_add( PluginConstructor, plugin ) {
 		this._plugins.set( PluginConstructor, plugin );

+ 76 - 17
packages/ckeditor5-core/tests/editor/editor.js

@@ -66,6 +66,28 @@ class PluginD extends Plugin {
 	}
 }
 
+class PluginE {
+	constructor( editor ) {
+		this.editor = editor;
+		this.init = sinon.spy().named( 'E' );
+	}
+
+	static get pluginName() {
+		return 'E';
+	}
+}
+
+class PluginF {
+	constructor( editor ) {
+		this.editor = editor;
+		this.afterInit = sinon.spy().named( 'F-after' );
+	}
+
+	static get pluginName() {
+		return 'F';
+	}
+}
+
 describe( 'Editor', () => {
 	afterEach( () => {
 		delete Editor.build;
@@ -127,9 +149,7 @@ describe( 'Editor', () => {
 		} );
 
 		it( 'loads plugins', () => {
-			return Editor.create( {
-				plugins: [ PluginA ]
-			} )
+			return Editor.create( { plugins: [ PluginA ] } )
 				.then( editor => {
 					expect( getPlugins( editor ).length ).to.equal( 1 );
 
@@ -183,19 +203,20 @@ describe( 'Editor', () => {
 			const pluginsReadySpy = sinon.spy().named( 'pluginsReady' );
 			editor.on( 'pluginsReady', pluginsReadySpy );
 
-			return editor.initPlugins().then( () => {
-				sinon.assert.callOrder(
-					editor.plugins.get( PluginA ).init,
-					editor.plugins.get( PluginB ).init,
-					editor.plugins.get( PluginC ).init,
-					editor.plugins.get( PluginD ).init,
-					editor.plugins.get( PluginA ).afterInit,
-					editor.plugins.get( PluginB ).afterInit,
-					editor.plugins.get( PluginC ).afterInit,
-					editor.plugins.get( PluginD ).afterInit,
-					pluginsReadySpy
-				);
-			} );
+			return editor.initPlugins()
+				.then( () => {
+					sinon.assert.callOrder(
+						editor.plugins.get( PluginA ).init,
+						editor.plugins.get( PluginB ).init,
+						editor.plugins.get( PluginC ).init,
+						editor.plugins.get( PluginD ).init,
+						editor.plugins.get( PluginA ).afterInit,
+						editor.plugins.get( PluginB ).afterInit,
+						editor.plugins.get( PluginC ).afterInit,
+						editor.plugins.get( PluginD ).afterInit,
+						pluginsReadySpy
+					);
+				} );
 		} );
 
 		it( 'should initialize plugins in the right order, waiting for asynchronous init()', () => {
@@ -443,7 +464,45 @@ describe( 'Editor', () => {
 			return editor.initPlugins()
 				.then( () => {
 					expect( getPlugins( editor ).length ).to.equal( 1 );
-					expect( editor.plugins.get( PluginA ) ).to.be.an.instanceof( Plugin );
+					expect( editor.plugins.get( PluginA ) ).to.not.be.undefined;
+				} );
+		} );
+
+		it( 'should not call "afterInit" method if plugin does not have this method', () => {
+			const editor = new Editor( {
+				plugins: [ PluginA, PluginE ]
+			} );
+
+			const pluginsReadySpy = sinon.spy().named( 'pluginsReady' );
+			editor.on( 'pluginsReady', pluginsReadySpy );
+
+			return editor.initPlugins()
+				.then( () => {
+					sinon.assert.callOrder(
+						editor.plugins.get( PluginA ).init,
+						editor.plugins.get( PluginE ).init,
+						editor.plugins.get( PluginA ).afterInit,
+						pluginsReadySpy
+					);
+				} );
+		} );
+
+		it( 'should not call "init" method if plugin does not have this method', () => {
+			const editor = new Editor( {
+				plugins: [ PluginA, PluginF ]
+			} );
+
+			const pluginsReadySpy = sinon.spy().named( 'pluginsReady' );
+			editor.on( 'pluginsReady', pluginsReadySpy );
+
+			return editor.initPlugins()
+				.then( () => {
+					sinon.assert.callOrder(
+						editor.plugins.get( PluginA ).init,
+						editor.plugins.get( PluginA ).afterInit,
+						editor.plugins.get( PluginF ).afterInit,
+						pluginsReadySpy
+					);
 				} );
 		} );
 	} );

+ 5 - 17
packages/ckeditor5-core/tests/plugin.js

@@ -18,24 +18,12 @@ describe( 'constructor()', () => {
 
 		expect( plugin ).to.have.property( 'editor' ).to.equal( editor );
 	} );
-} );
-
-describe( 'init', () => {
-	it( 'should exist and do nothing', () => {
-		const plugin = new Plugin( editor );
-
-		expect( plugin.init ).to.be.a( 'function' );
-
-		plugin.init();
-	} );
-} );
-
-describe( 'destroy', () => {
-	it( 'should exist and do nothing', () => {
-		const plugin = new Plugin( editor );
 
-		expect( plugin.destroy ).to.be.a( 'function' );
+	describe( 'destroy()', () => {
+		it( 'should be defined', () => {
+			const plugin = new Plugin( editor );
 
-		plugin.destroy();
+			expect( plugin.destroy ).to.be.a( 'function' );
+		} );
 	} );
 } );

+ 36 - 23
packages/ckeditor5-core/tests/plugincollection.js

@@ -187,52 +187,65 @@ describe( 'PluginCollection', () => {
 				} );
 		} );
 
-		it( 'should set the `editor` property on loaded plugins', () => {
+		it( 'should load plugin which does not extend the base Plugin class', () => {
+			class Y { }
+
 			const plugins = new PluginCollection( editor, availablePlugins );
 
-			return plugins.load( [ PluginA, PluginB ] )
+			return plugins.load( [ Y ] )
 				.then( () => {
-					expect( plugins.get( PluginA ).editor ).to.equal( editor );
-					expect( plugins.get( PluginB ).editor ).to.equal( editor );
+					expect( getPlugins( plugins ).length ).to.equal( 1 );
 				} );
 		} );
 
-		it( 'should reject on broken plugins (forward the error thrown in a plugin)', () => {
-			const logSpy = testUtils.sinon.stub( log, 'error' );
+		it( 'should load plugin which is a simple function', () => {
+			function pluginAsFunction( editor ) {
+				this.editor = editor;
+			}
 
 			const plugins = new PluginCollection( editor, availablePlugins );
 
-			return plugins.load( [ PluginA, PluginX, PluginB ] )
-				// Throw here, so if by any chance plugins.load() was resolved correctly catch() will be stil executed.
+			return plugins.load( [ pluginAsFunction ] )
 				.then( () => {
-					throw new Error( 'Test error: this promise should not be resolved successfully' );
-				} )
-				.catch( err => {
-					expect( err ).to.be.an.instanceof( TestError );
-					expect( err ).to.have.property( 'message', 'Some error inside a plugin' );
-
-					sinon.assert.calledOnce( logSpy );
-					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-load:/ );
+					expect( getPlugins( plugins ).length ).to.equal( 1 );
 				} );
 		} );
 
-		it( 'should reject when loading a module which is not a plugin', () => {
-			class Y {}
+		it( 'should set the `editor` property on loaded plugins', () => {
+			const plugins = new PluginCollection( editor, availablePlugins );
+
+			function pluginAsFunction( editor ) {
+				this.editor = editor;
+			}
 
-			availablePlugins.push( Y );
+			class Y {
+				constructor( editor ) {
+					this.editor = editor;
+				}
+			}
 
+			return plugins.load( [ PluginA, PluginB, pluginAsFunction, Y ] )
+				.then( () => {
+					expect( plugins.get( PluginA ).editor ).to.equal( editor );
+					expect( plugins.get( PluginB ).editor ).to.equal( editor );
+					expect( plugins.get( pluginAsFunction ).editor ).to.equal( editor );
+					expect( plugins.get( Y ).editor ).to.equal( editor );
+				} );
+		} );
+
+		it( 'should reject on broken plugins (forward the error thrown in a plugin)', () => {
 			const logSpy = testUtils.sinon.stub( log, 'error' );
 
 			const plugins = new PluginCollection( editor, availablePlugins );
 
-			return plugins.load( [ Y ] )
+			return plugins.load( [ PluginA, PluginX, PluginB ] )
 				// Throw here, so if by any chance plugins.load() was resolved correctly catch() will be stil executed.
 				.then( () => {
 					throw new Error( 'Test error: this promise should not be resolved successfully' );
 				} )
 				.catch( err => {
-					expect( err ).to.be.an.instanceof( CKEditorError );
-					expect( err.message ).to.match( /^plugincollection-instance/ );
+					expect( err ).to.be.an.instanceof( TestError );
+					expect( err ).to.have.property( 'message', 'Some error inside a plugin' );
 
 					sinon.assert.calledOnce( logSpy );
 					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-load:/ );
@@ -307,7 +320,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should load chosen plugins (plugins are names, removePlugins contains an anonymous plugin)', () => {
-			class AnonymousPlugin extends Plugin {}
+			class AnonymousPlugin {}
 
 			const plugins = new PluginCollection( editor, [ AnonymousPlugin ].concat( availablePlugins ) );