Skip to content
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

Destruction of the editor when ViewCollection is adding children may end up in errors #5342

Closed
oleq opened this issue Apr 12, 2017 · 3 comments · Fixed by ckeditor/ckeditor5-ui#204

Comments

@oleq
Copy link
Member

oleq commented Apr 12, 2017

ViewCollection.add( child ) returns a promise but it doesn't belong to editor.create() and editor.destroy() chain. So it is possible to destroy the editor when ViewCollection.add( child ) promise is working (i.e. child.init() takes a long time to finish).

Each promise created by ViewCollection.add( child ) should be remembered in the property of the VC like this._addPromises = [ promises for each add() ] and removed from the array once resolved so when ViewCollection.destroy() is called, it should wait for Promise.all( this._addPromises ) to finish.

A follow-up of ckeditor/ckeditor5-link#104.

oleq referenced this issue in ckeditor/ckeditor5-ui Apr 13, 2017
…omises to resolve to avoid errors. Closes #203.
@Reinmar
Copy link
Member

Reinmar commented Apr 13, 2017

I believe @scofalik has recently run into this problem (and it might've been in the link tests). I think I was meant to report it but I can't see a ticket for that so... I'm glad you run into this too :P

@Reinmar
Copy link
Member

Reinmar commented Apr 15, 2017

@oskarwrobel
Copy link
Contributor

Not exactly but It was our first thought too. ckeditor/ckeditor5-link#94 was because I was trying to test asynchronous code by the synchronous test. The assertion was in the first event loop but the code was finishing asynchronous and the error occurred in the next events loop just after the test has finished. It has no impact to assertion but It was produced an error.

oleq referenced this issue in ckeditor/ckeditor5-ui Apr 19, 2017
Fix: ViewCollection#destroy should wait for all ViewCollection#add promises to resolve to avoid errors. Closes #203.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the iteration 10 milestone Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants