Merge pull request #1665 from ckeditor/1640

Other: Change `Conversion` class API. Closes #1640.

BREAKING CHANGE: The `Conversion#register()` method was removed from the public API. Use constructor parameters to pass dispatchers and `Conversion#addAlias()` to register an alternative conversion group for registered upcast or downcast dispatchers.

src/conversion/conversion.js

@@ -8,6 +8,8 @@
  */
 
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
+import UpcastHelpers from './upcasthelpers';
+import DowncastHelpers from './downcasthelpers';
 
 /**
  * A utility class that helps add converters to upcast and downcast dispatchers.
@@ -56,45 +58,69 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 export default class Conversion {
 	/**
 	 * Creates a new conversion instance.
+	 *
+	 * @param {module:engine/conversion/downcastdispatcher~DowncastDispatcher|
+	 * Array.<module:engine/conversion/downcastdispatcher~DowncastDispatcher>} downcastDispatchers
+	 * @param {module:engine/conversion/upcastdispatcher~UpcastDispatcher|
+	 * Array.<module:engine/conversion/upcastdispatcher~UpcastDispatcher>} upcastDispatchers
 	 */
-	constructor() {
+	constructor( downcastDispatchers, upcastDispatchers ) {
 		/**
+		 * Maps dispatchers group name to ConversionHelpers instances.
+		 *
 		 * @private
-		 * @member {Map}
+		 * @member {Map.<String,module:engine/conversion/conversionhelpers~ConversionHelpers>}
 		 */
-		this._conversionHelpers = new Map();
+		this._helpers = new Map();
+
+		// Define default 'downcast' & 'upcast' dispatchers groups. Those groups are always available as two-way converters needs them.
+		this._downcast = Array.isArray( downcastDispatchers ) ? downcastDispatchers : [ downcastDispatchers ];
+		this._createConversionHelpers( { name: 'downcast', dispatchers: this._downcast, isDowncast: true } );
+
+		this._upcast = Array.isArray( upcastDispatchers ) ? upcastDispatchers : [ upcastDispatchers ];
+		this._createConversionHelpers( { name: 'upcast', dispatchers: this._upcast, isDowncast: false } );
+
 	}
 
 	/**
-	 * Registers one or more converters under a given group name. The group name can then be used to assign a converter
-	 * to multiple dispatchers at once.
+	 * Define an alias for registered dispatcher.
 	 *
-	 * If a given group name is used for the second time, the
-	 * {@link module:utils/ckeditorerror~CKEditorError `conversion-register-group-exists` error} is thrown.
+	 *		const conversion = new Conversion(
+	 *			[ dataDowncastDispatcher, editingDowncastDispatcher ],
+	 *			upcastDispatcher
+	 *		);
 	 *
-	 * @param {String} name The name for dispatchers group.
-	 * @param {module:engine/conversion/downcasthelpers~DowncastHelpers|
-	 * module:engine/conversion/upcasthelpers~UpcastHelpers} conversionHelpers
+	 *		conversion.addAlias( 'dataDowncast', dataDowncastDispatcher );
+	 *
+	 * @param {String} alias An alias of a dispatcher.
+	 * @param {module:engine/conversion/downcastdispatcher~DowncastDispatcher|
+	 * module:engine/conversion/upcastdispatcher~UpcastDispatcher} dispatcher Dispatcher which should have an alias.
 	 */
-	register( name, conversionHelpers ) {
-		if ( this._conversionHelpers.has( name ) ) {
+	addAlias( alias, dispatcher ) {
+		const isDowncast = this._downcast.includes( dispatcher );
+		const isUpcast = this._upcast.includes( dispatcher );
+
+		if ( !isUpcast && !isDowncast ) {
 			/**
-			 * Trying to register a group name that has already been registered.
+			 * Trying to register and alias for a dispatcher that nas not been registered.
 			 *
-			 * @error conversion-register-group-exists
+			 * @error conversion-add-alias-dispatcher-not-registered
 			 */
-			throw new CKEditorError( 'conversion-register-group-exists: Trying to register' +
-				'a group name that has already been registered.' );
+			throw new CKEditorError( 'conversion-add-alias-dispatcher-not-registered: ' +
+				'Trying to register and alias for a dispatcher that nas not been registered.' );
 		}
 
-		this._conversionHelpers.set( name, conversionHelpers );
+		this._createConversionHelpers( { name: alias, dispatchers: [ dispatcher ], isDowncast: isDowncast } );
 	}
 
 	/**
-	 * Provides a chainable API to assign converters to conversion dispatchers.
+	 * Provides a chainable API to assign converters to conversion dispatchers group.
+	 *
+	 * If the given group name has not been registered, the
+	 * {@link module:utils/ckeditorerror~CKEditorError `conversion-for-unknown-group` error} is thrown.
 	 *
 	 * You can use conversion helpers available directly in the `for()` chain or your custom ones via
-	 * the {@link module:engine/conversion/conversion~ConversionHelpers#add `add()`} method.
+	 * the {@link module:engine/conversion/conversionhelpers~ConversionHelpers#add `add()`} method.
 	 *
 	 * # Using bulit-in conversion helpers
 	 *
@@ -149,7 +175,16 @@ export default class Conversion {
 	 * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers|module:engine/conversion/upcasthelpers~UpcastHelpers}
 	 */
 	for( groupName ) {
-		return this._getConversionHelpers( groupName );
+		if ( !this._helpers.has( groupName ) ) {
+			/**
+			 * Trying to add a converter to an unknown dispatchers group.
+			 *
+			 * @error conversion-for-unknown-group
+			 */
+			throw new CKEditorError( 'conversion-for-unknown-group: Trying to add a converter to an unknown dispatchers group.' );
+		}
+
+		return this._helpers.get( groupName );
 	}
 
 	/**
@@ -535,26 +570,28 @@ export default class Conversion {
 	}
 
 	/**
-	 * Returns conversion helpers registered under a given name.
-	 *
-	 * If the given group name has not been registered, the
-	 * {@link module:utils/ckeditorerror~CKEditorError `conversion-for-unknown-group` error} is thrown.
+	 * Creates and caches conversion helpers for given dispatchers group.
 	 *
 	 * @private
-	 * @param {String} groupName
-	 * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers|module:engine/conversion/upcasthelpers~UpcastHelpers}
+	 * @param {Object} options
+	 * @param {String} options.name Group name.
+	 * @param {Array.<module:engine/conversion/downcastdispatcher~DowncastDispatcher|
+	 * module:engine/conversion/upcastdispatcher~UpcastDispatcher>} options.dispatchers
+	 * @param {Boolean} options.isDowncast
 	 */
-	_getConversionHelpers( groupName ) {
-		if ( !this._conversionHelpers.has( groupName ) ) {
+	_createConversionHelpers( { name, dispatchers, isDowncast } ) {
+		if ( this._helpers.has( name ) ) {
 			/**
-			 * Trying to add a converter to an unknown dispatchers group.
+			 * Trying to register a group name that has already been registered.
 			 *
-			 * @error conversion-for-unknown-group
+			 * @error conversion-group-exists
 			 */
-			throw new CKEditorError( 'conversion-for-unknown-group: Trying to add a converter to an unknown dispatchers group.' );
+			throw new CKEditorError( 'conversion-group-exists: Trying to register a group name that has already been registered.' );
 		}
 
-		return this._conversionHelpers.get( groupName );
+		const helpers = isDowncast ? new DowncastHelpers( dispatchers ) : new UpcastHelpers( dispatchers );
+
+		this._helpers.set( name, helpers );
 	}
 }
 
@@ -605,48 +642,3 @@ function* _getUpcastDefinition( model, view, upcastAlso ) {
 		}
 	}
 }
-
-/**
- * Base class for conversion helpers.
- */
-export class ConversionHelpers {
-	/**
-	 * Creates a conversion helpers instance.
-	 *
-	 * @param {Array.<module:engine/conversion/downcastdispatcher~DowncastDispatcher|
-	 * module:engine/conversion/upcastdispatcher~UpcastDispatcher>} dispatcher
-	 */
-	constructor( dispatcher ) {
-		this._dispatchers = Array.isArray( dispatcher ) ? dispatcher : [ dispatcher ];
-	}
-
-	/**
-	 * Registers a conversion helper.
-	 *
-	 * **Note**: See full usage example in the `{@link module:engine/conversion/conversion~Conversion#for conversion.for()}`
-	 * method description.
-	 *
-	 * @param {Function} conversionHelper The function to be called on event.
-	 * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers|module:engine/conversion/upcasthelpers~UpcastHelpers}
-	 */
-	add( conversionHelper ) {
-		this._addToDispatchers( conversionHelper );
-
-		return this;
-	}
-
-	/**
-	 * Helper function for the `Conversion` `.add()` method.
-	 *
-	 * Calls `conversionHelper` on each dispatcher from the group specified earlier in the `.for()` call, effectively
-	 * adding converters to all specified dispatchers.
-	 *
-	 * @private
-	 * @param {Function} conversionHelper
-	 */
-	_addToDispatchers( conversionHelper ) {
-		for ( const dispatcher of this._dispatchers ) {
-			conversionHelper( dispatcher );
-		}
-	}
-}

src/conversion/conversionhelpers.js

@@ -0,0 +1,40 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module engine/conversion/conversionhelpers
+ */
+
+/**
+ * Base class for conversion helpers.
+ */
+export default class ConversionHelpers {
+	/**
+	 * Creates a conversion helpers instance.
+	 *
+	 * @param {Array.<module:engine/conversion/downcastdispatcher~DowncastDispatcher|
+	 * module:engine/conversion/upcastdispatcher~UpcastDispatcher>} dispatchers
+	 */
+	constructor( dispatchers ) {
+		this._dispatchers = dispatchers;
+	}
+
+	/**
+	 * Registers a conversion helper.
+	 *
+	 * **Note**: See full usage example in the `{@link module:engine/conversion/conversion~Conversion#for conversion.for()}`
+	 * method description.
+	 *
+	 * @param {Function} conversionHelper The function to be called on event.
+	 * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers|module:engine/conversion/upcasthelpers~UpcastHelpers}
+	 */
+	add( conversionHelper ) {
+		for ( const dispatcher of this._dispatchers ) {
+			conversionHelper( dispatcher );
+		}
+
+		return this;
+	}
+}

src/conversion/downcasthelpers.js

@@ -9,7 +9,7 @@ import ModelElement from '../model/element';
 
 import ViewAttributeElement from '../view/attributeelement';
 import DocumentSelection from '../model/documentselection';
-import { ConversionHelpers } from './conversion';
+import ConversionHelpers from './conversionhelpers';
 
 import log from '@ckeditor/ckeditor5-utils/src/log';
 import { cloneDeep } from 'lodash-es';
@@ -23,7 +23,7 @@ import { cloneDeep } from 'lodash-es';
 /**
  * Downcast conversion helper functions.
  *
- * @extends module:engine/conversion/conversion~ConversionHelpers
+ * @extends module:engine/conversion/conversionhelpers~ConversionHelpers
  */
 export default class DowncastHelpers extends ConversionHelpers {
 	/**

src/conversion/upcasthelpers.js

@@ -5,7 +5,7 @@
 
 import Matcher from '../view/matcher';
 import ModelRange from '../model/range';
-import { ConversionHelpers } from './conversion';
+import ConversionHelpers from './conversionhelpers';
 
 import { cloneDeep } from 'lodash-es';
 import ModelSelection from '../model/selection';
@@ -20,7 +20,7 @@ import ModelSelection from '../model/selection';
 /**
  * Upcast conversion helper functions.
  *
- * @extends module:engine/conversion/conversion~ConversionHelpers
+ * @extends module:engine/conversion/conversionhelpers~ConversionHelpers
  */
 export default class UpcastHelpers extends ConversionHelpers {
 	/**

tests/controller/datacontroller.js

@@ -39,8 +39,8 @@ describe( 'DataController', () => {
 
 		data = new DataController( model, htmlDataProcessor );
 
-		upcastHelpers = new UpcastHelpers( data.upcastDispatcher );
-		downcastHelpers = new DowncastHelpers( data.downcastDispatcher );
+		upcastHelpers = new UpcastHelpers( [ data.upcastDispatcher ] );
+		downcastHelpers = new DowncastHelpers( [ data.downcastDispatcher ] );
 	} );
 
 	describe( 'constructor()', () => {

tests/controller/editingcontroller.js

@@ -90,7 +90,7 @@ describe( 'EditingController', () => {
 			model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
 			model.schema.register( 'div', { inheritAllFrom: '$block' } );
 
-			const downcastHelpers = new DowncastHelpers( editing.downcastDispatcher );
+			const downcastHelpers = new DowncastHelpers( [ editing.downcastDispatcher ] );
 
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 			downcastHelpers.elementToElement( { model: 'div', view: 'div' } );