Ckeditor5: Implement options extraPlugins and removePlugins

[Reinmar]

This is a response to https://github.com/ckeditor/ckeditor5-dev-bundler-rollup/issues/1. The ability to adjust which plugins are loaded from a bundle is implemented in exactly the same way in CKEditor 4 and I don't see another, simpler solution.

PS. Since plugins are named now (https://github.com/ckeditor/ckeditor5-core/issues/27), this option will be easy to use.

[fredck]

Right now, it would be done by the plugins option, where one has to list all plugins (expect dependencies, which are loaded automatically), right?

[Reinmar]

Well, it's the same problem as in CKEditor 4. If a bundler provides the default plugins it needs to do that through config.plugins. So if you take a bundle and specify config.plugins, you're overriding the existing ones and since the code of these plugins is provided through config.plugins in the first place, you need to do the same – so using strings won't work.

So there are two options:

Either we can implement config.extraPlugins and config.removePlugins to tune up the bundled config.options (as proposed above).

Or we can try to take a different path, about which I forgot when reporting this ticket. We can add config.pluginMap (or something like that) option to be used only by bundlers. It would let the bundler compile-in plugins into a package, while leaving config.plugins to be used by developers. Then, config.plugins could be an array of plugin names and the plugin source would be taken from config.pluginMap.

WDYT? When I'm thinking about this now, I like the second option more as it's pretty transparent to the developers. We don't reserve the config.plugins option for bundlers like we did in CKEditor 4.

[fredck]

+1

I would call it pluginsMap, if this is a list of 'pluginName': pluginConstructor "mappings" or pluginClasses, if it is just a list of constructors.

[Reinmar]

pluginsMap or pluginMap? Isn't the latter correct?

[fredck]

pluginsMap because it is the "Map of Plugins`.

[Reinmar]

AFAIK, it should be singular if it works as an adjective. E.g. @AnnaTomanek was telling us to e.g. have PluginCollection (which is a collection of plugins).

[fredck]

While in a collection you have a "basket full of plugins" in the "map" you have a "basket full of plugin mappings"... it's a different context. So for me it is either pluginMappings or pluginsMap.

[Reinmar]

Oh god... :D

[Reinmar]

In order to simplify creating builds, I'd like the pluginsMap to be a static property of the editor class. Also, it doesn't have to (and can't) be a map – it needs to be a single array. This is because the builder, when creating an entry file, may not know the names. It'd need to parse plugins code to extract the names which is an unnecessary complication.

This will allow creating such an entry file to bundle:

import BaseClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import PluginA from '...';
import PluginB from '...';
import PluginC from '...';

export default class ClassicEditor extends BaseClassicEditor {
    static get availablePlugins() {
        return [ PluginA, PluginB, PluginC ];
    }
}

And then, if you use the editor class like:

ClassicEditor.create();

All the available plugins will be used.

Or, you can list the plugins you want to use:

ClassicEditor.create( {
    plugins: [ 'plugina/plugina', 'pluginb/pluginb' ]
} );

We can also later think about introducing removePlugins, but it's not necessary in the beginning.

NOTE: If PluginA requires PluginDDD (which is not present in ClassicEditor#availablePlugins), that plugin should also be available by name.

[Reinmar]

Or, a small amend – instead of a property a static setBundledPlugins() and getBundledPlugins() methods would be better, because the latter could already returned a map of all bundled plugins.

The methods would be used like this:

import BaseClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import PluginA from '...';
import PluginB from '...';
import PluginC from '...';

export default class ClassicEditor extends BaseClassicEditor {}

ClassicEditor.setBundledPlugins( [ PluginA, PluginB, PluginC ] );

And later, somewhere inside the editor's constructor (although, I'm unsure how safe is accessing constructor when it comes to transpilation to ES5 ;|):

this.constructor.getBundledPlugins(); // [ PluginA, ... ]

[Reinmar]

I needed to ensure myself that this won't blow up with inheritance and everything (that constructors inherit from one another too):

[Reinmar]

this.constructor.getBundledPlugins().map( plugin => plugin.pluginName ); // [ 'plugina/plugina', ... ]

[Reinmar]

Or yet another proposal:

import BaseClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import PluginA from '...';
import PluginB from '...';
import PluginC from '...';

export default class ClassicEditor extends BaseClassicEditor {}

ClassicEditor.bundledPlugins = [ PluginA, PluginB, PluginC ]; // (or with a static getter for consistency)

And if you want to get all the available plugins (so the bundled one with their dependencies) then instantiate an editor and use Array.from( editor.plugins ):

ClassicEditor.create()
    .then( ( editor ) => {
        Array.from( editor.plugins ).map( plugin => plugin.pluginName );
    } );

[Reinmar]

The reason why I thought it doesn't make sense to have setBundledPlugins() and getBundledPlugins() which returns already resolved dependencies is that the config.removePlugins must be matched with the set of bundled plugins.

E.g. if bundledPlugins constains Image, then you can't remove ImageEngine anyway, because it's required by Image anyway. So the only plugins you can work with in removePlugins are those in bundledPlugins. Unfortunately, the same will be true for presets – if you use an ArticleEditor preset but don't want one of the plugins, you won't be able to remove it. Hence, builds should rather not use presets because in this case presets reduce configurability.

[Reinmar]

Time for a final proposal.

Related tickets:

• https://github.com/ckeditor/ckeditor5-core/issues/67 – Editor.build property.
• https://github.com/ckeditor/ckeditor5/issues/408 – CKEditor 5 builds.

In order to add support for the new options config.extraPlugins and config.removePlugins and to add support for setting config.plugins by names, we need to:

1. Pass available plugins to PluginCollection constructor. This is to provide the PluginCollection a list of plugin constructors from which it can then get constructors when config.plugins/removePlugins/extraPlugins are defined by names. It's fine if right now we support loading/unloading by name only these plugins which were directly specified in availablePlugins (they can have deps which we can ignore now).
2. PluginCollection#load() needs to start supporting plugins specified by name.
3. I wonder how to support removePlugins. I think that it can either be passed to the PluginCollection constructor or to load(). The latter may be a bit cleaner. Note: it will be a bit strange to call load( pluginsToLoad, removePlugins ), but the editor can't filter the list itself, so it will be useful if PluginCollection did it for it.
4. If a plugin is defined in removePlugins by name or constructor then it must not be loaded even though being passed to load( pluginsToLoad ).
5. If a plugin was listed in removePlugins and is required by another plugin which weren't added to that list an error must be thrown.

[pomek]

I wonder how to support removePlugins. I think that it can either be passed to the PluginCollection constructor or to load().

I'm for the second option - pass the plugin names directly to the load() method.

I'm also thinking about renaming the removePlugins to ignorePlugins. For me, removePlugins option sounds like "plugins to deletion". We do not want to remove anything. We just want to ignore these plugins during loading the rest.

Does it make a sense?

[Reinmar]

In the config object it should be removePlugins because you're removing plugins from a build. Plus, it's like this in CKE4. In the PluginCollection it could be both.

[pomek]

because you're removing plugins from a build.

Not exactly. When I'm creating a new instance of an editor, I'm ignoring the plugins that are built-in the bundled package. I don't want to use all plugins. I want to use some part of them, so I'm ignoring the rest. I cannot remove the plugins because they are compiled in the sources but I can ignore part of them.

I'll do as you wish but it's my point of view.

[fredck]

@pomek, I understand your POV, but we have the very same concept in CKEditor 4 and therefore it would be good to use the same configuration name here: removePlugins

[Reinmar]

Turns out that https://github.com/ckeditor/ckeditor5-core/pull/70 didn't introduce config.removePlugins option. We missed it at the end and added it to the PluginCollection only.