React 19 support

[Mati365]

Suggested merge commit message (convention)

Add support to React 19 and create demos. Adjust context format to prevent race conditions in CKEditor component.

Additional Information

Collaboration samples update PR: https://github.com/cksource/ckeditor5-collaboration-samples/pull/146
Closes:

1. React 19 support #478
2. shouldComponentUpdate incorrectly async #312
3. CKEditorContext erorrs with next.js dev server #485

May fix some of these bugs that seems to be related to Context:

1. Double rendering in StrictMode when using Context #476 (race condition)
2. Investigate double-rendering in React StrictMode #294 (race condition)
3. Option to disable watchdog on <CKEditorContext> #409 (exported ContextWatchdogContext and useCKEditorWatchdogContext so now it is possible to implement own version of context used by CKEditor)

New features

1. Export context access hook useCKEditorWatchdogContext

const Component = () => {
  const context = useCKEditorWatchdogContext();
  // ...
};

2. Export context ContextWatchdogValue typings and guards (isContextWatchdogValue, isContextWatchdogValueWithStatus).

export type ContextWatchdogValue =
	| {
		status: 'initializing';
	}
	| {
		status: 'initialized';
		watchdog: ContextWatchdog;
	}
	| {
		status: 'error';
		error: ErrorDetails;
	};

3. Export context provider ContextWatchdogContext.

const Component = () => {
   // ...
   return (
      <ContextWatchdogContext.Provider value={yourCustomContext}>
        ..
      </ContextWatchdogContext.Provider>
   );
};

❓ Exports above allow slightly better customization of contexts used by CKEditor component. Especially mentioned #409 in this thread.

Long story short

New installation methods

All demos have been moved to new installation methods and use ckeditor5 dependency.

CKEditorContext crashes

CKEditorContext was using a class approach to define the component. However, with React 19, there have been changes in the calling order of lifecycle methods, which introduced race conditions in watchdog context creation that were fully reproducible in our demos. You can find an example here.

This error occurred because CKEditorContext created two instances of CKEditorContext in Strict Mode, resulting in the first watchdog instance being used in the children CKEditor component instead of the second one.

It was difficult to store additional information between the renders in a class component (constructor is called two times in strict mode), so I decided to refactor the component to a functional type. Unlike class components, functional components can hold some data between strict mode re-renders. Now, we store the last watchdog ID in a useRef and then check if the fully initialized watchdog ID matches the last requested watchdog ID. If it does, it is set to the state; otherwise, it is destroyed.

Child CKEditorContext components, such as CKEditor component, incorrectly implemented detection of context state. After mounting of child, it was unknown if child was placed in CKEditorContext or not because CKEditorContext default value (while being initialized in async lifecycle methods) was null. I added initializing state to check if child is being initialized in CKEditorContext.

⚠️ Possible breaking changes

1. The CKEditorContext is no longer a class component, so these statements (which were highly error-prone and undocumented in our documentation) are no longer functional:

const contextRef = useRef();

useEffect( () => {
   // Right now it is not possible to access `contextWatchdog` anymore and it will return `undefined`.
   const { contextWatchdog } = contextRef.current;
   
   // ...
}, [] );

<CKEditorContext
   ...
   ref={ contextRef }
/>

3. The status field has been added to the context type, indicating when the watchdog has been fully initialized. Current Context format:

export type ContextWatchdogValue =
	| {
		status: 'initializing';
	}
	| {
		status: 'initialized';
		watchdog: ContextWatchdog;
	}
	| {
		status: 'error';
		error: ErrorDetails;
	};

It simplifies deduction and prevents race condition in reusing watchdog logic inside components. Especially here. From now, the CKEditor component is waiting for fully initialization of watchdog to start bootstrapping editor.

However, CKEditorContext (React Context) was not exported from the package, so it should not be used by external integrations. I would mark it as a potential breaking change because there is one additional re-render due to the change from initializing to initialized state set in the context.

useMultiRootEditor crashes

It was impossible to register new root in React 19 due to this exception. It happened because EditorEditable was rendered two times in strict mode and second execution was performed on the DOM element that was already assigned to editor.

[coveralls]

Pull Request Test Coverage Report for Build 2bc51473-7e69-462a-a7b2-65b22bfb53b7

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build d747c632-24f1-4d68-8f5a-d567b67a0b37:	0.0%
Covered Lines:	70
Relevant Lines:	70

---

💛 - Coveralls

[Mati365]

@scofalik / @DawidKossowski No regressions according to QA testing results.

[Mati365]

Do not force rebase, this branch will be used by new installation methods.

[Mati365]

Note after call: It'll be merged after releasing new installation methods.

[Mati365]

PR should now work with new installation methods

[Mati365]

@f1ames Can you take a look at this PR? I've heard that you worked with React before, it'd be nice to hear additional feedback about changes. Thanks!

