Can't destroy the editor within a context

[Mgsy]

Steps to reproduce

1. Download the example project - react-context-destroy-issue.zip.
2. Unzip, run npm install && npm run build.
3. Open samples/real-time-collaboration-for-react.html.
4. Click on the Show/hidden CKE button.

Actual result

The editor instance is not destroyed.

Expected result

The editor instance is destroyed.

Notes

It's a regression introduced in v5.0.5.

[Mgsy]

Some debugging notes

It seems that the issue has been introduced while working on #349. The original problem concerned a situation, where <CKEditorContext /> component was destroyed and this action triggered another destroy chain in the <CKEditor /> component. However, destroying <CKEditorContext /> is calling ContextWatchdog#destroy(), which is calling ContextWatchdog._destroy() that destroys all available watchdog instances. At the same time, the <CKEditor /> component was calling ContextWatchdog#remove(), which executes watchdog.destroy().

The above shows an issue, where some watchdog instances are destroyed within <CKEditorContext /> and <CKEditor /> components at the same time, resulting in a double-destroy process and operating on unavailable instances. 

To avoid this behavior, we introduced a fix that delegates the destroy process to ContextWatchdog#destroy() in case <CKEditor /> is running under the context (see the fix). Unfortunately, we forgot about a case, when the editor is removed from the context, without destroying the context. As a consequence, it's impossible to destroy a single instance, so the removed line:

this._contextWatchdog.remove( this._id );

is required.

Potential solution

Potentially, we can try to detect if destroy was triggered by <CKEditorContext /> or individual <CKEditor /> component. If it was done by <CKEditorContext />, do nothing, as the whole destroy process will be handled by ContextWatchdog#destroy(), otherwise, remove a single instance using the old code - this._contextWatchdog.remove( this._id );.

Unfortunately, we don't have any information on which component was removed, so it might be difficult to detect that the whole process should be handled by ContextWatchdog#destroy(). However, once ContextWatchdog#destroy() is called, it immediately changes the state to destroyed, so we might try to detect if ContextWatchdog#state is destroyed, which indicates that the destroy process was triggered by <CKEditorContext /> removal. If the state is ready, we may assume that the context is not destroyed and it is safe to destroy an individual watchdog instance using this._contextWatchdog.remove( this._id );.

One thing to consider - after destroying <CKEditorContext />, both <CKEditorContext /> and <CKEditor /> will call componentWillUnmount(), so there might be some race conditions, due to the asynchronous nature of the destroy process and the information about the state might not be available yet.

The above should fix #349 and the current issue.

I will try to push some PoC fix to the branch and share it here.

[Mgsy]

I have created a PoC for the above on branch i/354 - https://github.com/ckeditor/ckeditor5-react/compare/i/354.