Synchronous calls of asynchronous methods causes errors

[Piotrek Koszuliński]

See e.g. #11295. Calling editor.destroy() right after (or before it's completed) CKEDITOR.replace() or editor.setData() causes errors.

That's because developer has to care about calling methods when previous finished. There are callbacks and events which notify developer when action is completed, but it's his duty to find which method can be called when.

However, not every developer understand problems with asynchronous methods and not in every case it's easy to handle this. For example when editor is loaded in one tab of some UI component and user switches between tabs too quickly, destroy() may be called before editor fully initialises.

On the other hand, if editor's methods would take care of that (e.g. destroy() would wait until initialisation finished), then API would start to work unpredictably. Developer would never know if destroy() will be done immediately or if it's going to wait until something (setData, initialisation, etc.) ends. This may be even worse situation than the current one.

We could make even longer step and make all editor's methods asynchronous and e.g. based on promises. Then everything would be fine... if you understand all of this :D.

Therefore, instead of forcing developers to understand some not trivial concepts to do basic stuff we can simply clarify this in docs. But there still will be cases in which it will be hard to handle editor state (like the tabs case).

Opinions needed.

---

More details about the current situation in ​http://dev.ckeditor.com/ticket/11502#comment:8

[Anders Johansson]

In the case of destroy() being called early, or perhaps DOM nodes simply being gone from the document, I think the editor only need to be more forgiving in the asynchronous functions.

It surely isn't expected to work after destroy() is called, and it would be nice if there was no error messages from pending calls that could not be aborted by destroy().

[Piotrek Koszuliński]

That's not a solution. Hiding errors is the last thing API should do.

[Anders Johansson]

What I'm trying to suggest is that it should not be an error to call destroy() at any time, or to add some other way to explicitly abort/ignore pending operations.

My use case here is like your example with tabed UI component, the CKEditor is only a part of a larger application. A lot of stuff can happen between starting the initialization of the editor and the execution of a delayed function coming back from setTimeout(f, 0);

The only workaround I can currently think of would be to move the editor to a hidden corner of the DOM and wait for it to finish whatever it is doing before it is killed. And that might not even work due to problems with the iframe loosing its state.

[Artur Delura]

Me as a new developer had a lot of trouble with this. I called destroy method before editor was fully initialized. First of all, I saw an error in wyswig plugin which tells me nothing. I commented out about 350 lines of my code to discover the problem (destroy method was the last thing I thougt about :) ).

I think that destroy method should handle asynchromous calls which are not done, and interrupt them. Or at least developer should see proper information what is wrong. It could save him a lot of time, and overall feeling about API would be better.

[Piotrek Koszuliński]

DUP reported: #11924.

[Jakub Ś]

#11747 was marked as duplicate.

[Piotrek Koszuliński]

I wrote a pretty detailed post about the current issues and problems with solving them in #12859. Quoting myself... :D

---

Why CKEditor fails when I try to call some methods one after another?

editor.setData( someData );
// somewhere later...
editor.destroy();

The answer, unfortunately, isn't simple. Otherwise we'd have this fixed a long time ago. This is a mix of history, backwards compatibility, architectural limitations and lack of good solutions.

First thing to notice is that this is not only editor.setData's problem, because other editor's methods could malfunction too. Editor.getData, editor.destroy, editor.setReadOnly, etc. are not going to work if editor is not in the ready state. Furthermore, this problem concerns some other classes as well - e.g. CKEDITOR.editable.

Second thing to notice is that this problem occurs not only when editor isn't loaded yet, but also when switching between states and setting data (both operations can be asynchronous). This introduces more variables to the equation and could even make the state checks pretty ugly. Fortunately, we introduced editor.status and editable.status (although, we did this pretty recently), so currently this wouldn't be that bad.

Another architectural aspect that would have to be taken into consideration is how the code is decoupled. For instance, the editor.setData method only fires an event. If we started checking the editable's state inside this method we would create a tight relations and could break some existing implementations.

Backwards compatibility is important for one more reason - we can make backward incompatible changes, but we must do them in a smart way. For instance, once it's done it should be final - we can't ship some other solution in the next major release if this one turns out to be unsatisfactory. This rises the stake pretty high.

The last thing, and actually the most unfortunate, is that throwing errors is not that good solution. It may tell you that you're doing something wrong, but it won't tell you when you can do it.

try {
	editor.setData( ... );
} catch ( err ) {
	if ( err instanceof CKEDITOR.editorNotReadyError ) {
		// apparently, it wasn't the right moment
		// ... and what now? Ok, let's wait for some imaginary "ready" event
		// (which doesn't exist right now):
		editor.once( 'ready', function() {
			editor.setData( ... );
			// ... actually, this could throw again if some other editor#ready
			// listener called setData too and haven't cancel the event.
		} );
	} else {
		throw err;
	}
}

Isn't it ugly? I think it's a terrible developer experience. A more useful approach would to be cache method calls and execute them when the editor reaches the correct state. But this would mean that for instance editor.destroy, which is currently a synchronous method, would become an asynchronous one because it can't be executed while editor is not ready. Moreover, to make all this possible to implement we would need to do serious changes not only in the API, but in the editor internals as well. Even the highly unsatisfactory solution with throwing errors requires significant effort to and may lead to ugly compatibility issues.

To sum up. I don't want say that fixing this in CKEditor 4.x is impossible, but for now we didn't have enough resources and well... courage, to attempt that. I would also risk a statement that it's late in CKEditor 4.x for such changes. But most importantly, we are aware of these problems and will definitely ​design CKEditor 5's API differently.