-
Notifications
You must be signed in to change notification settings - Fork 40
1640: Remove conversion.register() add conversion.addAlias() #1665
Conversation
src/conversion/conversion.js
Outdated
register( name, conversionHelpers ) { | ||
if ( this._conversionHelpers.has( name ) ) { | ||
addAlias( alias, dispatcher ) { | ||
const groupEntry = [ ...this._groups.entries() ].find( ( [ , dispatchers ] ) => dispatchers.includes( dispatcher ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be simplified :)
src/conversion/conversion.js
Outdated
|
||
const ConversionHelper = this._helpers.get( groupName ); | ||
|
||
return new ConversionHelper( this._groups.get( groupName ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it efficient to create a new instance whenever the converter is used? Maybe we could store instances of helpers?
Since we only have So the both groups are stored in separate private fields (easier & faster to find if dispatcher was registered in one of group - simpler code). The dispatcher groups/aliases are stored in |
Suggested merge commit message (convention)
Other: Change
Conversion
class API. Closes ckeditor/ckeditor5#4467.BREAKING CHANGE: The
Conversion#register()
method was removed from public API. Use constructor parameters to pass dispatchers andConversion#addAlias()
to register an alternative conversion group for registered upcast or downcast dispatcher.Additional information
As agreed in https://github.com/ckeditor/ckeditor5-engine/issues/1640#issuecomment-461078858 I've changed the conversion API to:
constellation of change on ckeditor5#t/ckeditor5-engine/1640 branch and the CI is green
Related PR In the core: Align code to the changes in Conversion API. ckeditor5-core#162.