Skip to content

Commit

Permalink
Merge pull request #310 from ckeditor/i/294
Browse files Browse the repository at this point in the history 
Fix: Fixed component double rendering in StrictMode. Closes #294.
  • Loading branch information
@pomek
pomek authored May 18, 2022
2 parents d680f5d + 77b6bab commit b17ddd4
Show file tree
Hide file tree
Showing 4 changed files with 644 additions and 1,085 deletions.
141 changes: 141 additions & 0 deletions sample/index-18.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="utf-8">
<title>CKEditor 5 – React Component – development sample</title>
<style>
body {
max-width: 800px;
margin: 20px auto;
}

.button_read-only {
margin: 10px 0;
}

</style>
</head>

<body>
<h1>CKEditor 5 – React@18 Component – development sample</h1>
<p style="text-align: center;">Component's events are logged to the console. <b>React.StrictMode</b> is enabled.</p>
<button id="readOnly" class="button_read-only" disabled>Switch to read-only mode</button>
<button id="simulateError" class="button_read-only" disabled>Simulate an error</button>
<button id="previousDocumentID" class="button_read-only" disabled>Previous document id</button>
<button id="nextDocumentID" class="button_read-only">Next document id</button>
<div id="root"></div>
<script src="https://unpkg.com/react@18/umd/react.development.js"></script>
<script src="https://unpkg.com/react-dom@18/umd/react-dom.development.js"></script>
<script src="https://cdn.ckeditor.com/ckeditor5/34.0.0/classic/ckeditor.js"></script>
<script src="../dist/ckeditor.js"></script>
<script>
// TODO: Move buttons to the App component.
const nextDocumentIDButton = document.getElementById( 'nextDocumentID' );
const previousDocumentIDButton = document.getElementById( 'previousDocumentID' );
const readOnlyButton = document.getElementById( 'readOnly' );
const simulateErrorButton = document.getElementById( 'simulateError' );

const SAMPLE_READ_ONLY_LOCK_ID = 'Integration Sample';

class App extends React.Component {
constructor( props ) {
super( props );

this.state = { documents: [ editorContent ], documentID: 0 }
}

render() {
return React.createElement( CKEditor.CKEditor, {
editor: ClassicEditor,
data: this.state.documents[ this.state.documentID ],
id: this.state.documentID,
onReady: editor => {
window.editor = editor;

console.log( 'event: onReady' );
console.log( 'Editor is ready to use! You can use "editor" variable to play with it.' );

readOnlyButton.disabled = false;
simulateErrorButton.disabled = false;
previousDocumentIDButton.disabled = this.state.documentID === 0;

editor.ui.view.listenTo( simulateErrorButton, 'click', () => {
setTimeout( () => {
const err = new Error( 'foo' );
err.context = editor;
err.is = () => true;

throw err;
} );
} );

editor.ui.view.listenTo( nextDocumentIDButton, 'click', () => {
this.setState( {
documentID: this.state.documentID + 1,
documents: this.state.documents.length < this.state.documentID + 1 ?
this.state.documents :
[ ...this.state.documents, editorContent ]
} )
} );

editor.ui.view.listenTo( previousDocumentIDButton, 'click', () => {
this.setState( { documentID: Math.max( this.state.documentID - 1, 0 ) } )
} );

editor.ui.view.listenTo( readOnlyButton, 'click', () => {
if ( editor.isReadOnly ) {
editor.disableReadOnlyMode( SAMPLE_READ_ONLY_LOCK_ID );
readOnlyButton.innerText = 'Switch to read-only mode';
} else {
editor.enableReadOnlyMode( SAMPLE_READ_ONLY_LOCK_ID );
readOnlyButton.innerText = 'Switch to editable mode';
}
} );
},
onChange: ( event, editor ) => {
this.updateData();

console.log( 'event: onChange', { event, editor } );
},

onBlur: ( event, editor ) => {
console.log( 'event: onBlur', { event, editor } );
},
onFocus: ( event, editor ) => {
console.log( 'event: onFocus', { event, editor } );
}
} )
}

updateData() {
this.setState( {
documents: this.state.documents.map( ( data, index ) => {
if ( index === this.state.documentID ) {
return editor.getData();
}

return data;
} )
} );
}
}

const editorContent = `
<h2>Sample</h2>
<p>This is an instance of the <a href="https://ckeditor.com/docs/ckeditor5/latest/builds/guides/overview.html#classic-editor">classic editor build</a>.</p>
<figure class="image">
<img src="./sample.jpg" alt="CKEditor 5 Sample image." />
</figure>
<p>You can use this sample to validate whether your <a href="https://ckeditor.com/docs/ckeditor5/latest/builds/guides/development/custom-builds.html">custom build</a> works fine.</p>
`;

