-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do we need support for asynchronous components initialisation? #5355
Comments
The longer we maintain this out-of–the–box async UI component support, the more I consider it a burden rather than a useful feature. It complicated tons of tests in the past and we haven't seen a single benefit of it yet. The problem is especially visible when some async code is executed in sync event listeners like Certainly, getting rid of it will shorten and simplify lots of code (and tests). |
Another example of the problem our async–driven API has created: the destruction of the editor, mostly in tests. Both UI initialization and destruction processes are chains of Promises. They work fine but when the editor is already running, there's no (easy) way to synchronize things like To handle this issue, we'd need to create an UI structure, sort of queue, which would inform the outside world about upcoming operations and make it wait for them to finish, e.g. before destruction or something like this. It feels like a lot of work with no clear benefit and, besides, the tests would still remain very complex. |
Other: Made the UI component initialization and destruction processes synchronous. Closes #225. BREAKING CHANGE: `View#init()`, `View#destroy()` (also `ViewCollection#init()`, `ViewCollection#destroy()` and `ViewCollection#add()`) no longer return `Promise`. It may require updates in UI components which inherit from `View` and rely on the value returned by these methods. BREAKING CHANGE: Various UI components switched to synchronous `init()` and `destroy()` (no longer returning `Promise`), which means that features using these components may need some updates to work properly.
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…synchronous initialization and destruction (see https://github.com/ckeditor/ckeditor5-ui/issues/225).
…iew() helper became synchronous (see ckeditor/ckeditor5-ui#225).
Long, long time ago we assumed that our UI framework needs to support async components. An example of an async component is one which uses an iframe. For it to be useful, ready to work, we need to wait for the iframe's load event. Otherwise, we wouldn't be able to put child components inside this one (if it had some collection).
So, examples of async components are, for instance:
In order to support this, we split component initialisation into two parts – the constructor and potentially async
init()
(it can return a promise). We also alloweddestroy()
to be asynchronous (just in case). All this meant thatViewCollection.add/destroy()
are asynchronous too. (I noticed thatremove()
isn't async... which may be a bug in this plan, cause removing an iframe unloads it, so it's close to destroy :D).It'd be good if this was where the asynchronous execution would end. But if one of your methods returns a promise then you need to carry it around everywhere and all your code needs to deal with an asynchronous execution. I report this ticket exactly because we noticed that a high level component (
ContextualBalloon
) has a problem with it.Do we need this at all?
The first question which we should ask immediately is – do we need all these problems at all? Do we really need to support asynchronous components initialisation?
The idea behind creating our own UI framework was that we have a very limited and quite specific use case so we might be able to come up with something specialised and simple at the same time. Async support works against this. It heavily complicates our code.
At the same time, we don't have any async component yet and, even though we implemented iframed editable in the past to see how the concept works, I'm pretty sure that we missed something (just like the
remove()
issue).So, perhaps we can drop support for async initialisation which will allow simplifying the code significantly. It will not only affect the UI lib but also features.
But what about those special, rare cases where we really need to deal with asynchronousity?
Who should be concerned about component's state?
Disclaimer: It may be a total BS what I'll write here because I didn't do any research and I haven't tested this concept, but let's see.
The idea about Web Components is dear to me. It assumes that every component is totally independent which makes it reusable and allows easy composition. Components should have, therefore, a clear and simple API. Just like the entire DOM.
But how does DOM handle asynchronousity? Well, it doesn't (I'm not sure about Web Components, though. @Comandeer?).
If you created an
<iframe>
and added it to the DOM usingappendChild()
it's your duty to postpone any further execution until theload
event.appendChild()
doesn't return a promise too. Nothing like that.This means that the platform is simple (yeah... DOM is so simple!) and the potential complexity is shifted to its users. After all, how often do you deal with iframes? Why making the entire API async if you need to deal with async cases once in a lifetime?
Let's also look on this from a different angle. If we have
ContextualBalloon
which delegates the add/remove API of its internal container, then who really needs to care about the fact that an iframe added to theContextualBalloon
is not ready immediately? Certainly not theContextualBalloon
because it only cares about<iframe>
being its child. It's the code which creates that iframe component and uses it. So it's the caller, not the callee.So, tl;dr here is that we made sure that the entire tree of components and the API needs to be aware of the state of some component that may be added to it. But we should do the exact opposite – save everyone from this knowledge and only make the smallest possible piece of code (usually, some glue code or controllers) aware of it.
The text was updated successfully, but these errors were encountered: