Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move PluginCollection and Plugin to utils #2877

Closed
pjasiun opened this issue Jan 16, 2017 · 6 comments
Closed

Move PluginCollection and Plugin to utils #2877

pjasiun opened this issue Jan 16, 2017 · 6 comments
Labels
package:core resolution:wontfix This issue will not be fixed because the team decided that for given reasons it does not make sense.

Comments

@pjasiun
Copy link

pjasiun commented Jan 16, 2017

What Plugin/PluginCollection are, is the util to solve problems with the dependencies tree. It's enough to rename editor variable to context and change docs to make it more flexible util. It's very helpful to split a project into pieces, event if it is editor-independent.

Note that it's possible to make it compatible with the editors plugins:

export default class Plugin {
	constructor( contex ) {
		_.extend( this, contex );
	}

	// ...
}
export default class Editor {
	constructor( config ) {
		this.plugins = new PluginCollection( { editor: this } );

		// ...
	}

	// ...
}
@pjasiun
Copy link
Author

pjasiun commented Jan 16, 2017

Also, I think it would be a good change in general. A class should do one task. This class has a task to solve dependencies and it is what it really do. Making it a util lets developers understand it easier.

@Reinmar
Copy link
Member

Reinmar commented Jan 17, 2017

Makes sense. Then Plugin will need to become a util as well :D But in fact, I was rather planning to make Plugin an interface, so we could do both things at the same time. Although, the latter is a bigger change, unfortunately.

Related ticket: https://github.com/ckeditor/ckeditor5-core/issues/54.

@Reinmar
Copy link
Member

Reinmar commented Jan 17, 2017

But I don't like this for CKEditor:

_.extend( this, contex );

We should keep the editor param (so the ctx of Editor#plugins will be the editor itself). Then, in the plugins we'll do this:

constructor( editor ) {
    this.editor = editor;
}

@Reinmar
Copy link
Member

Reinmar commented Apr 11, 2017

This should be handled together with https://github.com/ckeditor/ckeditor5-core/issues/78.

@Reinmar
Copy link
Member

Reinmar commented Apr 11, 2017

BTW, soon we'll make commands and command collection utils too and ckeditor5-core will become a set of utils and interfaces (cause editor will be an interface too).

@Reinmar
Copy link
Member

Reinmar commented Nov 13, 2018

Closing due to lack of interest in this now.

@Reinmar Reinmar closed this as completed Nov 13, 2018
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-core Oct 9, 2019
@mlewand mlewand added resolution:wontfix This issue will not be fixed because the team decided that for given reasons it does not make sense. package:core labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:core resolution:wontfix This issue will not be fixed because the team decided that for given reasons it does not make sense.
Projects
None yet
Development

No branches or pull requests

3 participants