Skip to content

Commit

Permalink
Merge pull request #156 from ckeditor/i/153-performance
Browse files Browse the repository at this point in the history 
Fix: The editor should now slow down with lots of content when using the integration. Closes #153.
  • Loading branch information
@pomek
pomek authored Sep 18, 2020
2 parents 20dee60 + ad5afb4 commit df4410a
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 41 deletions.
2 changes: 1 addition & 1 deletion dist/ckeditor.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/ckeditor.js.map
View file

Large diffs are not rendered by default.

33 changes: 17 additions & 16 deletions src/ckeditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ 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,
Expand All @@ -63,8 +63,8 @@ export default {

this.editor.create( this.$el, editorConfig )
.then( editor => {
// Save the reference to the instance for further use.
this.instance = editor;
// Save the reference to the $_instance for further use.
this.$_instance = editor;

// Set initial disabled state.
editor.isReadOnly = this.disabled;
Expand All @@ -80,19 +80,19 @@ export default {
},

beforeDestroy() {
if ( this.instance ) {
this.instance.destroy();
this.instance = null;
if ( this.$_instance ) {
this.$_instance.destroy();
this.$_instance = null;
}

// Note: By the time the editor is destroyed (promise resolved, editor#destroy fired)
// the Vue component will not be able to emit any longer. So emitting #destroy a bit earlier.
this.$emit( 'destroy', this.instance );
this.$emit( 'destroy', this.$_instance );
},

watch: {
value( newValue, oldValue ) {
// Synchronize changes of instance#value. There are two sources of changes:
// Synchronize changes of #value. There are two sources of changes:
//
// External value change ------\
// -----> +-----------+
Expand All @@ -102,32 +102,33 @@ export default {
// (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.
// 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.
// this.$_lastEditorData, which has been cached on #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
// 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 );
this.$_instance.setData( newValue );
}
},

// Synchronize changes of #disabled.
disabled( val ) {
this.instance.isReadOnly = val;
this.$_instance.isReadOnly = val;
}
},

methods: {
$_setUpEditorEvents() {
const editor = this.instance;
const editor = this.$_instance;

// Use the leading edge so the first event in the series is emitted immediately.
// Failing to do so leads to race conditions, for instance, when the component value
// is set twice in a time span shorter than the debounce time.
Expand All @@ -142,7 +143,7 @@ export default {
this.$emit( 'input', data, evt, editor );
}, INPUT_EVENT_DEBOUNCE_WAIT, { leading: true } );

// Debounce emitting the #input event. When data is huge, instance#getData()
// 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
Expand Down
40 changes: 20 additions & 20 deletions tests/ckeditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe( 'CKEditor Component', () => {

wrapper.destroy();
sinon.assert.calledOnce( stub );
expect( vm.instance ).to.be.null;
expect( vm.$_instance ).to.be.null;
} );

it( 'should pass the editor promise rejection error to console#error()', async () => {
Expand Down Expand Up @@ -81,7 +81,7 @@ describe( 'CKEditor Component', () => {
await Vue.nextTick();

expect( vm.editor ).to.equal( MockEditor );
expect( vm.instance ).to.be.instanceOf( MockEditor );
expect( vm.$_instance ).to.be.instanceOf( MockEditor );

wrapper.destroy();
} );
Expand All @@ -100,8 +100,8 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

expect( vm.instance.config.initialData ).to.equal( 'foo' );
expect( vm.instance.setDataCounter ).to.equal( 0 );
expect( vm.$_instance.config.initialData ).to.equal( 'foo' );
expect( vm.$_instance.setDataCounter ).to.equal( 0 );

wrapper.destroy();
} );
Expand Down Expand Up @@ -135,7 +135,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

expect( vm.instance.isReadOnly ).to.be.true;
expect( vm.$_instance.isReadOnly ).to.be.true;
wrapper.destroy();
} );
} );
Expand All @@ -152,7 +152,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

expect( vm.instance.config ).to.deep.equal( { foo: 'bar' } );
expect( vm.$_instance.config ).to.deep.equal( { foo: 'bar' } );
wrapper.destroy();
} );

Expand Down Expand Up @@ -206,7 +206,7 @@ describe( 'CKEditor Component', () => {
it( '#instance should be defined', async () => {
await Vue.nextTick();

expect( vm.instance ).to.be.instanceOf( MockEditor );
expect( vm.$_instance ).to.be.instanceOf( MockEditor );
} );
} );

Expand All @@ -218,21 +218,21 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

expect( vm.instance.isReadOnly ).to.be.true;
expect( vm.$_instance.isReadOnly ).to.be.true;

wrapper.setProps( { disabled: false } );

await Vue.nextTick();

expect( vm.instance.isReadOnly ).to.be.false;
expect( vm.$_instance.isReadOnly ).to.be.false;

