Get rid of soft requirements #2910
Labels
package:core
status:discussion
type:improvement
This issue reports a possible enhancement of an existing feature.
type:task
This issue reports a chore (non-production change) and other types of "todos".
Milestone
Background: https://github.com/ckeditor/ckeditor5-core/issues/192 and https://github.com/ckeditor/ckeditor5-core/issues/148.
There are two types of plugins which we're not currently listing in
requires()
:Optional ones – those which may change the logic of your plugin when they are loaded, but which are not necessary for your plugin to work.
Example: if
Notification
is loaded, use its API to display a warning. Do nothing otherwise.Soft (dynamic) dependencies – those which are in fact required for your plugin to work correctly, but we didn't want to specify which implementation of this functionality we depend on. Instead, a bit like in "program to an interface" we wanted to depend on an interface. The fact that a plugin called
'Clipboard'
will be available, but we don't need to assume which instance it is precisely.The first mechanism works fine. It was improved in https://github.com/ckeditor/ckeditor5-core/issues/148 (
plugins.get()
throws, so you get the error ASAP).The second might be improved (with soft requirements – using strings in
requires()
as initially discussed in https://github.com/ckeditor/ckeditor5-core/issues/148), but it's actually not used by us. I checked @jodator's research (https://github.com/ckeditor/ckeditor5-core/issues/148#issuecomment-445794316) and did some more and we use this approach in very few cases.Therefore, let's agree that we don't use soft requirements. If you depending on a clipboard, define it by a class in your requirements. This isn't ideal, but this is how reality works, so I have no intention to deny it that :D.
Also, there's always the option to use this technique with
NormalModuleReplacementPlugin
to inject a different implementation by tuning webpack's configuration. While less convenient, the option exists, so no one will be locked.What needs to be done?
We need to use
requires()
in places where it's missing now – this can be found by scanning forplugins.get( '
and filtering out the "optional dependency" cases.The text was updated successfully, but these errors were encountered: