Dependency injection

[Reinmar]

When doing some research on DI I stumbled upon this short, Angular related article. Despite explaining the problems with Angular's DI, it contains a very good explanation of what DI is:

Aside from being one of those things that reads like Comp-Sci lecture notes in the Angular docs, dependency injection is actually incredibly straightforward.

Let’s break out some pseudocode — say I have a notification component.

let notificationComponent = NotificationComponent({
    title: 'My Notification Widget!'
});

What if I want that notification component to do some logging every time we ask it to display a new notification? I could just have my component call whatever logger it wants — like console.log(), or AbstractLoggerBeanFactory.getInstance().log()

But that’s not great, because now my component is tightly coupled to some particular logging library or implementation. Besides which, logging is really only a secondary concern for this component, and having it make the decision of which logger to use seems like an awfully big responsibility for such a small component. Also — what if five different components I pull in each try to use a different logger? Then I’m kinda screwed.

So instead, let’s just pass down the logger we want it to use to our component:

let notificationComponent = NotificationComponent({
   title: 'My Notification Widget!',
   logger: window.console
});

That’s it. That’s dependency injection. I pass in the dependency, rather than my component deciding which dependency it wants to use. Pretty simple!

Of course, there are more things to consider; like, for example, the component could fall back to a default logger if one isn’t passed — and I also have to make sure whatever logger I use has a standard interface that the component can understand.

But at its core, DI is pretty simple. It’s just passing down dependencies.

To be honest, it's like the first time when I saw someone taking the easy way when talking about DI. Usually, DI is presented in context of some specific tooling and this is how people approach it and on what they focus.

So, even though in another article we can read:

The value of DI is that we can depend on abstract ideas instead of concrete implementations.

the whole article is about how to inject dependencies (techniques to do that). And of course it makes a lot of sense – how could we talk about DI if we don't talk about injecting dependencies? However, I feel that a really important aspect of DI is being diminished in all this tool-oriented talk – that the step 1 is always to create the right abstractions and leave code decoupled as long as possible.

How you inject your modules is a secondary aspect. It can be done using some nice programmatic tools which allow you to easily rewire code for tests but this is not critical. If you bind modules late you will be able to recompose most of the significant parts of the code without any tools. It will all boil down to passing different dependencies when initialising your components.

To the point

I started the topic because of https://github.com/ckeditor/ckeditor5-link/issues/108 – "How to override LinkFormView?".

My first reaction was – use Webpack! You want a different view, why bothering us to add more tools – use the existing tools. Configure Webpack to load a different module instead of the original LinkFormView and that's it. (I'm quite sure it is doable, but I'm too lazy now to check it. I only mention this as a possible solution if someone wants to try. Especially that this is a good solution taken the code size – this allows totally skipping the original module's code.)

Then I started thinking about late binding – why we even talk about overriding anything? If you don't want LinkFormView then you should implement your own Link plugin which uses a different view. Recomposing features is just so nicer than overriding something.

However, this approach is not really a viable option in the current state of things. The Link plugin can be neither given the LinkFormView nor easily reimplemented.

The first option is simple (even without special DI tools) – let's allow overriding Link#createFormView() (it'd need to be public). This is an OK solution, but it's a bit too limited. This is not the only dependency that anyone will want to replace so a more general solution should be found. Also, this doesn't cut off the dependency to the standard view so its code will be included in the bundle.

I like the second option much more – reimplementing the Link plugin. Right now, that would be painful – the Link not only initialises the LinkFormView and other subcomponents, but it also contains a lot of code.

Plugins are the perfect place to glue together a set of modules by initialising and injecting proper dependencies. A job of a plugin should be to, using a minimal code, build a feature out of available building blocks. The building blocks used by plugins should be left decoupled for as long as possible.

In other words, plugins should be focused on initialising and configuring other modules.

I wanted to show an example of how Link could be refactored, but, TBH, this is too much work for now because I'd also have to think how to improve the way buttons are created and how other components work together. However, if we take a look at the current Link#init() code we can see that it initialises 3 things (form and 2 buttons) and glue them together. It should do exactly that but the initialisation code should be minimal and the glue should be in a form of external modules (simple functions most likely).

You may remember that it sounds very similar to what I've been proposing a few months ago – that certain behaviours should be enclosed in form of simple helpers. That makes them reusable. We have the same situation here.

