Skip to content

Commit

Permalink
Merge 8fad32b into f271c49
Browse files Browse the repository at this point in the history
  • Loading branch information
pomek committed Mar 25, 2021
2 parents f271c49 + 8fad32b commit b92fc28
Show file tree
Hide file tree
Showing 15 changed files with 664 additions and 365 deletions.
9 changes: 6 additions & 3 deletions scripts/utils/getkarmaconfig.js
Expand Up @@ -70,11 +70,11 @@ module.exports = function getKarmaConfig() {
customLaunchers: {
CHROME_TRAVIS_CI: {
base: 'Chrome',
flags: [ '--no-sandbox', '--disable-background-timer-throttling' ]
flags: [ '--disable-background-timer-throttling', '--js-flags="--expose-gc"' ]
},
CHROME_LOCAL: {
base: 'Chrome',
flags: [ '--disable-background-timer-throttling' ]
flags: [ '--disable-background-timer-throttling', '--js-flags="--expose-gc"' ]
}
},

Expand Down Expand Up @@ -123,7 +123,10 @@ module.exports = function getKarmaConfig() {
webpackConfig.module.rules.push( {
test: /\.jsx?$/,
loader: 'istanbul-instrumenter-loader',
include: /src/,
include: [
/src/,
/_utils-tests/
],
exclude: [
/node_modules/
],
Expand Down
63 changes: 45 additions & 18 deletions src/ckeditor.jsx
Expand Up @@ -21,13 +21,15 @@ export default class CKEditor extends React.Component {
/**
* An instance of EditorWatchdog or an instance of EditorWatchdog-like adapter for ContextWatchdog.
*
* @type {EditorWatchdog|EditorWatchdogAdapter}
* @type {module:watchdog/watchdog~Watchdog|EditorWatchdogAdapter}
*/
this.watchdog = null;
}

/**
* An editor instance.
*
* @type {module:core/editor/editor~Editor|null}
*/
get editor() {
if ( !this.watchdog ) {
Expand All @@ -37,17 +39,20 @@ export default class CKEditor extends React.Component {
return this.watchdog.editor;
}

// The CKEditor component should not be updated by React itself.
// However, if the component identifier changes, the whole structure should be created once again.
/**
* The CKEditor component should not be updated by React itself.
* However, if the component identifier changes, the whole structure should be created once again.
*
* @param {Object} nextProps
* @return {Boolean}
*/
shouldComponentUpdate( nextProps ) {
if ( !this.editor ) {
return false;
}

// Only when the editor component changes the whole structure should be restarted.
// Only when the component identifier changes the whole structure should be re-created once again.
if ( nextProps.id !== this.props.id ) {
this._destroyEditor();

return true;
}

Expand All @@ -62,22 +67,33 @@ export default class CKEditor extends React.Component {
return false;
}

// Initialize the editor when the component is mounted.
/**
* Initialize the editor when the component is mounted.
*/
componentDidMount() {
this._initializeEditor();
}

// Initialize the editor when the component is updated as it should be destroyed before the update.
/**
* Re-render the entire component once again. The old editor will be destroyed and the new one will be created.
*/
componentDidUpdate() {
this._destroyEditor();
this._initializeEditor();
}

// Destroy the editor before unmouting the component.
/**
* Destroy the editor before unmounting the component.
*/
componentWillUnmount() {
this._destroyEditor();
}

// Render a <div> element which will be replaced by CKEditor.
/**
* Render a <div> element which will be replaced by CKEditor.
*
* @return {JSX.Element}
*/
render() {
return (
<div ref={ this.domContainer }></div>
Expand All @@ -86,12 +102,14 @@ export default class CKEditor extends React.Component {

/**
* Initializes the editor by creating a proper watchdog and initializing it with the editor's configuration.
*
* @private
*/
_initializeEditor() {
if ( this.context instanceof ContextWatchdog ) {
this.watchdog = new EditorWatchdogAdapter( this.context );
} else {
this.watchdog = new EditorWatchdog( this.editor );
this.watchdog = new CKEditor._EditorWatchdog( this.props.editor );
}

this.watchdog.setCreator( ( el, config ) => this._createEditor( el, config ) );
Expand All @@ -107,6 +125,7 @@ export default class CKEditor extends React.Component {
/**
* Creates an editor from the element and configuration.
*
* @private
* @param {HTMLElement} element The source element.
* @param {Object} config CKEditor 5 editor configuration.
* @returns {Promise}
Expand Down Expand Up @@ -157,6 +176,8 @@ export default class CKEditor extends React.Component {

/**
* Destroys the editor by destroying the watchdog.
*
* @private
*/
_destroyEditor() {
// It may happen during the tests that the watchdog instance is not assigned before destroying itself. See: #197.
Expand All @@ -172,7 +193,8 @@ export default class CKEditor extends React.Component {
/**
* Returns true when the editor should be updated.
*
* @param {*} nextProps React's properties.
* @private
* @param {Object} nextProps React's properties.
* @returns {Boolean}
*/
_shouldUpdateEditor( nextProps ) {
Expand All @@ -192,6 +214,12 @@ export default class CKEditor extends React.Component {
return true;
}

/**
* Returns the editor configuration.
*
* @private
* @return {Object}
*/
_getConfig() {
if ( this.props.data && this.props.config.initialData ) {
console.warn(
Expand Down Expand Up @@ -277,12 +305,7 @@ class EditorWatchdogAdapter {
* An editor instance.
*/
get editor() {
// TODO - try/catch should not be necessary as `getItem` could return `null` instead of throwing errors.
try {
return this._contextWatchdog.getItem( this._id );
} catch ( err ) {
return null;
}
return this._contextWatchdog.getItem( this._id );
}
}

Expand Down Expand Up @@ -313,3 +336,7 @@ CKEditor.defaultProps = {
config: {},
onError: ( error, details ) => console.error( error, details )
};

// Store the API in the static property to easily overwrite it in tests.
// Too bad dependency injection does not work in Webpack + ES 6 (const) + Babel.
CKEditor._EditorWatchdog = EditorWatchdog;
64 changes: 45 additions & 19 deletions src/ckeditorcontext.jsx
Expand Up @@ -13,34 +13,59 @@ export default class CKEditorContext extends React.Component {
constructor( props, context ) {
super( props, context );

/**
* @type {module:watchdog/contextwatchdog~ContextWatchdog|null}
*/
this.contextWatchdog = null;

if ( this.props.isLayoutReady ) {
this._initializeContextWatchdog( this.props.config );
this._initializeContextWatchdog();
}
}

/**
* Checks whether the component should be updated by React itself.
*
* @param {Object} nextProps
* @return {Boolean}
*/
shouldComponentUpdate( nextProps ) {
// If the configuration changes then the ContextWatchdog needs to be destroyed and recreated
// On top of the new configuration.
// If the config has changed then the `ContextWatchdog` needs to be destroyed and recreated using the new config.
if ( nextProps.id !== this.props.id ) {
if ( this.contextWatchdog ) {
this.contextWatchdog.destroy();
}

this._initializeContextWatchdog( nextProps.config );
return true;
}

// Rerender the component once again if the layout is ready.
if ( nextProps.isLayoutReady && !this.contextWatchdog ) {
this._initializeContextWatchdog( nextProps.config );

return true;
}

// Rerender the component only when children has changed.
return this.props.children !== nextProps.children;
}

/**
* Re-render the entire component once again. The old editor will be destroyed and the new one will be created.
*/
componentDidUpdate() {
// If the `isLayoutReady` property has changed from `false` to `true`, the instance of `ContextWatchdog` does not exist.
if ( this.contextWatchdog ) {
this.contextWatchdog.destroy();
}

this._initializeContextWatchdog();
}

/**
* Destroy the context before unmounting the component.
*/
componentWillUnmount() {
this._destroyContext();
}

/**
* @return {JSX.Element}
*/
render() {
return (
<ContextWatchdogContext.Provider value={ this.contextWatchdog } >
Expand All @@ -49,14 +74,13 @@ export default class CKEditorContext extends React.Component {
);
}

componentWillUnmount() {
this._destroyContext();
}

_initializeContextWatchdog( config ) {
/**
* @private
*/
_initializeContextWatchdog() {
this.contextWatchdog = new ContextWatchdog( this.props.context );

this.contextWatchdog.create( config )
this.contextWatchdog.create( this.props.config )
.catch( error => {
this.props.onError( error, {
phase: 'initialization',
Expand All @@ -78,6 +102,9 @@ export default class CKEditorContext extends React.Component {
} );
}

/**
* @private
*/
_destroyContext() {
if ( this.contextWatchdog ) {
this.contextWatchdog.destroy();
Expand All @@ -88,16 +115,15 @@ export default class CKEditorContext extends React.Component {

CKEditorContext.defaultProps = {
isLayoutReady: true,
onError: ( error, details ) => console.error( error, details )
onError: console.error
};

// Properties definition.
CKEditorContext.propTypes = {
context: PropTypes.func.isRequired,
id: PropTypes.string,
isLayoutReady: PropTypes.bool,
context: PropTypes.func,
config: PropTypes.object,
onReady: PropTypes.func,
onError: PropTypes.func
};

40 changes: 40 additions & 0 deletions tests/_utils-tests/context.js
@@ -0,0 +1,40 @@
/**
* @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

import Context from '../_utils/context';

describe( 'Context', () => {
describe( 'constructor()', () => {
it( 'saves the config', () => {
const config = { foo: 'bar' };
const context = new Context( config );

expect( context.config ).to.equal( config );
} );
} );

describe( 'destroy()', () => {
it( 'should return a promise that resolves properly', () => {
return Context.create()
.then( context => {
const promise = context.destroy();

expect( promise ).to.be.an.instanceof( Promise );

return promise;
} );
} );
} );

describe( 'create()', () => {
it( 'should return a promise that resolves properly', () => {
const promise = Context.create();

expect( promise ).to.be.an.instanceof( Promise );

return promise;
} );
} );
} );
25 changes: 14 additions & 11 deletions tests/_utils-tests/turnoffdefaulterrorcatching.js
@@ -1,19 +1,22 @@
/**
* @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
* For licensing, see LICENSE.md.
*/

/* global window */

/**
* Turns off the default error catching
* so Mocha won't complain about errors caused by the called function.
*/
export default async function turnOffDefaultErrorCatching( fn ) {
const originalOnError = window.onerror;
window.onerror = null;
import turnOffDefaultErrorCatching from '../_utils/turnoffdefaulterrorcatching';

describe( 'turnOffDefaultErrorCatching()', () => {
it( 'should catch the error', () => {
const onErrorStub = sinon.stub( window, 'onerror' );

turnOffDefaultErrorCatching( () => {
window.onerror( 'Foo', null, 0 );
} );

await fn();
onErrorStub.restore();

window.onerror = originalOnError;
}
expect( onErrorStub.called ).to.equal( false );
} );
} );

0 comments on commit b92fc28

Please sign in to comment.