document.addEventListener( 'DOMContentLoaded', () => {
ReactDOM
.createRoot( document.getElementById( 'root' ) )
.render( React.createElement( React.StrictMode, null, React.createElement( App ) ) );
} );
</script>
</body>

</html>
36 changes: 24 additions & 12 deletions src/ckeditor.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,24 +92,30 @@ export default class CKEditor extends React.Component {

/**
* Initialize the editor when the component is mounted.
*
* @returns {Promise}
*/
componentDidMount() {
this._initializeEditor();
async componentDidMount() {
await this._initializeEditor();
}

/**
* Re-render the entire component once again. The old editor will be destroyed and the new one will be created.
*
* @returns {Promise}
*/
componentDidUpdate() {
this._destroyEditor();
this._initializeEditor();
async componentDidUpdate() {
await this._destroyEditor();
await this._initializeEditor();
}

/**
* Destroy the editor before unmounting the component.
*
* @returns {Promise}
*/
componentWillUnmount() {
this._destroyEditor();
async componentWillUnmount() {
await this._destroyEditor();
}

/**
Expand All @@ -127,8 +133,13 @@ export default class CKEditor extends React.Component {
* Initializes the editor by creating a proper watchdog and initializing it with the editor's configuration.
*
* @private
* @returns {Promise}
*/
_initializeEditor() {
async _initializeEditor() {
if ( this.watchdog ) {
return;
}

if ( this.context instanceof ContextWatchdog ) {
this.watchdog = new EditorWatchdogAdapter( this.context );
} else {
Expand All @@ -141,7 +152,7 @@ export default class CKEditor extends React.Component {
this.props.onError( error, { phase: 'runtime', willEditorRestart: causesRestart } );
} );

this.watchdog.create( this.domContainer.current, this._getConfig() )
await this.watchdog.create( this.domContainer.current, this._getConfig() )
.catch( error => this.props.onError( error, { phase: 'initialization', willEditorRestart: false } ) );
}

Expand Down Expand Up @@ -205,15 +216,16 @@ export default class CKEditor extends React.Component {
* Destroys the editor by destroying the watchdog.
*
* @private
* @returns {Promise}
*/
_destroyEditor() {
async _destroyEditor() {
// It may happen during the tests that the watchdog instance is not assigned before destroying itself. See: #197.
/* istanbul ignore next */
if ( !this.watchdog ) {
if ( !this.editor ) {
return;
}

this.watchdog.destroy();
await this.watchdog.destroy();
this.watchdog = null;
}

Expand Down
30 changes: 15 additions & 15 deletions src/ckeditorcontext.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,20 @@ export default class CKEditorContext extends React.Component {
}
}

shouldComponentUpdate( nextProps ) {
async shouldComponentUpdate( nextProps ) {

This comment has been minimized.

Sign in to view

Copy link
@imjordanxd

imjordanxd May 18, 2022

Bug? shouldComponentUpdate should synchronously return a boolean. Promises are treated as truthy, therefore this will force an update regardless
// If the configuration changes then the ContextWatchdog needs to be destroyed and recreated
// On top of the new configuration.
if ( nextProps.id !== this.props.id ) {
/* istanbul ignore else */
if ( this.contextWatchdog ) {
this.contextWatchdog.destroy();
await this.contextWatchdog.destroy();
}

this._initializeContextWatchdog( nextProps.config );
await this._initializeContextWatchdog( nextProps.config );
}

if ( nextProps.isLayoutReady && !this.contextWatchdog ) {
this._initializeContextWatchdog( nextProps.config );
await this._initializeContextWatchdog( nextProps.config );

return true;
}
Expand All @@ -50,21 +50,13 @@ export default class CKEditorContext extends React.Component {
);
}

componentWillUnmount() {
this._destroyContext();
async componentWillUnmount() {
await this._destroyContext();
}

_initializeContextWatchdog( config ) {
async _initializeContextWatchdog( config ) {
this.contextWatchdog = new ContextWatchdog( this.props.context );

this.contextWatchdog.create( config )
.catch( error => {
this.props.onError( error, {
phase: 'initialization',
willContextRestart: false
} );
} );

this.contextWatchdog.on( 'error', ( _, errorEvent ) => {
this.props.onError( errorEvent.error, {
phase: 'runtime',
Expand All @@ -77,6 +69,14 @@ export default class CKEditorContext extends React.Component {
this.props.onReady( this.contextWatchdog.context );
}
} );

await this.contextWatchdog.create( config )
.catch( error => {
this.props.onError( error, {
phase: 'initialization',
willContextRestart: false
} );
} );
}

async _destroyContext() {
Expand Down
Loading

0 comments on commit b17ddd4

Please sign in to comment.