To sum up

If you want to override part of some existing plugin this should be your way of thinking:

• If I can reimplement a single module to do what I want (e.g. change the template), then let's inject it using Webpack (quick googling found me 1 and 2).
• If I need a more long-term and flexible solution I should use the fact that I can recompose an existing plugins by reusing some of its existing helpers and reimplementing the pieces that I want to totally change.

If you are implementing a plugin:

• Make it minimal – focus on initialising components and glueing them.
• Bind your components very late – ideally, the plugin should initialise all of them (components should not initialise other components – instead, those dependencies should be passed to them).
• Don't go crazy. Not everything needs to be reusable and not every component must be replaceable. Try to identify those things which will often change (e.g. views) and try to decouple them as much as possible. Be more lazy about stuff which is unlikely to be ever touched – e.g. no one will bother to replace the engine (or its parts). And if they did, there will be a lot of work anyway and you can't predict what kind of changes will be required anyway. For example, we've been at some point thinking what if someone wants to add more pipelines (besides the editing and data pipelines)? Well – they'd need to align all the existing features anyway because there's no way to make them generically aware of non-existent pipelines :D.

Plans for the future:

• Once we're out with 1.0.0 alpha we need to sit, look at the code we have and clean it up. We decided not to go with too much abstraction too early but this is also one of the reasons why some code may be hard to be reused now.

[pjasiun]

I recently had some thoughts about "plugin" classes, I want to share, loosely connected to this topic.

1. Plugins are controllers

For every part of the editor (except core and engine) which need a controller, a plugin should be created. I was thinking if we are no overusing plugins, if we should create plugins for instance for UI components, like a contextual toolbar. Such component might be an integrated part of the editor, not very pluggable (you should not be able to disable it). But I think that even in such cases using plugins is not very costly and give us a nice architecture for splitting the controlling flow, handling dependencies. Each plugin/controller can create a nice API for other which can be reached by plugins.get, each can require other.

2. There can be separate plugins for separate UI elements

