Race condition in initialization

[jbalsas]

There is a weird edge case discovered from https://issues.liferay.com/browse/LPS-69981. Even though the way to reproduce it is in fact an application issue, we can improve the editor to avoid it.

It is as follows:

• CKEditor's ScriptLoader.load may run the callback synchronously if the scripts were already loaded.
• If AlloyEditor.Strings is null, but the CKEditor lang files have been requested, then the callback to _renderUI will be invoked synchronously.
• Down the path, this means that we start rendering before setting the _editor property which is the one returned on props.editor.get('nativeEditor').

So, probably we just need to move the this._editor assignment above the loadLanguageResourcescall.

[ipeychev]

Interesting, that is a good catch and great analyze!
I remember I especially wrote this line to call the callback asynchronously even when the strings were loaded. I don't remember CKEditor to call the callback synchronously, either I misread it this time or they have changed it meanwhile.

Moving this._editor = editor; would probably solve the issue, but something worries me and it is this behavior here. It was always meant to call the callback asynchronously and never either synchronously or asynchronously. I would rather either remove this setTimeout, allowing the callback to be executed synchronously too, or call this callback asynchronously always too:

AlloyEditor.once('languageResourcesLoaded', function() {
    setTimeout(callback, 0);
});

Thinking now, maybe we can do the second one in addition to the change, which moves this._editor = editor;, i.e., always call the callback asynchronously. In this case, we have consistent behavior no matter of loaded or not loaded script, as it was always meant to be.

What about that?

[jbalsas]

Hey Iliyan!

Making sure the callback is always async makes total sense. This way we have more consistency and this won't come to bite us again if we ever forget and add more code under that 😂

Do you want me to push the change to the open PR?

[ipeychev]

Yes, Chema, thanks :)