Merge pull request #555 from ckeditor/ck/552

Fix: Call `onChangeInitializedEditors` on startup of `CKEditorContext` if there are ready editors.

demos/cdn-multiroot-react/main.tsx

@@ -8,7 +8,7 @@ import App from './App.js';
 
 const element = document.getElementById( 'root' ) as HTMLDivElement;
 
-if ( __REACT_VERSION__ === 16 ) {
+if ( __REACT_VERSION__ <= 17 ) {
 	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
 	// @ts-ignore
 	const ReactDOM = await import( 'react-dom' );

demos/cdn-react/main.tsx

@@ -8,7 +8,7 @@ import { App } from './App.js';
 
 const element = document.getElementById( 'root' ) as HTMLDivElement;
 
-if ( __REACT_VERSION__ === 16 ) {
+if ( __REACT_VERSION__ <= 17 ) {
 	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
 	// @ts-ignore
 	const ReactDOM = await import( 'react-dom' );

demos/npm-multiroot-react/main.tsx

@@ -8,7 +8,7 @@ import App from './App.js';
 
 const element = document.getElementById( 'root' ) as HTMLDivElement;
 
-if ( __REACT_VERSION__ === 16 ) {
+if ( __REACT_VERSION__ <= 17 ) {
 	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
 	// @ts-ignore
 	const ReactDOM = await import( 'react-dom' );

demos/npm-react/ContextDemo.tsx

@@ -44,6 +44,7 @@ export default function ContextDemo( props: ContextDemoProps ): JSX.Element {
 				context={ ClassicEditor.Context as any }
 				contextWatchdog={ ClassicEditor.ContextWatchdog as any }
 				onChangeInitializedEditors={ editors => {
+					console.log( 'Initialized editors:', editors );
 					setState( editors as any );
 				} }
 			>

demos/npm-react/main.tsx

@@ -8,7 +8,7 @@ import App from './App.js';
 
 const element = document.getElementById( 'root' ) as HTMLDivElement;
 
-if ( __REACT_VERSION__ === 16 ) {
+if ( __REACT_VERSION__ <= 17 ) {
 	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
 	// @ts-ignore
 	const ReactDOM = await import( 'react-dom' );

package.json

@@ -63,6 +63,8 @@
     "react-dom": "^18.0.0",
     "react16": "npm:react@^16.0.0",
     "react16-dom": "npm:react-dom@^16.0.0",
+    "react17": "npm:react@^17.0.0",
+    "react17-dom": "npm:react-dom@^17.0.0",
     "react18": "npm:react@^18.0.0",
     "react18-dom": "npm:react-dom@^18.0.0",
     "react19": "npm:[email protected]",
@@ -83,8 +85,9 @@
     "node": ">=18.0.0"
   },
   "scripts": {
-    "dev": "echo \"Use 'dev:16', 'dev:18', or 'dev:19' depending on the version of React you want to test\"",
+    "dev": "echo \"Use 'dev:16', 'dev:17', 'dev:18', or 'dev:19' depending on the version of React you want to test\"",
     "dev:16": "REACT_VERSION=16 vite",
+    "dev:17": "REACT_VERSION=17 vite",
     "dev:18": "REACT_VERSION=18 vite",
     "dev:19": "REACT_VERSION=19 vite",
     "build": "vite build && tsc --emitDeclarationOnly",

src/context/useInitializedCKEditorsMap.ts

@@ -6,7 +6,7 @@
 import { useEffect } from 'react';
 import { useRefSafeCallback } from '../hooks/useRefSafeCallback.js';
 
-import type { CollectionAddEvent, Context, ContextWatchdog, Editor } from 'ckeditor5';
+import type { CollectionAddEvent, Context, ContextWatchdog, Editor, GetCallback } from 'ckeditor5';
 import type { ContextWatchdogValue } from './ckeditorcontext.js';
 
 import {
@@ -80,15 +80,25 @@ export const useInitializedCKEditorsMap = <TContext extends Context>(
 		};
 
 		// Add the existing editors to the map.
-		const onAddEditor = ( _: unknown, editor: Editor ) => {
+		const trackEditorLifecycle = ( editor: Editor ) => {
 			editor.once( 'ready', onEditorStatusChange, { priority: 'lowest' } );
 			editor.once( 'destroy', onEditorStatusChange, { priority: 'lowest' } );
 		};
 
-		editors.on<CollectionAddEvent<Editor>>( 'add', onAddEditor );
+		const onAddEditorToCollection: GetCallback<CollectionAddEvent<Editor>> = ( _, editor ) => {
+			trackEditorLifecycle( editor );
+		};
+
+		editors.forEach( trackEditorLifecycle );
+		editors.on<CollectionAddEvent<Editor>>( 'add', onAddEditorToCollection );
+
+		// Fire the initial change event if there is at least one editor ready, otherwise wait for the first ready editor.
+		if ( Array.from( editors ).some( editor => editor.state === 'ready' ) ) {
+			onEditorStatusChange();
+		}
 
 		return () => {
-			editors.off( 'add', onAddEditor );
+			editors.off( 'add', onAddEditorToCollection );
 		};
 	}, [ currentContextWatchdog ] );
 };

tests/_utils/editor.ts

@@ -18,6 +18,7 @@ export default class MockEditor {
 
 	// In order to tests events, we need to somehow mock those properties.
 	public static _on = (): void => {};
+	public static _once = (): void => {};
 
 	public static _model = {
 		document: createDocument(),
@@ -30,11 +31,13 @@ export default class MockEditor {
 		}
 	};
 
+	public declare state?: string;
 	public declare element?: HTMLElement;
 	public declare config?: Record<string, any>;
 	public declare model: any;
 	public declare editing: any;
 	public declare on: any;
+	public declare once: any;
 	public declare data: any;
 	public declare createEditable: any;
 	public declare ui: any;
@@ -52,6 +55,7 @@ export default class MockEditor {
 		this.model = MockEditor._model;
 		this.editing = MockEditor._editing;
 		this.on = MockEditor._on;
+		this.once = MockEditor._once;
 		this.data = {
 			get() {
 				return '';

tests/context/useInitializedCKEditorsMap.test.tsx

@@ -0,0 +1,157 @@
+/**
+ * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { renderHook } from '@testing-library/react';
+import { Collection } from 'ckeditor5';
+
+import { useInitializedCKEditorsMap } from '../../src/context/useInitializedCKEditorsMap.js';
+
+import type { ContextWatchdogValue } from '../../src/context/ckeditorcontext.js';
+import type { CKEditorConfigContextMetadata } from '../../src/context/setCKEditorReactContextMetadata.js';
+
+import MockEditor from '../_utils/editor.js';
+
+describe( 'useInitializedCKEditorsMap', () => {
+	afterEach( () => {
+		vi.clearAllMocks();
+	} );
+
+	it( 'should not call onChangeInitializedEditors when context is not initialized', () => {
+		const onChangeInitializedEditors = vi.fn();
+		const mockWatchdog = {
+			status: 'initializing' as const,
+			watchdog: null
+		};
+
+		renderHook( () => useInitializedCKEditorsMap( {
+			currentContextWatchdog: mockWatchdog,
+			onChangeInitializedEditors
+		} ) );
+
+		expect( onChangeInitializedEditors ).not.toHaveBeenCalled();
+	} );
+
+	it( 'should not track editors that are not ready', () => {
+		const editors = new Collection();
+		const onChangeInitializedEditors = vi.fn();
+		const notReadyEditor = createMockEditor( 'initializing', { name: '1' } );
+		editors.add( notReadyEditor );
+
+		renderHook( () => useInitializedCKEditorsMap( {
+			currentContextWatchdog: createMockContextWatchdog( editors ),
+			onChangeInitializedEditors
+		} ) );
+
+		expect( onChangeInitializedEditors ).not.toHaveBeenCalled();
+	} );
+
+	it( 'should track ready editors', () => {
+		const editors = new Collection();
+		const onChangeInitializedEditors = vi.fn();
+		const readyEditor = createMockEditor( 'ready', { name: '1' } );
+		editors.add( readyEditor );
+
+		renderHook( () => useInitializedCKEditorsMap( {
+			currentContextWatchdog: createMockContextWatchdog( editors ),
+			onChangeInitializedEditors
+		} ) );
+
+		expect( onChangeInitializedEditors ).toHaveBeenCalledWith(
+			expect.objectContaining( {
+				'1': {
+					instance: readyEditor,
+					metadata: {
+						name: '1'
+					}
+				}
+			} ),
+			expect.anything()
+		);
+	} );
+
+	it( 'should handle adding new editors to collection', () => {
+		const editors = new Collection();
+		const onChangeInitializedEditors = vi.fn();
+
+		renderHook( () => useInitializedCKEditorsMap( {
+			currentContextWatchdog: createMockContextWatchdog( editors ),
+			onChangeInitializedEditors
+		} ) );
+
+		const newEditor = createMockEditor( 'ready', { name: '2' } );
+		editors.add( newEditor );
+
+		// Simulate 'ready' event
+		const readyCallback = newEditor.once.mock.calls.find( ( call: any ) => call[ 0 ] === 'ready' )![ 1 ];
+		readyCallback();
+
+		expect( onChangeInitializedEditors ).toHaveBeenLastCalledWith(
+			expect.objectContaining( {
+				'2': {
+					instance: newEditor,
+					metadata: {
+						name: '2'
+					}
+				}
+			} ),
+			expect.anything()
+		);
+	} );
+
+	it( 'should handle editor destruction', () => {
+		const editors = new Collection();
+		const onChangeInitializedEditors = vi.fn();
+		const editor = createMockEditor( 'ready', { name: '1' } );
+		editors.add( editor );
+
+		renderHook( () => useInitializedCKEditorsMap( {
+			currentContextWatchdog: createMockContextWatchdog( editors ),
+			onChangeInitializedEditors
+		} ) );
+
+		// Simulate 'destroy' event
+		const destroyCallback = editor.once.mock.calls.find( ( call: any ) => call[ 0 ] === 'destroy' )![ 1 ];
+		editors.remove( editor );
+		destroyCallback();
+
+		expect( onChangeInitializedEditors ).toHaveBeenLastCalledWith(
+			expect.objectContaining( {} ),
+			expect.anything()
+		);
+	} );
+} );
+
+function createMockEditor( state = 'ready', contextMetadata: CKEditorConfigContextMetadata, config = {} ) {
+	const editor = new MockEditor( document.createElement( 'div' ), {
+		get( name: string ) {
+			if ( name === '$__CKEditorReactContextMetadata' ) {
+				return contextMetadata;
+			}
+
+			if ( Object.prototype.hasOwnProperty.call( config, name ) ) {
+				return config[ name ];
+			}
+
+			return undefined;
+		}
+	} );
+
+	editor.state = state;
+	editor.once = vi.fn();
+
+	return editor;
+}
+
+function createMockContextWatchdog( editors = new Collection() ) {
+	return ( {
+		status: 'initialized' as const,
+		watchdog: {
+			context: {
+				editors
+			}
+		}
+	} ) as unknown as ContextWatchdogValue<any>;
+}

vite.config.ts

@@ -93,7 +93,7 @@ export default defineConfig( {
 	resolve: {
 		alias: {
 			'react': resolve( __dirname, `node_modules/react${ REACT_VERSION }` ),
-			'react-dom/client': resolve( __dirname, `node_modules/react${ REACT_VERSION }-dom${ REACT_VERSION === 16 ? '' : '/client' }` ),
+			'react-dom/client': resolve( __dirname, `node_modules/react${ REACT_VERSION }-dom${ REACT_VERSION <= 17 ? '' : '/client' }` ),
 			'react-dom': resolve( __dirname, `node_modules/react${ REACT_VERSION }-dom` )
 		}
 	},