Ckeditor5: Watchdog#create().then() is confusing

Created on 8 Jul 2019  Â·  10Comments  Â·  Source: ckeditor/ckeditor5

How many times will then() be executed? I assume, only once because it's a simple, right? So, what's the purpose of it?

watchdog docs question

Most helpful comment

Like this?

const watchdog = new Watchdog();

watchdog.setCreator( ( elementOrData, editorConfig ) => {
    return ClassicEditor
        .create( elementOrData, editorConfig )
        .then( editor => {
            // Do something with the new editor instance.
        } );
} );

watchdog.setDestructor( editor => {
    // Do something before the editor is destroyed.

    return editor
        .destroy()
        .then( () => {
            // Do something after the editor is destroyed.
        } );
 } );

watchdog.create( elementOrData, editorConfig );

All 10 comments

So, how can I easily handle the creation of a new editor instance – e.g. I may need to insert it to my app's DOM structure. The same with destruction.

I found only such a fragment in the docs:

It also means that you should not execute any actions on editor.create because these actions will not be executed when the editor restarts. Use watchdog.create instead, add a plugin and add your code in the plugin#init, or listen on watchdog#restart to handle restarts.

But it creates even more confusion.

The problem here is also that the method is called create(). So you expect it to create (and hence return) something. While, it shouldn't do that. Because you should not react on the one-time event creation of the editor. You should handle it by events instead.

Additional issue – right now you need to handle both create().then() and Watchdog#restart if there's any piece of code which should be executed for all new editor instances. Which, in turn, means that this event is not named too well.

You're right. It is incorrect. There are 2 options:

  1. We can remove create.then and tell users to use watchdog.setCreator instead. In the creator callback, they can put all of the custom initialization code they want. It will be called each time the editor is created. Same for watchdog.setDestructor.

  2. We can provide a custom implementation of this then method, so it will be called each time watchdog is created. However, it might be hacky and do not work as expected.

2. We can provide a custom implementation of this then method, so it will be called each time watchdog is created. However, it might be hacky and do not work as expected.

👎 Too confusing, IMO.

Also, I'd get back to the problem I pointed out previously that create() is not the most fortunate name here. It's actually where the problem begins.

We can provide a custom implementation of this then method, so it will be called each time watchdog is created. However, it might be hacky and do not work as expected.

Please, no.

How many times will then() be executed? I assume, only once because it's a simple, right? So, what's the purpose of it?

This shouldn't be mentioned in docs (the .then( () => {} ). However, I use it internally in the restart() method (I'm waiting for these promises), so IMO the promise should be returned but without any value inside and it might be not documented.

I also think that we should make it clear, that one should use watchdog.setCreator in such case.

OK, I'll document what we have currently in the best possible way possible now. I still don't like the naming because it's still confusing, but I'll open a new ticket for that.

Like this?

const watchdog = new Watchdog();

watchdog.setCreator( ( elementOrData, editorConfig ) => {
    return ClassicEditor
        .create( elementOrData, editorConfig )
        .then( editor => {
            // Do something with the new editor instance.
        } );
} );

watchdog.setDestructor( editor => {
    // Do something before the editor is destroyed.

    return editor
        .destroy()
        .then( () => {
            // Do something after the editor is destroyed.
        } );
 } );

watchdog.create( elementOrData, editorConfig );

Yep.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

msamsel picture msamsel  Â·  3Comments

Reinmar picture Reinmar  Â·  3Comments

pomek picture pomek  Â·  3Comments

oleq picture oleq  Â·  3Comments

oleq picture oleq  Â·  3Comments