Ckeditor5: Allow naming features

[Reinmar]

So far we decided to not name the features explicitly because it gave us flexibility when compiling the code. Despite that you can still access the feature through editor.plugins.get( FeatureConstructor ) or, if you loaded the feature using editor.plugins.load( 'featurename' ), using the name.

However, it seems that naming some features will allow extending the editor's API in a way which will be useful for developers integrating bundled editors. In such a case you... I wanted to write that you don't have a name, but then you do have a name. So you can always do editor.plugins.get( 'myApi' ) on a bundle, at least when you're using the Rollup bundler.

@jodator, @pjasiun – what's your case? Cause I'm a bit lost. If you use the Rollup bundler you should have no problems with accessing your features.

[jodator]

My use case is to expose some features on editor instance like other things are exposed (ie editor.ui.

Simplest way to do it is add another (preferable unique) key to the editor but this looks like bad practice:

class MyApi extends Feature {
    init() {
        this.editor.myApi = {
            doFoo: () => this.doFoo(),
            doBar: () => this.doBar()
        }
        doBar {}
        doFoo {}
    }
}

The reason for exposing some methods on editor is that I would like to use some of internal classes from editor code base, namely from engine/model but bundled editor does not expose internal classes.

Any other, standardized inside CKEditor 5 plugins, way of exposing some API will be acceptable:

editor.plugins.get( 'myApi' ).doFoo();
editor.plugins.get( 'myApi' ).doBar();

Now any bundled editor that will have plugin myApi will have that behavior defined so I can use them in my code just by linking do the bundled editor.js.

[Reinmar]

Any other, standardized inside CKEditor 5 plugins, way of exposing some API will be acceptable:

So this should work already.

[jodator]

So this should work already.

how? I mean that getting feature from bundled editor by name? Without having access to class?

[Reinmar]

Yyy... Or maybe not. I wanted to enable that in https://github.com/ckeditor/ckeditor5-dev-bundler-rollup/issues/1.

[pjasiun]

I wanted to write that you don't have a name, but then you do have a name. So you can always do editor.plugins.get( 'myApi' ) on a bundle, at least when you're using the Rollup bundler.

Yea, you're right. It should work :)

[Reinmar]

After a long battle, I agreed that to not piss everyone we should name features. The feature will have property name and that's it. It will always be available under such name in the plugin collection. The thing that won't change is that requires need to return constructors – because that's the only way we can load this code and it still makes great sense.

As for the names – they'd need to consider the whole package. We need to only decide whether it will be (for ckeditor5-typing/src/typing.js):

• typing/typing, which can also be written as a shorthand typing,
• ckeditor5-typing/typing

The first option is better because that's how they are called now everywhere. But I'm a bit unsure whether the shorthands will still be easy to implement.

[jodator]

Plus side note about ckeditor5-* names. Already discussed somewhere else but probably there will be external plugins that will not have ckeditor5- prefix in name (this requires compiler changes) and I don't remember if it is still considered.
So I think that going with ckeditor5-typing/typing would be future proof.

[pjasiun]

This issue is a huge pain in the neck. Whenever I try to prototype something I realized that I can not get any plugin from the editor in the HTML page, so I can not call any method of it or test any property.

[pjasiun]

It also makes debugging much harder since I can not get a plugin in the console.

[Reinmar]

Roger that. I'll reserve time for it in this iteration.

[Reinmar]

Already discussed somewhere else but probably there will be external plugins that will not have ckeditor5- prefix in name (this requires compiler changes) and I don't remember if it is still considered.

The ckeditor5-* prefix is required for the sake of... actually, it's just required :D

Without it, the compiler won't be able to understand which of the installed packages are cke5 packages and which are not. It can be configured to source additional packages from any directory, but that directory should contain ckeditor5-* for the sake of simplicity. I don't want logic like "if it starts with ckeditor5-* then * is the name, take the whole piece otherwise". We've got enough scenarios to cover anyway :P

[Reinmar]

I've got a problem with where to put the plugin name.

Cause it cannot be in Plugin.name (static property, like the current Plugin.requires), because this is a reserved function property (most likely we can override it but it'd be a mess). So there are three choices:

Plugin.id
Plugin#name // (instance property; inconsistent with Plugin.requires)
Plugin.pluginName

?

[Reinmar]

Or we go with overriding Plugin.name. That would mean that a plugin is always available under its constructor name, unless someone overriden it.

I should have mentioned that I wanted this property to be optional to not make testing irritating (you'd always need to define it when mocking a plugin). But perhaps using Plugin.name would be a solution to that. Or perhaps it should be required anyway, just for consistency.

[Reinmar]

  TypeError: Cannot assign to read only property 'name' of function 'class extends _plugin2.default {

So no... we cannot use Plugin.name.

[szymonkups]

Plugin.id :+1: from me.

[Reinmar]

What would you say about using the Plugin.name without overriding it? So e.g. Image extends Plugin {} will yield 'Image'. This way devs won't need to set it unless they want some special behaviour.

[Reinmar]

My problem with Plugin.id is that you're always talking about "plugin names", just like about "package names". And in CKEditor 4 we also have a name.

OTOH, there's item.id in the Collection class, so there would be some consistency in this direction.

[scofalik]

I am for using Plugin.name without overriding. It may feel like hacking, but it's just using of a useful property we already have access too. My only worry is that we could have two plugins named the same, but this shouldn't really happen in real case scenarios (are you really going to bundle editor with two Image plugins called same?).

+1 from me.

BTW. for me, id is (as name suggests) something that identifies given "thing" from other similar "things". I see it more as instance property than static member.

[szymonkups]

Is using Plugin.name obfuscation-safe? I mean, in future we might use some code minification tools, won't that be a problem?

[Reinmar]

Is using Plugin.name obfuscation-safe? I mean, in future we might use some code minification tools, won't that be a problem?

Good question... hm.

[Reinmar]

Yep, they can try to do so: https://github.com/babel/babili/blob/3b9e37e20fc6eba486719548f27e3c6aef81859f/packages/babel-plugin-minify-mangle-names/src/index.js#L281-L286

[scofalik]

Let's go with .pluginName then. .id is less explicit, you might expect some kind of number or unique hashed string there.

[Reinmar]

I've got one more idea. To wrap requires and name in e.g. SomePlugin.meta or SomePlugin.info. It would returns an object with requires and name properties.

This would look good from the plugin code perspective cause you'd only need one static getter.

Crazy or not?

[Reinmar]

class SomePlugin extends Plugin {
    static get info() {
       return { name: 'ckeditor5-some/some', requires: [ Foo, Bar ] };
    }
}

[scofalik]

Seems complicated and returns a bigger object each time. I am for two properties.

[Reinmar]

OK, thanks guys. Sorry for bothering you with such a stupid problem :D.

So we'll have pluginName.