From 038033de1a35878c41f6350865c8641b27291bf6 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 22 Mar 2019 09:14:59 +0100 Subject: [PATCH 1/3] The component will not be updated by React itself. --- src/ckeditor.jsx | 15 ++++------ tests/ckeditor.jsx | 68 ++++++++++------------------------------------ 2 files changed, 20 insertions(+), 63 deletions(-) diff --git a/src/ckeditor.jsx b/src/ckeditor.jsx index 8008ee0..bfe3c94 100644 --- a/src/ckeditor.jsx +++ b/src/ckeditor.jsx @@ -17,18 +17,13 @@ export default class CKEditor extends React.Component { this.domContainer = React.createRef(); } - componentDidUpdate() { - if ( !this.editor ) { - return; + // This component should never be updated by React itself. + shouldComponentUpdate( nextProps ) { + if ( 'disabled' in nextProps ) { + this.editor.isReadOnly = nextProps.disabled; } - if ( 'data' in this.props && this.props.data !== this.editor.getData() ) { - this.editor.setData( this.props.data ); - } - - if ( 'disabled' in this.props ) { - this.editor.isReadOnly = this.props.disabled; - } + return false; } // Initialize the editor when the component is mounted. diff --git a/tests/ckeditor.jsx b/tests/ckeditor.jsx index 5d70567..1bdfc94 100644 --- a/tests/ckeditor.jsx +++ b/tests/ckeditor.jsx @@ -87,6 +87,21 @@ describe( 'CKEditor Component', () => { } ); } ); + it( 'must not update the component by React itself', done => { + sandbox.stub( Editor, 'create' ).resolves( new Editor() ); + + wrapper = mount( ); + + setTimeout( () => { + const component = wrapper.instance(); + + // This method always is called with an object with component's properties. + expect( component.shouldComponentUpdate( {} ) ).to.equal( false ); + + done(); + } ); + } ); + it( 'displays an error if something went wrong', done => { const error = new Error( 'Something went wrong.' ); const consoleErrorStub = sandbox.stub( console, 'error' ); @@ -108,59 +123,6 @@ describe( 'CKEditor Component', () => { } ); describe( 'properties', () => { - it( 'sets editor\'s data if properties have changed and contain the "data" key', done => { - const editorInstance = new Editor(); - - sandbox.stub( Editor, 'create' ).resolves( editorInstance ); - sandbox.stub( editorInstance, 'setData' ); - sandbox.stub( editorInstance, 'getData' ).returns( '

 

' ); - - wrapper = mount( ); - - setTimeout( () => { - wrapper.setProps( { data: '

Foo Bar.

' }); - - expect( editorInstance.setData.calledOnce ).to.be.true; - expect( editorInstance.setData.firstCall.args[ 0 ] ).to.equal( '

Foo Bar.

' ); - - done(); - } ); - } ); - - it( 'does not update the editor\'s data if value under "data" key is equal to editor\'s data', done => { - const editorInstance = new Editor(); - - sandbox.stub( Editor, 'create' ).resolves( editorInstance ); - sandbox.stub( editorInstance, 'setData' ); - sandbox.stub( editorInstance, 'getData' ).returns( '

Foo Bar.

' ); - - wrapper = mount( ); - - setTimeout( () => { - wrapper.setProps( { data: '

Foo Bar.

' }); - - expect( editorInstance.setData.calledOnce ).to.be.false; - - done(); - } ); - } ); - - it( 'does not set editor\'s data if the editor is not ready', () => { - const editorInstance = new Editor(); - - sandbox.stub( Editor, 'create' ).resolves( editorInstance ); - sandbox.stub( editorInstance, 'setData' ); - - wrapper = mount( ); - - const component = wrapper.instance(); - - component.componentDidUpdate( { data: 'Foo' } ); - - expect( component.editor ).to.be.null; - expect( editorInstance.setData.called ).to.be.false; - } ); - describe( '#config', () => { it( 'should replace all react DOM references with the `current` DOM element', done => { const spy = sinon.spy( Editor, 'create' ); From d4d7aa59ad152d84ba27065fde0dda85e77e0984 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 22 Mar 2019 09:18:59 +0100 Subject: [PATCH 2/3] Improved Tests section in the README. --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5420714..9ec1d52 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,9 @@ npm install ### Executing tests +Before starting tests execution, you need to build the package. You can use `npm run build` in order to build the production-ready version +or `npm run develop` which produces a development version with attached watcher for all sources files. + ```bash npm run test -- [additional options] # or @@ -46,8 +49,6 @@ an environment variable, e.g.: BROWSER_STACK_USERNAME=[...] BROWSER_STACK_ACCESS_KEY=[...] npm t -- -b BrowserStack_Edge,BrowserStack_Safari -c ``` -If you are going to change the source (`src/ckeditor.jsx`) file, remember about rebuilding the package. You can use `npm run develop` in order to do it automatically. - ### Building the package Build a minified version of the package that is ready to publish: From bdef1dfd5ac7caa03c562f7377fd302b9f6118a8 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 22 Mar 2019 13:22:51 +0100 Subject: [PATCH 3/3] Working solution for updating content of editor when the state has changed. --- package.json | 2 +- src/ckeditor.jsx | 21 +++++ tests/39/1.jsx | 1 + tests/ckeditor-integration.jsx | 153 +++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 tests/ckeditor-integration.jsx diff --git a/package.json b/package.json index 22a1fc5..6e24cfc 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "@babel/core": "^7.0.0", "@babel/preset-env": "^7.3.1", "@babel/preset-react": "^7.0.0", - "@ckeditor/ckeditor5-build-classic": "^11.0.1", + "@ckeditor/ckeditor5-build-classic": "^12.0.0", "@ckeditor/ckeditor5-dev-env": "^13.0.2", "@ckeditor/ckeditor5-dev-utils": "^11.0.1", "babel-loader": "^8.0.5", diff --git a/src/ckeditor.jsx b/src/ckeditor.jsx index bfe3c94..42b8f67 100644 --- a/src/ckeditor.jsx +++ b/src/ckeditor.jsx @@ -19,6 +19,10 @@ export default class CKEditor extends React.Component { // This component should never be updated by React itself. shouldComponentUpdate( nextProps ) { + if ( this._shouldUpdateContent( nextProps ) ) { + this.editor.setData( nextProps.data ); + } + if ( 'disabled' in nextProps ) { this.editor.isReadOnly = nextProps.disabled; } @@ -97,6 +101,23 @@ export default class CKEditor extends React.Component { } ); } } + + _shouldUpdateContent( nextProps ) { + // Check whether `nextProps.data` is equal to `this.props.data` is required if somebody defined the `#data` + // property as a static string and updated a state of component when the editor's content has been changed. + // If we avoid checking those properties, the editor's content will back to the initial value because + // the state has been changed and React will call this method. + if ( this.props.data === nextProps.data ) { + return false; + } + + // We should not change data if the editor's content is equal to the `#data` property. + if ( this.editor.getData() === nextProps.data ) { + return false; + } + + return true; + } } // Properties definition. diff --git a/tests/39/1.jsx b/tests/39/1.jsx index 33087b5..3aa3c6f 100644 --- a/tests/39/1.jsx +++ b/tests/39/1.jsx @@ -2,6 +2,7 @@ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md. */ + import React from 'react'; import ReactDOM from 'react-dom'; import { configure, mount } from 'enzyme'; diff --git a/tests/ckeditor-integration.jsx b/tests/ckeditor-integration.jsx new file mode 100644 index 0000000..5ff6759 --- /dev/null +++ b/tests/ckeditor-integration.jsx @@ -0,0 +1,153 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import React from 'react'; +import ReactDOM from 'react-dom'; +import { configure, mount } from 'enzyme'; +import Adapter from 'enzyme-adapter-react-16'; + +import CKEditor from '../src/ckeditor.jsx'; + +configure( { adapter: new Adapter() } ); + +const Editor = ( props ) => { + return ( + + ) +}; + +class AppUsingState extends React.Component { + constructor( props ) { + super( props ); + + this.state = { + content: '' + }; + + this.editor = null; + } + + render() { + return ( + this.setState( { content: editor.getData() } ) } + onInit={ _editor => this.editor = _editor } + /> + ) + } +} + +class AppUsingStaticString extends AppUsingState { + render() { + return ( + Initial data.

' } + onChange={ (evt, editor) => this.setState( { content: editor.getData() } ) } + onInit={ _editor => this.editor = _editor } + /> + ) + } +} + +describe( 'CKEditor Component - integration', () => { + describe('update the editor\'s content', () => { + // Common usage of the component - a component's state is passed to the component. + describe( '#data is connected with the state', () => { + let div, component; + + beforeEach( () => { + div = document.createElement( 'div' ); + document.body.appendChild( div ); + + return new Promise( resolve => { + component = ReactDOM.render( , div ); + + setTimeout( resolve ); + } ); + } ); + + afterEach( () => { + div.remove(); + } ); + + it( 'returns initial state', () => { + const editor = component.editor; + + expect( editor.getData() ).to.equal( '' ); + expect( component.state.content ).to.equal( '' ); + } ); + + it( 'updates the editor\'s content when state has changed', () => { + component.setState( { content: 'Foo.' } ); + + const editor = component.editor; + + // State has been updated because we attached the `onChange` callback. + expect( component.state.content ).to.equal( '

Foo.

' ); + expect( editor.getData() ).to.equal( '

Foo.

' ); + } ); + + it( 'updates state when a user typed something', () => { + const editor = component.editor; + + editor.model.change( writer => { + writer.insertText( 'Plain text.', editor.model.document.selection.getFirstPosition() ); + } ); + + expect( component.state.content ).to.equal( '

Plain text.

' ); + expect( editor.getData() ).to.equal( '

Plain text.

' ); + } ); + } ); + + // Non-common usage but it shouldn't blow or freeze the editor. + describe( '#data is a static string', () => { + let div, component; + + beforeEach( () => { + div = document.createElement( 'div' ); + document.body.appendChild( div ); + + return new Promise( resolve => { + component = ReactDOM.render( , div ); + + setTimeout( resolve ); + } ); + } ); + + afterEach( () => { + div.remove(); + } ); + + it( 'returns initial state', () => { + const editor = component.editor; + + expect( component.state.content ).to.equal( '' ); + expect( editor.getData() ).to.equal( '

Initial data.

' ); + } ); + + it( 'updates the editor\'s content when state has changed', () => { + component.setState( { content: 'Foo.' } ); + + const editor = component.editor; + + // The editor's content has not been updated because the component's `[data]` property isn't connected with it. + expect( editor.getData() ).to.equal( '

Initial data.

' ); + expect( component.state.content ).to.equal( 'Foo.' ); + } ); + + it( 'updates state when a user typed something', () => { + const editor = component.editor; + + editor.model.change( writer => { + writer.insertText( 'Plain text. ', editor.model.document.selection.getFirstPosition() ); + } ); + + expect( component.state.content ).to.equal( '

Plain text. Initial data.

' ); + expect( editor.getData() ).to.equal( '

Plain text. Initial data.

' ); + } ); + } ); + } ); +} );