Skip to content

Commit

Permalink
Merge pull request #8632 from ckeditor/i/2907
Browse files Browse the repository at this point in the history
Internal: Plugin collection will allow to require plugin by name when it is provided in `config.plugins` or if it was already loaded. Closes #2907.
  • Loading branch information
pomek committed Dec 11, 2020
2 parents fa6235b + ed5bbcd commit 9c24fca
Show file tree
Hide file tree
Showing 18 changed files with 157 additions and 35 deletions.
5 changes: 1 addition & 4 deletions packages/ckeditor5-ckfinder/src/ckfinder.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@

import { Plugin } from 'ckeditor5/src/core';

// TODO: softRequires()
// import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';

import CKFinderUI from './ckfinderui';
import CKFinderEditing from './ckfinderediting';

Expand Down Expand Up @@ -45,7 +42,7 @@ export default class CKFinder extends Plugin {
* @inheritDoc
*/
static get requires() {
return [ CKFinderEditing, CKFinderUI ];
return [ 'Image', 'Link', 'CKFinderUploadAdapter', CKFinderEditing, CKFinderUI ];
}
}

Expand Down
6 changes: 1 addition & 5 deletions packages/ckeditor5-ckfinder/src/ckfinderediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
import { Plugin } from 'ckeditor5/src/core';
import { Notification } from 'ckeditor5/src/ui';

// TODO: softRequires()
// import ImageEditing from '@ckeditor/ckeditor5-image/src/image/imageediting';
// import LinkEditing from '@ckeditor/ckeditor5-link/src/linkediting';

import CKFinderCommand from './ckfindercommand';

