Bladeren bron

Merge pull request #34 from ckeditor/i/4699

Make watchdog not dependent on other CKEditor 5 packages
Szymon Cofalik 5 jaren geleden
bovenliggende
commit
7de08e1cf1

+ 13 - 9
packages/ckeditor5-watchdog/docs/features/watchdog.md

@@ -119,17 +119,22 @@ watchdog.editor;
 // * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` depending on how many and how frequency errors have been caught recently,
 // * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed,
 // * `destroyed` - a state when the editor is manually destroyed by the user after calling `watchdog.destroy()`.
-//
-// This property is observable.
 watchdog.state;
 
 // Listen to state changes.
-watchdog.on( 'change:state' ( evt, name, currentState, prevState ) => {
+
+let prevState = watchdog.state;
+
+watchdog.on( 'stateChange', () => {
+	const currentState = watchdog.state;
+
 	console.log( `State changed from ${ currentState } to ${ prevState }` );
 
 	if ( currentState === 'crashedPermanently' ) {
 		watchdog.editor.isReadOnly = true;
 	}
+
+	prevState = currentState;
 } );
 
 // An array of editor crashes info.
@@ -296,9 +301,7 @@ const contextState = watchdog.state;
 // The `error` event is fired when the context watchdog catches a context-related error.
 // Note that errors fired by items are not delegated to `ContextWatchdog#event:error`.
 // See also `ContextWatchdog#event:itemError`.
-watchdog.on( 'error', ( evt, { error } ) => {
-	console.log( 'The context crashed.' );
-} );
+watchdog.on( 'error', ( _, { error } ) => {
 
 // The `restart` event is fired when the context is set back to the `ready` state (after it was in `crashed` state).
 // Similarly, this event is not thrown for internal item restarts.
@@ -306,14 +309,15 @@ watchdog.on( 'restart', () => {
 	console.log( 'The context has been restarted.' );
 } );
 
+
 // The `itemError` event is fired when an error occurred in one of the added items.
-watchdog.on( 'itemError', ( evt, { error, itemId } ) => {
+watchdog.on( 'itemError', ( _, { error, itemId } ) => {
 	console.log( `An error occurred in an item with the '${ itemId }' id.` );
 } );
 
 // The `itemRestart` event is fired when an item is set back to the `ready` state (after it was in `crashed` state).
-watchdog.on( 'itemRestart', ( evt, { itemId } ) => {
-	console.log( 'An item with '${ itemId }' id has been restarted.' );
+watchdog.on( 'itemRestart', ( _, { itemId } ) => {
+	console.log( 'An item with with the '${ itemId }' id has been restarted.' );
 } );
 ```
 

+ 1 - 1
packages/ckeditor5-watchdog/package.json

@@ -9,7 +9,6 @@
     "ckeditor5-lib"
   ],
   "dependencies": {
-    "@ckeditor/ckeditor5-utils": "^16.0.0",
     "lodash-es": "^4.17.10"
   },
   "devDependencies": {
@@ -17,6 +16,7 @@
     "@ckeditor/ckeditor5-editor-classic": "^16.0.0",
     "@ckeditor/ckeditor5-engine": "^16.0.0",
     "@ckeditor/ckeditor5-paragraph": "^16.0.0",
+    "@ckeditor/ckeditor5-utils": "^16.0.0",
     "eslint": "^5.5.0",
     "eslint-config-ckeditor5": "^2.0.0",
     "husky": "^2.4.1",

+ 12 - 5
packages/ckeditor5-watchdog/src/contextwatchdog.js

@@ -102,6 +102,7 @@ export default class ContextWatchdog extends Watchdog {
 		this._actionQueue.onEmpty( () => {
 			if ( this.state === 'initializing' ) {
 				this.state = 'ready';
+				this._fire( 'stateChange' );
 			}
 		} );
 
@@ -268,7 +269,7 @@ export default class ContextWatchdog extends Watchdog {
 					// Enqueue the internal watchdog errors within the main queue.
 					// And propagate the internal `error` events as `itemError` event.
 					watchdog.on( 'error', ( evt, { error, causesRestart } ) => {
-						this.fire( 'itemError', { itemId: item.id, error } );
+						this._fire( 'itemError', { itemId: item.id, error } );
 
 						// Do not enqueue the item restart action if the item will not restart.
 						if ( !causesRestart ) {
@@ -276,11 +277,15 @@ export default class ContextWatchdog extends Watchdog {
 						}
 
 						this._actionQueue.enqueue( () => new Promise( res => {
-							watchdog.once( 'restart', () => {
-								this.fire( 'itemRestart', { itemId: item.id } );
+							watchdog.on( 'restart', rethrowRestartEventOnce.bind( this ) );
+
+							function rethrowRestartEventOnce() {
+								watchdog.off( 'restart', rethrowRestartEventOnce );
+
+								this._fire( 'itemRestart', { itemId: item.id } );
 
 								res();
-							} );
+							}
 						} ) );
 					} );
 
@@ -331,6 +336,7 @@ export default class ContextWatchdog extends Watchdog {
 	destroy() {
 		return this._actionQueue.enqueue( () => {
 			this.state = 'destroyed';
+			this._fire( 'stateChange' );
 
 			super.destroy();
 
@@ -347,13 +353,14 @@ export default class ContextWatchdog extends Watchdog {
 	_restart() {
 		return this._actionQueue.enqueue( () => {
 			this.state = 'initializing';
+			this._fire( 'stateChange' );
 
 			return this._destroy()
 				.catch( err => {
 					console.error( 'An error happened during destroying the context or items.', err );
 				} )
 				.then( () => this._create() )
-				.then( () => this.fire( 'restart' ) );
+				.then( () => this._fire( 'restart' ) );
 		} );
 	}
 

+ 5 - 2
packages/ckeditor5-watchdog/src/editorwatchdog.js

@@ -140,6 +140,7 @@ export default class EditorWatchdog extends Watchdog {
 		return Promise.resolve()
 			.then( () => {
 				this.state = 'initializing';
+				this._fire( 'stateChange' );
 
 				return this._destroy();
 			} )
@@ -158,7 +159,7 @@ export default class EditorWatchdog extends Watchdog {
 				}
 			} )
 			.then( () => {
-				this.fire( 'restart' );
+				this._fire( 'restart' );
 			} );
 	}
 
@@ -189,12 +190,13 @@ export default class EditorWatchdog extends Watchdog {
 			.then( editor => {
 				this._editor = editor;
 
-				this.listenTo( editor.model.document, 'change:data', this._throttledSave );
+				editor.model.document.on( 'change:data', this._throttledSave );
 
 				this._lastDocumentVersion = editor.model.document.version;
 				this._data = this._getData();
 
 				this.state = 'ready';
+				this._fire( 'stateChange' );
 			} );
 	}
 
@@ -209,6 +211,7 @@ export default class EditorWatchdog extends Watchdog {
 		return Promise.resolve()
 			.then( () => {
 				this.state = 'destroyed';
+				this._fire( 'stateChange' );
 
 				super.destroy();
 

+ 79 - 27
packages/ckeditor5-watchdog/src/watchdog.js

@@ -9,9 +9,6 @@
 
 /* globals window */
 
-import mix from '@ckeditor/ckeditor5-utils/src/mix';
-import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
-
 /**
  * An abstract watchdog class that handles most of the error handling process and the state of the underlying component.
  *
@@ -42,7 +39,7 @@ export default class Watchdog {
 		this.crashes = [];
 
 		/**
-		 * Specifies the state of the watchdog item handled by the watchdog. The state can be one of the following values:
+		 * Specifies the state of the item watched by the watchdog. The state can be one of the following values:
 		 *
 		 * * `initializing` - before the first initialization, and after crashes, before the item is ready,
 		 * * `ready` - a state when a user can interact with the item,
@@ -52,10 +49,9 @@ export default class Watchdog {
 		 * * `destroyed` - a state when the item is manually destroyed by the user after calling `watchdog.destroy()`
 		 *
 		 * @public
-		 * @observable
 		 * @member {'initializing'|'ready'|'crashed'|'crashedPermanently'|'destroyed'} #state
 		 */
-		this.set( 'state', 'initializing' );
+		this.state = 'initializing';
 
 		/**
 		 * @protected
@@ -96,13 +92,6 @@ export default class Watchdog {
 			}
 		};
 
-		if ( !this._restart ) {
-			throw new Error(
-				'The Watchdog class was split into the abstract `Watchdog` class and the `EditorWatchdog` class. ' +
-				'Please, use `EditorWatchdog` if you have used the `Watchdog` class previously.'
-			);
-		}
-
 		/**
 		 * The creation method.
 		 *
@@ -120,7 +109,7 @@ export default class Watchdog {
 		 */
 
 		/**
-		 * The handled watchdog item.
+		 * The watched item.
 		 *
 		 * @abstract
 		 * @protected
@@ -128,7 +117,7 @@ export default class Watchdog {
 		 */
 
 		/**
-		 * The method responsible for the watchdog item restarting.
+		 * The method responsible for restarting the watched item.
 		 *
 		 * @abstract
 		 * @protected
@@ -136,7 +125,7 @@ export default class Watchdog {
 		 */
 
 		/**
-		 * Traverses the error context and the handled watchdog item to find out whether the error should
+		 * Traverses the error context and the watched item to find out whether the error should
 		 * be handled by the given item.
 		 *
 		 * @abstract
@@ -144,10 +133,25 @@ export default class Watchdog {
 		 * @method #_isErrorComingFromThisItem
 		 * @param {module:utils/ckeditorerror~CKEditorError} error
 		 */
+
+		/**
+		 * A dictionary of event emitter listeners.
+		 *
+		 * @private
+		 * @type {Object.<String,Array.<Function>>}
+		 */
+		this._listeners = {};
+
+		if ( !this._restart ) {
+			throw new Error(
+				'The Watchdog class was split into the abstract `Watchdog` class and the `EditorWatchdog` class. ' +
+				'Please, use `EditorWatchdog` if you have used the `Watchdog` class previously.'
+			);
+		}
 	}
 
 	/**
-	 * Sets the function that is responsible for the watchdog item creation.
+	 * Sets the function that is responsible for creating watchded items.
 	 *
 	 * @param {Function} creator A callback responsible for creating an item. Returns a promise
 	 * that is resolved when the item is created.
@@ -157,7 +161,7 @@ export default class Watchdog {
 	}
 
 	/**
-	 * Sets the function that is responsible for the watchdog item destruction.
+	 * Sets the function that is responsible for destructing watched items.
 	 *
 	 * @param {Function} destructor A callback that takes the item and returns the promise
 	 * to the destroying process.
@@ -167,11 +171,59 @@ export default class Watchdog {
 	}
 
 	/**
-	 * Destroys the watchdog and release the resources.
+	 * Destroys the watchdog and releases the resources.
 	 */
 	destroy() {
 		this._stopErrorHandling();
-		this.stopListening();
+
+		this._listeners = {};
+	}
+
+	/**
+	 * Starts listening to the specific event name by registering a callback that will be executed
+	 * whenever an event with given name fires.
+	 *
+	 * Note that this method differs from the CKEditor 5's default `EventEmitterMixin` implementation.
+	 *
+	 * @param {String} eventName  Event name.
+	 * @param {Function} callback A callback which will be added to event listeners.
+	 */
+	on( eventName, callback ) {
+		if ( !this._listeners[ eventName ] ) {
+			this._listeners[ eventName ] = [];
+		}
+
+		this._listeners[ eventName ].push( callback );
+	}
+
+	/**
+	 * Stops listening to the specified event name by removing the callback from event listeners.
+	 *
+	 * Note that this method differs from the CKEditor 5's default `EventEmitterMixin` implementation.
+	 *
+	 * @param {String} eventName Event name.
+	 * @param {Function} callback A callback which will be removed from event listeners.
+	 */
+	off( eventName, callback ) {
+		this._listeners[ eventName ] = this._listeners[ eventName ]
+			.filter( cb => cb !== callback );
+	}
+
+	/**
+	 * Fires an event with given event name and arguments.
+	 *
+	 * Note that this method differs from the CKEditor 5's default `EventEmitterMixin` implementation.
+	 *
+	 * @protected
+	 * @param {String} eventName Event name.
+	 * @param  {...any} args Event arguments.
+	 */
+	_fire( eventName, ...args ) {
+		const callbacks = this._listeners[ eventName ] || [];
+
+		for ( const callback of callbacks ) {
+			callback.apply( this, [ null, ...args ] );
+		}
 	}
 
 	/**
@@ -195,7 +247,7 @@ export default class Watchdog {
 	}
 
 	/**
-	 * Checks if the error comes from the watchdog item and restarts it.
+	 * Checks if the error comes from the watched item and restarts it.
 	 * It reacts to {@link module:utils/ckeditorerror~CKEditorError `CKEditorError` errors} only.
 	 *
 	 * @private
@@ -223,12 +275,14 @@ export default class Watchdog {
 			const causesRestart = this._shouldRestart();
 
 			this.state = 'crashed';
-			this.fire( 'error', { error, causesRestart } );
+			this._fire( 'stateChange' );
+			this._fire( 'error', { error, causesRestart } );
 
 			if ( causesRestart ) {
 				this._restart();
 			} else {
 				this.state = 'crashedPermanently';
+				this._fire( 'stateChange' );
 			}
 		}
 	}
@@ -245,7 +299,7 @@ export default class Watchdog {
 			error.is( 'CKEditorError' ) &&
 			error.context !== undefined &&
 
-			// In some cases the watchdog item should not be restarted - e.g. during the item initialization.
+			// In some cases the watched item should not be restarted - e.g. during the item initialization.
 			// That's why the `null` was introduced as a correct error context which does cause restarting.
 			error.context !== null &&
 
@@ -284,19 +338,17 @@ export default class Watchdog {
 	 */
 }
 
-mix( Watchdog, ObservableMixin );
-
 /**
  * The watchdog plugin configuration.
  *
  * @typedef {Object} WatchdogConfig
  *
- * @property {Number} [crashNumberLimit=3] A threshold specifying the number of watchdog item crashes
+ * @property {Number} [crashNumberLimit=3] A threshold specifying the number of watched item crashes
  * when the watchdog stops restarting the item in case of errors.
  * After this limit is reached and the time between last errors is shorter than `minimumNonErrorTimePeriod`
  * the watchdog changes its state to `crashedPermanently` and it stops restarting the item. This prevents an infinite restart loop.
  *
- * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last watchdog item errors
+ * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last watched item errors
  * (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached
  * the watchdog changes its state to `crashedPermanently` and it stops restarting the item. This prevents an infinite restart loop.
  *

+ 27 - 47
packages/ckeditor5-watchdog/tests/editorwatchdog.js

@@ -862,15 +862,6 @@ describe( 'EditorWatchdog', () => {
 	} );
 
 	describe( 'state', () => {
-		let orphanEditors = [];
-
-		afterEach( () => {
-			return Promise.all( orphanEditors.map( editor => editor.destroy() ) )
-				.then( () => {
-					orphanEditors = [];
-				} );
-		} );
-
 		it( 'should reflect the state of the watchdog', async () => {
 			const watchdog = new EditorWatchdog( ClassicTestEditor );
 
@@ -882,11 +873,8 @@ describe( 'EditorWatchdog', () => {
 
 			await watchdog.create( element );
 
-			orphanEditors.push( watchdog.editor );
 			expect( watchdog.state ).to.equal( 'ready' );
 
-			await watchdog.create( element );
-
 			setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ) );
 			setTimeout( () => throwCKEditorError( 'bar', watchdog.editor ) );
 
@@ -905,49 +893,41 @@ describe( 'EditorWatchdog', () => {
 			const watchdog = new EditorWatchdog( ClassicTestEditor );
 			const states = [];
 
-			watchdog.on( 'change:state', ( evt, propName, newValue ) => {
-				states.push( newValue );
+			watchdog.on( 'stateChange', () => {
+				states.push( watchdog.state );
 			} );
 
 			const originalErrorHandler = window.onerror;
 			window.onerror = undefined;
 
-			return watchdog.create( element ).then( () => {
-				orphanEditors.push( watchdog.editor );
+			await watchdog.create( element );
 
-				return watchdog.create( element ).then( () => {
-					setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ) );
-					setTimeout( () => throwCKEditorError( 'bar', watchdog.editor ) );
-					setTimeout( () => throwCKEditorError( 'baz', watchdog.editor ) );
-					setTimeout( () => throwCKEditorError( 'biz', watchdog.editor ) );
+			setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ) );
+			setTimeout( () => throwCKEditorError( 'bar', watchdog.editor ) );
+			setTimeout( () => throwCKEditorError( 'baz', watchdog.editor ) );
+			setTimeout( () => throwCKEditorError( 'biz', watchdog.editor ) );
 
-					return new Promise( res => {
-						setTimeout( () => {
-							window.onerror = originalErrorHandler;
+			await waitCycle();
 
-							watchdog.destroy().then( () => {
-								expect( states ).to.deep.equal( [
-									'ready',
-									'crashed',
-									'initializing',
-									'ready',
-									'crashed',
-									'initializing',
-									'ready',
-									'crashed',
-									'initializing',
-									'ready',
-									'crashed',
-									'crashedPermanently',
-									'destroyed'
-								] );
-
-								res();
-							} );
-						} );
-					} );
-				} );
-			} );
+			window.onerror = originalErrorHandler;
+
+			await watchdog.destroy();
+
+			expect( states ).to.deep.equal( [
+				'ready',
+				'crashed',
+				'initializing',
+				'ready',
+				'crashed',
+				'initializing',
+				'ready',
+				'crashed',
+				'initializing',
+				'ready',
+				'crashed',
+				'crashedPermanently',
+				'destroyed'
+			] );
 		} );
 	} );