[DawidKossowski]

Several additional things that I found:

1. Unit tests produce a lot of errors in the console:
   
   Is it the desired behavior?

2. I got this error once in the Context demo (React 19):
   
   However, I am no longer able to reproduce it. I believe there is still a possibility of a race condition. Perhaps we should invest more time in testing and debugging it.

3. The isLayoutReady prop in CKEditorContext works differently in the "classic" integration and multi-root integration. After providing isLayoutReady={false} in demo-multiroot-react-19, roots are rendered, and you can edit the content. However, when you pass the same prop in demo-react-19, the editor is not shown.

4. Generally, I understand it was a good opportunity to refactor some code, but adding new functionalities made the overall changes unclear. I'm not sure which changes were necessary in the context of supporting React 19 and which were made for refactoring purposes. I believe we could have done it in two steps. Additionally, I need to rethink the additional types, guards, and so on.

[Mati365]

@DawidKossowski

Unit tests produce a lot of errors in the console

It happens when CKEditor has roots that are not assigned to DOM while being destroyed. Happens frequently in renderHook test suite. I added fake elements before destroying editor to prevent this situation (commit). Overall I think it's not super crucial and maybe useful in testing only.

I got this error once in the Context demo (React 19):

I cannot reproduce this race condition altough I believe that you are right. There can be race condition between synchronization of watchdog state and React state. Basically context might be destroyed and react state update (about reinitialization of such react context) is still pending while editor is initializing. I pushed commit that fixes the issue I think.

The isLayoutReady prop in CKEditorContext works differently in the "classic" integration and multi-root integration. After providing isLayoutReady={false} in demo-multiroot-react-19, roots are rendered, and you can edit the content. However, when you pass the same prop in demo-react-19, the editor is not shown.

Is it correct behavior? It looks like our current implementation is broken (or it has another race condition). It makes absolutely no sense to initialize any editor within ContextProvider without waiting for initialization of context (because it will fall into the context adapter fallback in these lines).

Generally, I understand it was a good opportunity to refactor some code, but adding new functionalities made the overall changes unclear. I'm not sure which changes were necessary in the context of supporting React 19 and which were made for refactoring purposes. I believe we could have done it in two steps. Additionally, I need to rethink the additional types, guards, and so on.

Generally speaking it's complicated because it solves complex problem. React 19 tends to be more async than any other previous release and it only shown us hidden for many years issues with our implementation. The example might be async lifecycle methods in context class component which were reported already by community. These methods added other smaller race conditions. StrictMode another ones. null value in default context value too. This PR fixes all these smaller things to fix "big" Race Condition issue.

I believe we could have done it in two steps.

Agree however this PR is the first step. The next ones should be related to DX improvements and should utilize newer react hooks such as https://react.dev/reference/react/useSyncExternalStore which might help us with fixing with that kind of race condition issues (I hope so at least).

[Mati365]

Probably fixes this issue #485 (comment). Must be checked.

[Witoso]

Probably fixes this issue #485 (comment). Must be checked.

Shouldn't those fixes be made against master/alpha to release a new version?

[Mati365]

@Witoso At this moment, nope. Too many changes. Initially, I thought that the watchdog race conditions were only reproducible on React 19. However, it turns out that they can also occur occasionally on older versions. On React 19, these race conditions happen consistently.

[Witoso]

Hmm, will it mean that context in next.js won't work until we release this PR?

[Mati365]

@Witoso I suspect that it's reproducible on old installation methods, actually. Needs to be verified.

[f1ames]

@Witoso I suspect that it's reproducible on old installation methods, actually. Needs to be verified.

Yes, I checked with old installation methods and stable ckeditor5-react package and there is the same issue as reported in #485 for NIM setup.

[DawidKossowski]

Generally speaking it's complicated because it solves complex problem.

@Mati365, sure, the problem is complex, and it is understandable. However, you have exported a lot of new things as a public API. Who should use it and when? We need to provide user guides describing the correct use cases and so on.

Moreover, we still want to refactor the whole integration. In that case, the public API could change. I would rather reduce the breaking changes in the future by only making internal changes that fix the issue instead of extending the API. That is what I meant.

[Mati365]

@DawidKossowski I thought it may be useful here. I don't think that context format will be changed in future iterations so much. If you think we should not export it, I'll remove it.

[DawidKossowski]

That's what I meant by doing it in two steps. The first step will fix the integration with React 19 internally (the current PR), and the second step will enhance DX, the public API, and so on. I think the second step should be done as a separate task since it requires some research, analysis, and probably discussion. I'm not 100% sure about the current shape, and I don't want to block this PR, so I would remove it and address it in another task.

[Mati365]

@DawidKossowski gotcha, will be removed

[DawidKossowski]

I will merge it after releasing NIM.