Introduced ObservableMixin#decorate() and support for fire()'s return values

[Reinmar]

Suggested merge commit message (convention)

Feature: Introduced ObservableMixin#decorate() and support for setting EmitterMixin#fire()'s return value by listeners. Closes ckeditor/ckeditor5#4958.

[pjasiun]

I would keep it simple: we don't expect many methods to be decorated at the same time, so I don't find it needed to add handling multiple methods at the same time by this function. Also, it would be a problem if we will want to add some parameters to it in the future.

[Reinmar]

I actually took that into consideration. I think that if we'd add options it would become an object anyway:

decorate( { name: 'foo', how: 'bar' } )

But perhaps you're right that support for decorating multiple methods at once is an overkill. I'll remove that.

[pjasiun]

It should be docs for fire not for listenTo. However, a comment here and next to the on method about how callback can set the return value would be fine (most probably a link to the return property docs.

[Reinmar]

Haha. Good catch.

[Reinmar]

When working on tests for ckeditor5-core I stumbled upon a case where I tried to spy on command.execute and the command was created using new Command(). This got me an ugly error because command.execute did exist, but the originalMethod property inside observablemixin.js didn't. That's because I forgot to implement the execute() method.

To avoid such issues I'll make decorate() throw an error.

[Reinmar]

Back on review.

[Reinmar]

One last doubt – the decorate() method could be moved to EmitterMixin because it only requires the event system. It semantically makes more sense in ObservableMixin but it means that classes may need to start mixing it instead of EmitterMixin to get this functionality (it was a case for DataController).

[szymonkups]

Wrong button :D