wrapper.destroy();
} );

it( '#value should trigger editor#setData', async () => {
await Vue.nextTick();

const spy = sandbox.spy( vm.instance, 'setData' );
const spy = sandbox.spy( vm.$_instance, 'setData' );
wrapper.setProps( { value: 'foo' } );

await Vue.nextTick();
Expand Down Expand Up @@ -263,7 +263,7 @@ describe( 'CKEditor Component', () => {
await Vue.nextTick();

expect( wrapper.emitted().ready.length ).to.equal( 1 );
expect( wrapper.emitted().ready[ 0 ] ).to.deep.equal( [ vm.instance ] );
expect( wrapper.emitted().ready[ 0 ] ).to.deep.equal( [ vm.$_instance ] );
} );

it( 'should emit #destroy when the editor is destroyed', async () => {
Expand All @@ -274,7 +274,7 @@ describe( 'CKEditor Component', () => {
wrapper.destroy();

expect( wrapper.emitted().destroy.length ).to.equal( 1 );
expect( wrapper.emitted().destroy[ 0 ] ).to.deep.equal( [ vm.instance ] );
expect( wrapper.emitted().destroy[ 0 ] ).to.deep.equal( [ vm.$_instance ] );
} );

describe( '#input event', () => {
Expand All @@ -284,7 +284,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

const on = vm.instance.model.document.on;
const on = vm.$_instance.model.document.on;
const evtStub = {};

expect( on.calledOnce ).to.be.true;
Expand All @@ -299,7 +299,7 @@ describe( 'CKEditor Component', () => {

expect( wrapper.emitted().input.length ).to.equal( 1 );
expect( wrapper.emitted().input[ 0 ] ).to.deep.equal( [
'foo', evtStub, vm.instance
'foo', evtStub, vm.$_instance
] );
} );

Expand All @@ -310,7 +310,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

const on = vm.instance.model.document.on;
const on = vm.$_instance.model.document.on;
const evtStub = {};

expect( on.calledOnce ).to.be.true;
Expand All @@ -323,7 +323,7 @@ describe( 'CKEditor Component', () => {

expect( wrapper.emitted().input.length ).to.equal( 1 );
expect( wrapper.emitted().input[ 0 ] ).to.deep.equal( [
'foo', evtStub, vm.instance
'foo', evtStub, vm.$_instance
] );
} );
} );
Expand All @@ -333,7 +333,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

const on = vm.instance.editing.view.document.on;
const on = vm.$_instance.editing.view.document.on;
const evtStub = {};

expect( on.calledTwice ).to.be.true;
Expand All @@ -346,7 +346,7 @@ describe( 'CKEditor Component', () => {

expect( wrapper.emitted().focus.length ).to.equal( 1 );
expect( wrapper.emitted().focus[ 0 ] ).to.deep.equal( [
evtStub, vm.instance
evtStub, vm.$_instance
] );
} );

Expand All @@ -355,7 +355,7 @@ describe( 'CKEditor Component', () => {

await Vue.nextTick();

const on = vm.instance.editing.view.document.on;
const on = vm.$_instance.editing.view.document.on;
const evtStub = {};

expect( on.calledTwice ).to.be.true;
Expand All @@ -368,7 +368,7 @@ describe( 'CKEditor Component', () => {

expect( wrapper.emitted().blur.length ).to.equal( 1 );
expect( wrapper.emitted().blur[ 0 ] ).to.deep.equal( [
evtStub, vm.instance
evtStub, vm.$_instance
] );
} );
} );
Expand Down
2 changes: 1 addition & 1 deletion tests/plugin/integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe( 'CKEditor plugin', () => {
template: '<ckeditor :editor="editor" @ready="onReady()" v-model="editorData"></ckeditor>',
methods: {
onReady: () => {
const instance = wrapper.vm.$children[ 0 ].instance;
const instance = wrapper.vm.$children[ 0 ].$_instance;

expect( instance ).to.be.instanceOf( ClassicEditor );
expect( instance.getData() ).to.equal( '<p>foo</p>' );
Expand Down
4 changes: 2 additions & 2 deletions tests/plugin/localcomponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe( 'CKEditor plugin', () => {

await Vue.nextTick();

expect( wrapperFoo.vm.$children[ 0 ].instance ).to.be.instanceOf( FooEditor );
expect( wrapperBar.vm.$children[ 0 ].instance ).to.be.instanceOf( BarEditor );
expect( wrapperFoo.vm.$children[ 0 ].$_instance ).to.be.instanceOf( FooEditor );
expect( wrapperBar.vm.$children[ 0 ].$_instance ).to.be.instanceOf( BarEditor );
} );
} );

0 comments on commit df4410a

Please sign in to comment.