Merge pull request #46 from ckeditor/t/42

Fix: Improved performance when editing large content. Debounced the component `#input` event. Closes #42.

Author: pomek

Diff for: dist/ckeditor.js

@@ -42,6 +42,7 @@
     "karma-sinon": "^1.0.5",
     "karma-sourcemap-loader": "^0.3.7",
     "karma-webpack": "^3.0.0",
+    "lodash-es": "^4.17.11",
     "minimist": "^1.2.0",
     "mocha": "^5.2.0",
     "sinon": "^7.2.3",

Diff for: dist/ckeditor.js.map

@@ -5,6 +5,10 @@
 
 /* global console */
 
+import { debounce } from 'lodash-es';
+
+const INPUT_EVENT_DEBOUNCE_WAIT = 300;
+
 export default {
 	name: 'ckeditor',
 
@@ -39,7 +43,12 @@ export default {
 		return {
 			// Don't define it in #props because it produces a warning.
 			// https://vuejs.org/v2/guide/components-props.html#One-Way-Data-Flow
-			instance: null
+			instance: null,
+
+			$_lastEditorData: {
+				type: String,
+				default: ''
+			}
 		};
 	},
 
@@ -77,14 +86,31 @@ export default {
 	},
 
 	watch: {
-		// Synchronize changes of #value.
-		value( val ) {
-			// If the change is the result of typing, the #value is the same as instance.getData().
-			// In that case, the change has been triggered by instance.model.document#change:data
-			// so #value and instance.getData() are already in sync. Executing instance#setData()
-			// would demolish the selection.
-			if ( this.instance.getData() !== val ) {
-				this.instance.setData( val );
+		value( newValue, oldValue ) {
+			// Synchronize changes of instance#value. There are two sources of changes:
+			//
+			//                     External value change      ------\
+			//                                                       -----> +-----------+
+			//                                                              | Component |
+			//                                                       -----> +-----------+
+			//                     Internal data change       ------/
+			//              (typing, commands, collaboration)
+			//
+			// Case 1: If the change was external (via props), the editor data must be synced with
+			// the component using instance#setData() and it is OK to destroy the selection.
+			//
+			// Case 2: If the change is the result of internal data change, the #value is the same as
+			// instance#$_lastEditorData, which has been cached on instance#change:data. If we called
+			// instance#setData() at this point, that would demolish the selection.
+			//
+			// To limit the number of instance#setData() which is time-consuming when there is a
+			// lot of data we make sure:
+			//    * the new value is at least different than the old value (Case 1.)
+			//    * the new value is different than the last internal instance state (Case 2.)
+			//
+			// See: https://github.com/ckeditor/ckeditor5-vue/issues/42.
+			if ( newValue !== oldValue && newValue !== this.$_lastEditorData ) {
+				this.instance.setData( newValue );
 			}
 		},
 
@@ -97,13 +123,21 @@ export default {
 	methods: {
 		$_setUpEditorEvents() {
 			const editor = this.instance;
-
-			editor.model.document.on( 'change:data', evt => {
-				const data = editor.getData();
+			const emitInputEvent = evt => {
+				// Cache the last editor data. This kind of data is a result of typing,
+				// editor command execution, collaborative changes to the document, etc.
+				// This data is compared when the component value changes in a 2-way binding.
+				const data = this.$_lastEditorData = editor.getData();
 
 				// The compatibility with the v-model and general Vue.js concept of input–like components.
 				this.$emit( 'input', data, evt, editor );
-			} );
+			};
+
+			// Debounce emitting the #input event. When data is huge, instance#getData()
+			// takes a lot of time to execute on every single key press and ruins the UX.
+			//
+			// See: https://github.com/ckeditor/ckeditor5-vue/issues/42
+			editor.model.document.on( 'change:data', debounce( emitInputEvent, INPUT_EVENT_DEBOUNCE_WAIT ) );
 
 			editor.editing.view.document.on( 'focus', evt => {
 				this.$emit( 'focus', evt, editor );

Diff for: package.json

@@ -30,6 +30,8 @@ describe( 'CKEditor Component', () => {
 	} );
 
 	it( 'calls editor#create when initializing', done => {
+		Vue.config.errorHandler = done;
+
 		const stub = sandbox.stub( MockEditor, 'create' ).resolves( new MockEditor() );
 		const { wrapper } = createComponent();
 
@@ -42,6 +44,8 @@ describe( 'CKEditor Component', () => {
 	} );
 
 	it( 'calls editor#destroy when destroying', done => {
+		Vue.config.errorHandler = done;
+
 		const stub = sandbox.stub( MockEditor.prototype, 'destroy' ).resolves();
 		const { wrapper, vm } = createComponent();
 
@@ -55,6 +59,8 @@ describe( 'CKEditor Component', () => {
 	} );
 
 	it( 'passes editor promise rejection error to console.error', done => {
+		Vue.config.errorHandler = done;
+
 		const error = new Error( 'Something went wrong.' );
 		const consoleErrorStub = sandbox.stub( console, 'error' );
 
@@ -75,6 +81,8 @@ describe( 'CKEditor Component', () => {
 	describe( 'properties', () => {
 		it( '#editor', () => {
 			it( 'accepts a string', done => {
+				Vue.config.errorHandler = done;
+
 				expect( vm.editor ).to.equal( 'classic' );
 
 				Vue.nextTick( () => {
@@ -85,6 +93,8 @@ describe( 'CKEditor Component', () => {
 			} );
 
 			it( 'accepts an editor constructor', done => {
+				Vue.config.errorHandler = done;
+
 				const { wrapper, vm } = createComponent( {
 					editor: MockEditor
 				} );
@@ -105,6 +115,8 @@ describe( 'CKEditor Component', () => {
 			} );
 
 			it( 'should set the initial data', done => {
+				Vue.config.errorHandler = done;
+
 				const setDataStub = sandbox.stub( MockEditor.prototype, 'setData' );
 				const { wrapper } = createComponent( {
 					value: 'foo'
@@ -141,6 +153,8 @@ describe( 'CKEditor Component', () => {
 			} );
 
 			it( 'should set the initial editor#isReadOnly', done => {
+				Vue.config.errorHandler = done;
+
 				const { wrapper, vm } = createComponent( {
 					disabled: true
 				} );
@@ -159,6 +173,8 @@ describe( 'CKEditor Component', () => {
 			} );
 
 			it( 'should set the initial editor#config', done => {
+				Vue.config.errorHandler = done;
+
 				const { wrapper, vm } = createComponent( {
 					config: { foo: 'bar' }
 				} );
@@ -172,6 +188,8 @@ describe( 'CKEditor Component', () => {
 		} );
 
 		it( '#instance should be defined', done => {
+			Vue.config.errorHandler = done;
+
 			Vue.nextTick( () => {
 				expect( vm.instance ).to.be.instanceOf( MockEditor );
 
@@ -182,6 +200,8 @@ describe( 'CKEditor Component', () => {
 
 	describe( 'bindings', () => {
 		it( '#disabled should control editor#isReadOnly', done => {
+			Vue.config.errorHandler = done;
+
 			const { wrapper, vm } = createComponent( {
 				disabled: true
 			} );
@@ -198,18 +218,22 @@ describe( 'CKEditor Component', () => {
 		} );
 
 		it( '#value should trigger editor#setData', done => {
+			Vue.config.errorHandler = done;
+
 			Vue.nextTick( () => {
 				const spy = sandbox.spy( vm.instance, 'setData' );
 
 				wrapper.setProps( { value: 'foo' } );
 				wrapper.setProps( { value: 'bar' } );
 				wrapper.setProps( { value: 'bar' } );
 
+				sinon.assert.calledTwice( spy );
+
 				// Simulate typing: The #value changes but at the same time, the instance update
 				// its own data so instance.getData() and #value are immediately the same.
 				// Make sure instance.setData() is not called in this situation because it would destroy
 				// the selection.
-				sandbox.stub( vm.instance, 'getData' ).returns( 'barq' );
+				wrapper.vm.$_lastEditorData = 'barq';
 				wrapper.setProps( { value: 'barq' } );
 
 				sinon.assert.calledTwice( spy );
@@ -223,6 +247,8 @@ describe( 'CKEditor Component', () => {
 
 	describe( 'events', () => {
 		it( 'emits #ready when editor is created', done => {
+			Vue.config.errorHandler = done;
+
 			Vue.nextTick( () => {
 				expect( wrapper.emitted().ready.length ).to.equal( 1 );
 				expect( wrapper.emitted().ready[ 0 ] ).to.deep.equal( [ vm.instance ] );
@@ -232,6 +258,8 @@ describe( 'CKEditor Component', () => {
 		} );
 
 		it( 'emits #destroy when editor is destroyed', done => {
+			Vue.config.errorHandler = done;
+
 			const { wrapper, vm } = createComponent();
 
 			Vue.nextTick( () => {
@@ -244,7 +272,9 @@ describe( 'CKEditor Component', () => {
 			} );
 		} );
 
-		it( 'emits #input when editor data changes', done => {
+		it( 'emits debounced #input when editor data changes', done => {
+			Vue.config.errorHandler = done;
+
 			sandbox.stub( ModelDocument.prototype, 'on' );
 			sandbox.stub( MockEditor.prototype, 'getData' ).returns( 'foo' );
 
@@ -260,16 +290,20 @@ describe( 'CKEditor Component', () => {
 
 				on.firstCall.args[ 1 ]( evtStub );
 
-				expect( wrapper.emitted().input.length ).to.equal( 1 );
-				expect( wrapper.emitted().input[ 0 ] ).to.deep.equal( [
-					'foo', evtStub, vm.instance
-				] );
+				setTimeout( () => {
+					expect( wrapper.emitted().input.length ).to.equal( 1 );
+					expect( wrapper.emitted().input[ 0 ] ).to.deep.equal( [
+						'foo', evtStub, vm.instance
+					] );
 
-				done();
+					done();
+				}, 350 );
 			} );
 		} );
 
 		it( 'emits #focus when editor editable is focused', done => {
+			Vue.config.errorHandler = done;
+
 			sandbox.stub( ViewlDocument.prototype, 'on' );
 
 			Vue.nextTick( () => {
@@ -294,6 +328,8 @@ describe( 'CKEditor Component', () => {
 		} );
 
 		it( 'emits #blur when editor editable is focused', done => {
+			Vue.config.errorHandler = done;
+
 			sandbox.stub( ViewlDocument.prototype, 'on' );
 
 			Vue.nextTick( () => {