/**
Expand All @@ -33,7 +29,7 @@ export default class CKFinderEditing extends Plugin {
* @inheritDoc
*/
static get requires() {
return [ Notification ];
return [ Notification, 'ImageEditing', 'LinkEditing' ];
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-ckfinder/tests/ckfinder.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe( 'CKFinder', () => {

return ClassicTestEditor
.create( editorElement, {
plugins: [ Image, Link, CKFinder ]
plugins: [ CKFinderUploadAdapter, Image, Link, CKFinder ]
} )
.then( newEditor => {
editor = newEditor;
Expand All @@ -47,8 +47,8 @@ describe( 'CKFinder', () => {
expect( editor.plugins.get( CKFinderEditing ) ).to.instanceOf( CKFinderEditing );
} );

it.skip( 'should load AdapterCKFinder plugin', () => {
expect( editor.plugins.get( CKFinderUploadAdapter ) ).to.instanceOf( CKFinderUploadAdapter );
it( 'should require CKFinderUploadAdapter by name', () => {
expect( CKFinder.requires ).to.contain( 'CKFinderUploadAdapter' );
} );

it( 'has proper name', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-ckfinder/tests/ckfinderediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import CKFinder from '../src/ckfinder';
import CKFinderEditing from '../src/ckfinderediting';
import Image from '@ckeditor/ckeditor5-image/src/image';
import Link from '@ckeditor/ckeditor5-link/src/link';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';

describe( 'CKFinderEditing', () => {
let editorElement, editor;
Expand All @@ -27,7 +28,7 @@ describe( 'CKFinderEditing', () => {

return ClassicTestEditor
.create( editorElement, {
plugins: [ Image, Link, CKFinder ]
plugins: [ CKFinderUploadAdapter, Image, Link, CKFinder ]

} )
.then( newEditor => {
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-ckfinder/tests/ckfinderui.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import Image from '@ckeditor/ckeditor5-image/src/image';
import Link from '@ckeditor/ckeditor5-link/src/link';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';

import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';

Expand All @@ -27,7 +28,7 @@ describe( 'CKFinderUI', () => {

return ClassicTestEditor
.create( editorElement, {
plugins: [ Image, Link, CKFinder ]
plugins: [ CKFinderUploadAdapter, Image, Link, CKFinder ]

} )
.then( newEditor => {
Expand Down
7 changes: 4 additions & 3 deletions packages/ckeditor5-ckfinder/tests/manual/ckfinder.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';

import CKFinder from '../../src/ckfinder';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
import ImageUpload from '@ckeditor/ckeditor5-image/src/imageupload';

import CKFinder from '../../src/ckfinder';

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ ArticlePluginSet, ImageUpload, CKFinder ],
plugins: [ ArticlePluginSet, ImageUpload, CKFinderUploadAdapter, CKFinder ],
toolbar: [ 'heading', '|', 'undo', 'redo', 'ckfinder' ],
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
Expand Down
43 changes: 42 additions & 1 deletion packages/ckeditor5-core/src/plugincollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,15 @@ export default class PluginCollection {
const context = this._context;
const loading = new Set();
const loaded = [];
const loadedPluginsNames = new Set();

const pluginsMap = new Map();

for ( const pluginOrName of plugins ) {
if ( typeof pluginOrName == 'function' && pluginOrName.pluginName ) {
pluginsMap.set( pluginOrName.pluginName, pluginOrName );
}
}

const pluginConstructors = mapToAvailableConstructors( plugins );
const removePluginConstructors = mapToAvailableConstructors( removePlugins );
Expand Down Expand Up @@ -273,8 +282,36 @@ export default class PluginCollection {

if ( PluginConstructor.requires ) {
PluginConstructor.requires.forEach( RequiredPluginConstructorOrName => {
if ( loadedPluginsNames.has( RequiredPluginConstructorOrName ) ) {
return;
}

const RequiredPluginConstructor = getPluginConstructor( RequiredPluginConstructorOrName );

if ( !RequiredPluginConstructor ) {
/**
* A required "soft" dependency was not found on plugin list.
*
* Plugin classes (constructors) need to be provided to the editor before they can be loaded by name.
* This is usually done in CKEditor 5 builds by setting the
* {@link module:core/editor/editor~Editor.builtinPlugins} property. Alternatively they can be provided using
* {@link module:core/editor/editorconfig~EditorConfig#plugins} or
* {@link module:core/editor/editorconfig~EditorConfig#extraPlugins} configuration.
*
* **If you see this warning when using one of the {@glink builds/index CKEditor 5 Builds}**, it means
* that you didn't add the required plugin to the plugins list when loading the editor.
*
* @error plugincollection-soft-required
* @param {String} plugin The name of the required plugin.
* @param {String} requiredBy The name of the plugin that was requiring other plugin.
*/
throw new CKEditorError(
'plugincollection-soft-required',
null,
{ plugin: RequiredPluginConstructorOrName, requiredBy: PluginConstructor.name }
);
}

if ( PluginConstructor.isContextPlugin && !RequiredPluginConstructor.isContextPlugin ) {
/**
* If a plugin is a context plugin, all plugins it requires should also be context plugins
Expand Down Expand Up @@ -318,6 +355,10 @@ export default class PluginCollection {
that._add( PluginConstructor, plugin );
loaded.push( plugin );

if ( PluginConstructor.pluginName ) {
loadedPluginsNames.add( PluginConstructor.pluginName );
}

resolve();
} );
}
Expand All @@ -327,7 +368,7 @@ export default class PluginCollection {
return PluginConstructorOrName;
}

return that._availablePlugins.get( PluginConstructorOrName );
return that._availablePlugins.get( PluginConstructorOrName ) || pluginsMap.get( PluginConstructorOrName );
}

function getMissingPluginNames( plugins ) {
Expand Down
48 changes: 48 additions & 0 deletions packages/ckeditor5-core/tests/plugincollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,54 @@ describe( 'PluginCollection', () => {
expect( plugins.get( PluginB ) ).to.equal( externalPlugins.get( PluginB ) ).to.instanceof( PluginB );
expect( plugins.get( PluginC ) ).to.instanceof( PluginC );
} );

it( 'should load dependency plugins using soft requirement', () => {
const plugins = new PluginCollection( editor, availablePlugins );
const spy = sinon.spy( plugins, '_add' );

return plugins.init( [ PluginJ ] )
.then( loadedPlugins => {
expect( getPlugins( plugins ).length ).to.equal( 3 );

expect( getPluginNames( getPluginsFromSpy( spy ) ) )
.to.deep.equal( [ 'A', 'K', 'J' ], 'order by plugins._add()' );
expect( getPluginNames( loadedPlugins ) )
.to.deep.equal( [ 'A', 'K', 'J' ], 'order by returned value' );
} );
} );

it( 'should reject dependency plugins using soft requirement when plugin is unavailable', () => {
PluginFoo.requires = [ 'A', 'Baz' ];
const consoleErrorStub = sinon.stub( console, 'error' );
const plugins = new PluginCollection( editor, availablePlugins );

return plugins.init( [ PluginFoo ] )
// Throw here, so if by any chance plugins.init() was resolved correctly catch() will be still executed.
.then( () => {
throw new Error( 'Test error: this promise should not be resolved successfully' );
} )
.catch( err => {
assertCKEditorError( err, /^plugincollection-soft-required/, null, { plugin: 'Baz', requiredBy: 'P' } );

sinon.assert.calledOnce( consoleErrorStub );
} );
} );

it( 'should not reject dependency plugins using soft requirement when plugin was loaded as dependency of other plugin', () => {
PluginFoo.requires = [ 'A' ];
const plugins = new PluginCollection( editor, availablePlugins );
const spy = sinon.spy( plugins, '_add' );

return plugins.init( [ PluginD, PluginFoo ] )
.then( loadedPlugins => {
expect( getPlugins( plugins ).length ).to.equal( 5 );

expect( getPluginNames( getPluginsFromSpy( spy ) ) )
.to.deep.equal( [ 'A', 'B', 'C', 'D', 'Foo' ], 'order by plugins._add()' );
expect( getPluginNames( loadedPlugins ) )
.to.deep.equal( [ 'A', 'B', 'C', 'D', 'Foo' ], 'order by returned value' );
} );
} );
} );

describe( 'get()', () => {
Expand Down
7 changes: 5 additions & 2 deletions packages/ckeditor5-easy-image/src/easyimage.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ import CloudServicesUploadAdapter from './cloudservicesuploadadapter';
*
* This is a "glue" plugin which enables:
*
* * {@link module:easy-image/cloudservicesuploadadapter~CloudServicesUploadAdapter}.
*
* This plugin requires plugin to be present in the editor configuration:
*
* * {@link module:image/image~Image},
* * {@link module:image/imageupload~ImageUpload},
* * {@link module:easy-image/cloudservicesuploadadapter~CloudServicesUploadAdapter}.
*
* See the {@glink features/image-upload/easy-image "Easy Image integration" guide} to learn how to configure
* and use this feature.
Expand All @@ -39,7 +42,7 @@ export default class EasyImage extends Plugin {
* @inheritDoc
*/
static get requires() {
return [ CloudServicesUploadAdapter ];
return [ CloudServicesUploadAdapter, 'Image', 'ImageUpload' ];
}

/**
Expand Down
10 changes: 8 additions & 2 deletions packages/ckeditor5-easy-image/tests/easyimage.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ describe( 'EasyImage', () => {
} );

it( 'should require other plugins', () => {
const plugins = EasyImage.requires;
expect( EasyImage.requires ).to.include( CloudServicesUploadAdapter );
} );

it( 'should require Image by name', () => {
expect( EasyImage.requires ).to.include( 'Image' );
} );

expect( plugins ).to.include( CloudServicesUploadAdapter );
it( 'should require ImageUpload by name', () => {
expect( EasyImage.requires ).to.include( 'ImageUpload' );
} );

it( 'should be able to initialize editor with itself', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-image/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"ckeditor5": "^24.0.0"
},
"devDependencies": {
"@ckeditor/ckeditor5-adapter-ckfinder": "^24.0.0",
"@ckeditor/ckeditor5-basic-styles": "^24.0.0",
"@ckeditor/ckeditor5-block-quote": "^24.0.0",
"@ckeditor/ckeditor5-ckfinder": "^24.0.0",
Expand Down
28 changes: 20 additions & 8 deletions packages/ckeditor5-image/tests/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,32 @@ describe( 'Image', () => {

expect( getViewData( view ) ).to.equal(
'<figure class="' +
'ck-widget ' +
'image" contenteditable="false"' +
'ck-widget ' +
'image" contenteditable="false"' +
'>' +
'<img alt="alt text" src="/assets/sample.png"></img>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'<img alt="alt text" src="/assets/sample.png"></img>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'</figure>' +
'[<figure class="' +
'ck-widget ' +
'ck-widget_selected image" contenteditable="false"' +
'ck-widget ' +
'ck-widget_selected image" contenteditable="false"' +
'>' +
'<img alt="alt text" src="/assets/sample.png"></img>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'<img alt="alt text" src="/assets/sample.png"></img>' +
'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
'</figure>]'
);
} );
} );

describe( 'isImageWidget()', () => {
it( 'should expose isImageWidget() utility', () => {
expect( editor.plugins.get( 'Image' ) ).to.respondTo( 'isImageWidget' );
} );

it( 'should return true for elements marked with toImageWidget()', () => {
setModelData( model, '[<image alt="alt text" src="/assets/sample.png"></image>]' );
const element = viewDocument.getRoot().getChild( 0 );
expect( editor.plugins.get( 'Image' ).isImageWidget( element ) ).to.be.true;
} );
} );
} );
6 changes: 5 additions & 1 deletion packages/ckeditor5-image/tests/imageinsert/imageinsertui.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import LabeledFieldView from '@ckeditor/ckeditor5-ui/src/labeledfield/labeledfie
import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';

import { UploadAdapterMock } from '@ckeditor/ckeditor5-upload/tests/_utils/mocks';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';
import Link from '@ckeditor/ckeditor5-link/src/link';

describe( 'ImageInsertUI', () => {
let editor, editorElement, fileRepository, dropdown;
Expand Down Expand Up @@ -298,9 +300,11 @@ describe( 'ImageInsertUI', () => {
const editor = await ClassicEditor
.create( editorElement, {
plugins: [
Link,
Image,
CKFinderUploadAdapter,
CKFinder,
Paragraph,
Image,
ImageInsert,
ImageInsertUI,
FileRepository,
Expand Down
7 changes: 5 additions & 2 deletions packages/ckeditor5-image/tests/imageinsert/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Link from '@ckeditor/ckeditor5-link/src/link';
import CKFinder from '@ckeditor/ckeditor5-ckfinder/src/ckfinder';
import { prepareIntegrations, createLabeledInputView } from '../../src/imageinsert/utils';
import CKFinderUploadAdapter from '@ckeditor/ckeditor5-adapter-ckfinder/src/uploadadapter';

describe( 'Upload utils', () => {
describe( 'prepareIntegrations()', () => {
Expand All @@ -24,10 +25,12 @@ describe( 'Upload utils', () => {
const editor = await ClassicEditor
.create( editorElement, {
plugins: [
CKFinder,
Paragraph,
Link,
Image,
ImageUploadUI
ImageUploadUI,
CKFinderUploadAdapter,
CKFinder
],
image: {
insert: {
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-link/src/linkimageediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class LinkImageEditing extends Plugin {
* @inheritDoc
*/
static get requires() {
return [ LinkEditing ];
return [ 'ImageEditing', LinkEditing ];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-link/src/linkimageui.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default class LinkImageUI extends Plugin {
* @inheritDoc
*/
static get requires() {
return [ LinkEditing, LinkUI ];
return [ LinkEditing, LinkUI, 'Image' ];
}

/**
Expand Down

0 comments on commit 9c24fca

Please sign in to comment.