We did it for upload. There is a separate plugin for the engine part of the upload, separate for the button which opens system file input dialog and separates for the upload animation. Thanks to it every part of the UI can be easily replaced, but also it gave us very nice code splitting (see https://github.com/ckeditor/ckeditor5-upload/blob/be7cf3942b1ea236bef07062ff39ab726b2e4f0a/src/imageupload.js and imagine how it would look like if this plugin would handle the whole UI).

3. There is nothing wrong in the plugin which only aggregates other plugins

Note that, thanks to our architecture, we can create a plugin which does nothing but requires a list of other plugins. Thanks to it the "feature" can be a set of concrete behaviors (engine, all needed UI elements).

[pjasiun]

And I need to add that injecting js files using Webpack outside tests smells bad. I can only imagine how a new developer feels when after hours of debugging he realized that one file is overwritten.

[Reinmar]

Related: https://github.com/ckeditor/ckeditor5-upload/issues/22#issuecomment-333090113

Compare this to inheritance chains. Requiring one plugin in another one is a bit like saying that one plugin inherits from the other one. The less often we do that, the better.

This is one more thing which we should avoid. Plugins should require other plugins as infrequently as possible. I think that requiring other plugin is good when that other plugin is a library (like FileRepository or Notification). In this case you rely on this specific plugin. But when you rely on some schema definitions, command (name), converters perhaps, then you should not rely on a spefic plugin. You, instead, rely on someone implementing and loading this (or similar) functionality.

Then, we may have "load them all" plugins which role will be to load all necessary pieces of a feature to make it easy for the developers to enable all the parts.

[Reinmar]

An example application of these rules to the ContextualToolbar problem that we had: https://github.com/ckeditor/ckeditor5-ui/issues/274#issuecomment-336176205.

[Reinmar]

Another example how the Link feature could be refactored so the views are bound late with the feature.

As you can see it will be really simple to reuse the link UI's logic with a different view (see the MyLink plugin):

class LinkUIController {
	constructor( editor, options ) {
		this.formView = options.formView;
		...
	}
}

class LinkUI extends Plugin {
	init() {
		this.controller = new LinkUIController( editor, {
			formView: new LinkFormView()
		} );
	}
}

class Link {
	static get requires() {
		return [ LinkEditing, LinkUI ];
	}
}

class MyLink extends Plugin {
	static get requires() {
		return [ LinkEditing ];
	}	

	init() {
		this.controller = new LinkUIController( editor, {
			formView: new MyLinkFormView()
		} );
	}
}

[pjasiun]

I finally found some time to read about DI ideas and I don't like the above concept, and there are reasons why:

It will grow

The main problem is, that when the dependencies chain groves you need to pass such reference down to all children, or, thinking contrariwise, pass the constructor up and up. It means that if you have, for instance, a UI component for comments sidebar, which is built from the comments thread components, and each has form components for comments, edit form, etc., then you need to pass all of them in the main constructor:

class CommentSidebar extends Plugin {
	static get requires() {
		return [ CommentsEditing ];
	}	

	init() {
		this.controller = new CommentSidebarUIController( editor, {
			commentView: new CommentView(),
			commentEditView: new CommentEditView(),
			commentRemoveViev: new CommentRemoveViev(),
			commentThreadView: new CommentThreadView(),
			addCommentToThreadView: new AddCommentToThreadView(),
			confirmCommentRemovealView: new ConfirmCommentRemovealView(),
			commentSidebarView: new CommentSidebarView(),
			// ...
		} );
	}
}

class CommentSidebarUIController {
	constructor( editor, options ) {
		this.commentThreadController = new CommentThreadController( editor, {
			commentView: options.commentView,
			commentEditView: options.commentEditView,
			commentRemoveViev: options.commentRemoveViev,
			commentThreadView: options.commentThreadView,
			addCommentToThreadView: options.addCommentToThreadView,
			confirmCommentRemovealView: options.confirmCommentRemovealView,
			// ...
		} );
	}
}

class CommentThreadController{
	constructor( editor, options ) {
		this.commentUIController = new CommentUIController( editor, {
			commentView: options.commentView,
			commentEditView: options.commentEditView,
			commentRemoveViev: options.commentRemoveViev
		} );
	}
}

// etc.

I think this is the reason, why all frameworks introduce an advanced mechanism for dependency injection.

Note that, this is a pretty simple case where the list of child views is static. If it will be dynamic the code we get even more complicated.

Interface have to be the same

Another problem is that injected view have to have the same interface that the original class. It's not that simple for instance for the ButtonView and its 15 public properties (I met this problem already). And inheritance does not solve this problem because you need to match parent class interface then. Both sides (original view and overwritten one) need to change in the same way, what discourages us to change these views in the future.

I prefer creating class connections in the logic level, not on the interface level. As original MVC proposed, it is a controller who should combine model and view. This way you will know how your views interface is used.

One more mechanism

We already have 2 mechanisms in CKE5 to decrease the number of dependencies: plugins and events (including observables). Here you introduce another one. Note that it won't be independent. For instance, a toolbar will have injected its view this way, but buttons will be injected using plugins. And the buttons view will be injected using this mechanism but within injected plugins. In fact, as I think about it, named plugins available through the context ("editor.plugins") is the mechanism very similar to the injector in many DI solutions.

One more class

Implementing DI as you proposed you need to introduce one more class in each, even a simple plugin. There is no other reason not to have plugin being controller, as I mentioned before. You need to create the separate controller class and special this.controller property. Quoting article you linked here: "In doing do it makes your code a lot harder to read and reason about — even the parts which don’t need DI".

Not much in exchange

Quoting another smart guy. "Don't go crazy. Not everything needs to be reusable and not every component must be replaceable.". When we will move more logic to the "editing" part of the plugin, it might be relatively simple to create your own UI part.

Moreover, at the end of the day, if you really need to overwrite views, you can always use webpack. I think that DI comes in pair with the module system. So, even if we find it ugly, it might be the proper way to go, when you really need to overwrite a view.

Summary

I would stick to the original plan: improve the split between editing and UI parts of a plugin, create proper utils, recommend users to use webpack to overwrite classes and... wait and see if there are any problems that cannot be solved this way. We may realize that all problems are solved or will find more specific issues, for instance, it might be enough to make templates overwritable.

[Reinmar]

From the current perspective, it seems that the split to editing and UI parts + the glue plugin is satisfactory. I don't think that we need more work here on the platform level.