소스 검색

Merge pull request #70 from ckeditor/t/49

Feature: Added support for loading plugins by name and the "removePlugins" option. Closes #49.
Piotrek Koszuliński 9 년 전
부모
커밋
d146dd54f8
2개의 변경된 파일283개의 추가작업 그리고 29개의 파일을 삭제
  1. 107 8
      packages/ckeditor5-core/src/plugincollection.js
  2. 176 21
      packages/ckeditor5-core/tests/plugincollection.js

+ 107 - 8
packages/ckeditor5-core/src/plugincollection.js

@@ -16,22 +16,43 @@ import log from '@ckeditor/ckeditor5-utils/src/log';
  */
 export default class PluginCollection {
 	/**
-	 * Creates an instance of the PluginCollection class, initializing it with a set of plugins.
+	 * Creates an instance of the PluginCollection class.
+	 * Allows loading and initializing plugins and their dependencies.
 	 *
 	 * @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).
+	 * 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.
 	 */
-	constructor( editor ) {
+	constructor( editor, availablePlugins = [] ) {
 		/**
 		 * @protected
 		 * @member {module:core/editor/editor~Editor} module:core/plugin~PluginCollection#_editor
 		 */
 		this._editor = editor;
 
+		/**
+		 * Map of plugin constructors which can be retrieved by their names.
+		 *
+		 * @protected
+		 * @member {Map.<String|Function,Function>} module:core/plugin~PluginCollection#_availablePlugins
+		 */
+		this._availablePlugins = new Map();
+
 		/**
 		 * @protected
 		 * @member {Map} module:core/plugin~PluginCollection#_plugins
 		 */
 		this._plugins = new Map();
+
+		for ( const PluginConstructor of availablePlugins ) {
+			this._availablePlugins.set( PluginConstructor, PluginConstructor );
+
+			if ( PluginConstructor.pluginName ) {
+				this._availablePlugins.set( PluginConstructor.pluginName, PluginConstructor );
+			}
+		}
 	}
 
 	/**
@@ -56,23 +77,56 @@ export default class PluginCollection {
 	}
 
 	/**
-	 * Loads a set of plugins and add them to the collection.
+	 * Loads a set of plugins and adds them to the collection.
 	 *
-	 * @param {Function[]} plugins An array of {@link module:core/plugin~Plugin plugin constructors}.
+	 * @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
+	 * `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.
-	 * @param {Array.<module:core/plugin~Plugin>} returns.loadedPlugins The array of loaded plugins.
+	 * @returns {Promise.<Array.<module:core/plugin~Plugin>>} returns.loadedPlugins The array of loaded plugins.
 	 */
-	load( plugins ) {
+	load( plugins, removePlugins = [] ) {
 		const that = this;
 		const editor = this._editor;
 		const loading = new Set();
 		const loaded = [];
 
-		return Promise.all( plugins.map( loadPlugin ) )
+		const pluginConstructors = mapToAvailableConstructors( plugins );
+		const removePluginConstructors = mapToAvailableConstructors( removePlugins );
+		const missingPlugins = getMissingPluginNames( plugins );
+
+		if ( missingPlugins ) {
+			// TODO update this error docs with links to docs because it will be a frequent problem.
+
+			/**
+			 * Some plugins are not available and could not be loaded.
+			 *
+			 * Plugin classes (constructors) need to be provided to the editor before they can be loaded by name.
+			 * This is usually done by the builder by setting the {@link module:core/editor/editor~Editor.build}
+			 * property.
+			 *
+			 * @error plugincollection-plugin-not-found
+			 * @param {Array.<String>} plugins The name of the plugins which could not be loaded.
+			 */
+			const errorMsg = 'plugincollection-plugin-not-found: Some plugins are not available and could not be loaded.';
+
+			// Log the error so it's more visible on the console. Hopefuly, for better DX.
+			log.error( errorMsg, { plugins: missingPlugins } );
+
+			return Promise.reject( new CKEditorError( errorMsg, { plugins: missingPlugins } ) );
+		}
+
+		return Promise.all( pluginConstructors.map( loadPlugin ) )
 			.then( () => loaded );
 
 		function loadPlugin( PluginConstructor ) {
+			if ( removePluginConstructors.includes( PluginConstructor ) ) {
+				return;
+			}
+
 			// The plugin is already loaded or being loaded - do nothing.
 			if ( that.get( PluginConstructor ) || loading.has( PluginConstructor ) ) {
 				return;
@@ -99,7 +153,26 @@ export default class PluginCollection {
 				assertIsPlugin( PluginConstructor );
 
 				if ( PluginConstructor.requires ) {
-					PluginConstructor.requires.forEach( loadPlugin );
+					PluginConstructor.requires.forEach( ( RequiredPluginConstructorOrName ) => {
+						const RequiredPluginConstructor = getPluginConstructor( RequiredPluginConstructorOrName );
+
+						if ( removePlugins.includes( RequiredPluginConstructor ) ) {
+							/**
+							 * Cannot load a plugin because one of its dependencies is listed in the `removePlugins` option.
+							 *
+							 * @error plugincollection-required
+							 * @param {Function} plugin The required plugin.
+							 * @param {Function} requiredBy The parent plugin.
+							 */
+							throw new CKEditorError(
+								'plugincollection-required: Cannot load a plugin because one of its dependencies is listed in' +
+								'the `removePlugins` option.',
+								{ plugin: RequiredPluginConstructor, requiredBy: PluginConstructor }
+							);
+						}
+
+						loadPlugin( RequiredPluginConstructor );
+					} );
 				}
 
 				const plugin = new PluginConstructor( editor );
@@ -110,6 +183,14 @@ export default class PluginCollection {
 			} );
 		}
 
+		function getPluginConstructor( PluginConstructorOrName ) {
+			if ( typeof PluginConstructorOrName == 'function' ) {
+				return PluginConstructorOrName;
+			}
+
+			return that._availablePlugins.get( PluginConstructorOrName );
+		}
+
 		function assertIsPlugin( PluginConstructor ) {
 			if ( !( PluginConstructor.prototype instanceof Plugin ) ) {
 				/**
@@ -124,6 +205,24 @@ export default class PluginCollection {
 				);
 			}
 		}
+
+		function getMissingPluginNames( plugins ) {
+			const missingPlugins = [];
+
+			for ( const pluginNameOrConstructor of plugins ) {
+				if ( !getPluginConstructor( pluginNameOrConstructor ) ) {
+					missingPlugins.push( pluginNameOrConstructor );
+				}
+			}
+
+			return missingPlugins.length ? missingPlugins : null;
+		}
+
+		function mapToAvailableConstructors( plugins ) {
+			return plugins
+				.map( pluginNameOrConstructor => getPluginConstructor( pluginNameOrConstructor ) )
+				.filter( PluginConstructor => !!PluginConstructor );
+		}
 	}
 
 	/**

+ 176 - 21
packages/ckeditor5-core/tests/plugincollection.js

@@ -10,8 +10,8 @@ import Plugin from '../src/plugin';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 
-let editor;
-let PluginA, PluginB, PluginC, PluginD, PluginE, PluginF, PluginG, PluginH, PluginI, PluginX;
+let editor, availablePlugins;
+let PluginA, PluginB, PluginC, PluginD, PluginE, PluginF, PluginG, PluginH, PluginI, PluginJ, PluginK, PluginX;
 class TestError extends Error {}
 class ChildPlugin extends Plugin {}
 class GrandPlugin extends ChildPlugin {}
@@ -28,6 +28,8 @@ before( () => {
 	PluginG = createPlugin( 'G', GrandPlugin );
 	PluginH = createPlugin( 'H' );
 	PluginI = createPlugin( 'I' );
+	PluginJ = createPlugin( 'J' );
+	PluginK = createPlugin( 'K' );
 	PluginX = class extends Plugin {
 		constructor( editor ) {
 			super( editor );
@@ -41,14 +43,33 @@ before( () => {
 	PluginF.requires = [ PluginE ];
 	PluginE.requires = [ PluginF ];
 	PluginH.requires = [ PluginI ];
+	PluginJ.requires = [ 'K' ];
+	PluginK.requires = [ PluginA ];
 
 	editor = new Editor();
 } );
 
 describe( 'PluginCollection', () => {
+	beforeEach( () => {
+		availablePlugins = [
+			PluginA,
+			PluginB,
+			PluginC,
+			PluginD,
+			PluginE,
+			PluginF,
+			PluginG,
+			PluginH,
+			PluginI,
+			PluginJ,
+			PluginK,
+			PluginX
+		];
+	} );
+
 	describe( 'load()', () => {
 		it( 'should not fail when trying to load 0 plugins (empty array)', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.load( [] )
 				.then( () => {
@@ -57,7 +78,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should add collection items for loaded plugins', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.load( [ PluginA, PluginB ] )
 				.then( () => {
@@ -68,8 +89,20 @@ describe( 'PluginCollection', () => {
 				} );
 		} );
 
+		it( 'should add collection items for loaded plugins using plugin names', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ 'A', 'B' ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( 'A' ) ).to.be.an.instanceof( PluginA );
+					expect( plugins.get( 'B' ) ).to.be.an.instanceof( PluginB );
+				} );
+		} );
+
 		it( 'should load dependency plugins', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 			let spy = sinon.spy( plugins, '_add' );
 
 			return plugins.load( [ PluginA, PluginC ] )
@@ -83,8 +116,23 @@ describe( 'PluginCollection', () => {
 				} );
 		} );
 
+		it( 'should load dependency plugins defined by plugin names', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+			let spy = sinon.spy( plugins, '_add' );
+
+			return plugins.load( [ 'J' ] )
+				.then( ( loadedPlugins ) => {
+					expect( getPlugins( plugins ).length ).to.equal( 3 );
+
+					expect( getPluginNames( getPluginsFromSpy( spy ) ) )
+						.to.deep.equal( [ 'A', 'K', 'J' ], 'order by plugins._add()' );
+					expect( getPluginNames( loadedPlugins ) )
+						.to.deep.equal( [ 'A', 'K', 'J' ], 'order by returned value' );
+				} );
+		} );
+
 		it( 'should be ok when dependencies are loaded first', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 			let spy = sinon.spy( plugins, '_add' );
 
 			return plugins.load( [ PluginA, PluginB, PluginC ] )
@@ -99,7 +147,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should load deep dependency plugins', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 			let spy = sinon.spy( plugins, '_add' );
 
 			return plugins.load( [ PluginD ] )
@@ -115,7 +163,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should handle cross dependency plugins', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 			let spy = sinon.spy( plugins, '_add' );
 
 			return plugins.load( [ PluginA, PluginE ] )
@@ -131,7 +179,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should load grand child classes', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.load( [ PluginG ] )
 				.then( () => {
@@ -140,7 +188,7 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should set the `editor` property on loaded plugins', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.load( [ PluginA, PluginB ] )
 				.then( () => {
@@ -152,7 +200,7 @@ describe( 'PluginCollection', () => {
 		it( 'should reject on broken plugins (forward the error thrown in a plugin)', () => {
 			let logSpy = testUtils.sinon.stub( log, 'error' );
 
-			let plugins = new PluginCollection( editor );
+			let 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.
@@ -169,11 +217,13 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'should reject when loading a module which is not a plugin', () => {
-			let logSpy = testUtils.sinon.stub( log, 'error' );
+			class Y {}
 
-			let plugins = new PluginCollection( editor );
+			availablePlugins.push( Y );
 
-			class Y {}
+			let logSpy = testUtils.sinon.stub( log, 'error' );
+
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			return plugins.load( [ Y ] )
 				// Throw here, so if by any chance plugins.load() was resolved correctly catch() will be stil executed.
@@ -188,14 +238,114 @@ describe( 'PluginCollection', () => {
 					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-load:/ );
 				} );
 		} );
+
+		it( 'should reject when loading non-existent plugin', () => {
+			let logSpy = testUtils.sinon.stub( log, 'error' );
+
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ 'NonExistentPlugin' ] )
+				// 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-plugin-not-found/ );
+
+					sinon.assert.calledOnce( logSpy );
+					expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^plugincollection-plugin-not-found:/ );
+				} );
+		} );
+
+		it( 'should load chosen plugins (plugins and removePlugins are constructors)', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ PluginA, PluginB, PluginC ], [ PluginA ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( PluginB ) ).to.be.an.instanceof( PluginB );
+					expect( plugins.get( PluginC ) ).to.be.an.instanceof( PluginC );
+				} );
+		} );
+
+		it( 'should load chosen plugins (plugins are constructors, removePlugins are names)', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ PluginA, PluginB, PluginC ], [ 'A' ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( PluginB ) ).to.be.an.instanceof( PluginB );
+					expect( plugins.get( PluginC ) ).to.be.an.instanceof( PluginC );
+				} );
+		} );
+
+		it( 'should load chosen plugins (plugins and removePlugins are names)', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ 'A', 'B', 'C' ], [ 'A' ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( PluginB ) ).to.be.an.instanceof( PluginB );
+					expect( plugins.get( PluginC ) ).to.be.an.instanceof( PluginC );
+				} );
+		} );
+
+		it( 'should load chosen plugins (plugins are names, removePlugins are constructors)', () => {
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ 'A', 'B', 'C' ], [ PluginA ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( PluginB ) ).to.be.an.instanceof( PluginB );
+					expect( plugins.get( PluginC ) ).to.be.an.instanceof( PluginC );
+				} );
+		} );
+
+		it( 'should load chosen plugins (plugins are names, removePlugins contains an anonymous plugin)', () => {
+			class AnonymousPlugin extends Plugin {}
+
+			let plugins = new PluginCollection( editor, [ AnonymousPlugin ].concat( availablePlugins ) );
+
+			return plugins.load( [ AnonymousPlugin, 'A', 'B' ], [ AnonymousPlugin ] )
+				.then( () => {
+					expect( getPlugins( plugins ).length ).to.equal( 2 );
+
+					expect( plugins.get( PluginA ) ).to.be.an.instanceof( PluginA );
+					expect( plugins.get( PluginB ) ).to.be.an.instanceof( PluginB );
+				} );
+		} );
+
+		it( 'should reject when loaded plugin requires not allowed plugins', () => {
+			let logSpy = testUtils.sinon.stub( log, 'error' );
+			let plugins = new PluginCollection( editor, availablePlugins );
+
+			return plugins.load( [ PluginA, PluginB, PluginC, PluginD ], [ PluginA, 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-required/ );
+
+					expect( logSpy.calledTwice ).to.equal( true );
+				} );
+		} );
 	} );
 
 	describe( 'get()', () => {
 		it( 'retrieves plugin by its constructor', () => {
-			let plugins = new PluginCollection( editor );
-
 			class SomePlugin extends Plugin {}
 
+			availablePlugins.push( SomePlugin );
+
+			let plugins = new PluginCollection( editor, availablePlugins );
+
 			return plugins.load( [ SomePlugin ] )
 				.then( () => {
 					expect( plugins.get( SomePlugin ) ).to.be.instanceOf( SomePlugin );
@@ -203,11 +353,13 @@ describe( 'PluginCollection', () => {
 		} );
 
 		it( 'retrieves plugin by its name and constructor', () => {
-			let plugins = new PluginCollection( editor );
-
 			class SomePlugin extends Plugin {}
 			SomePlugin.pluginName = 'foo/bar';
 
+			availablePlugins.push( SomePlugin );
+
+			let plugins = new PluginCollection( editor, availablePlugins );
+
 			return plugins.load( [ SomePlugin ] )
 				.then( () => {
 					expect( plugins.get( 'foo/bar' ) ).to.be.instanceOf( SomePlugin );
@@ -218,18 +370,21 @@ describe( 'PluginCollection', () => {
 
 	describe( 'iterator', () => {
 		it( 'exists', () => {
-			let plugins = new PluginCollection( editor );
+			let plugins = new PluginCollection( editor, availablePlugins );
 
 			expect( plugins ).to.have.property( Symbol.iterator );
 		} );
 
 		it( 'returns only plugins by constructors', () => {
-			let plugins = new PluginCollection( editor );
-
 			class SomePlugin1 extends Plugin {}
 			class SomePlugin2 extends Plugin {}
 			SomePlugin2.pluginName = 'foo/bar';
 
+			availablePlugins.push( SomePlugin1 );
+			availablePlugins.push( SomePlugin2 );
+
+			let plugins = new PluginCollection( editor, availablePlugins );
+
 			return plugins.load( [ SomePlugin1, SomePlugin2 ] )
 				.then( () => {
 					const pluginConstructors = Array.from( plugins )