Opened 8 years ago

Closed 8 years ago

#4415 closed Bug (fixed)

Bundled plugins should not be included in the distribution

Reported by: Alfonso Martínez de Lizarrondo Owned by: Wiktor Walc
Priority: Normal Milestone: CKEditor 3.1
Component: General Version: 3.0
Keywords: Confirmed Review+ Cc:

Description

First of all #3855 must be fixed, then: why are the compressed plugins distributed?

If they are merged into ckeditor.js they are just a waste of space as they will never be loaded and being compressed makes them no interesting at all.

If any plugin that it's compressed it's deleted it will make the distribution a little smaller and people won't waste time trying to understand them instead of looking at _source.

If the reason is about people repackaging their build, it should work with _source because sometimes they might want to modify some little code in some of the plugins.

At the very least: as most of them aren't needed, people should be able to easily know what they can delete before deploying CKEditor to the production server.

Attachments (2)

ckreleaser.jar (1.1 MB) - added by Wiktor Walc 8 years ago.
4415.patch (53.8 KB) - added by Wiktor Walc 8 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by Garry Yao

Keywords: Confirmed Discussion added

Roughly it's because there're dependencies in some plugins which are supposed to be lazy loaded, and there're certain plugins that doesn't configured by default, those two categories of files won't be bundled. While the distribution package could be further optimized based on a different deployment profile (loading way majorly).
A plugins repository and a builder is planned at our site as resolution which will comes later this year.
Leave the ticket open for discussion.

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Resolution: invalid
Status: newclosed

Let's say that those plugins are "production ready". The plugins don't know what we have in ckeditor.js, which can potentially be customized to not include the plugins code.

For now, we'll stay this way. We'll have a better solution with the builder... there is no date for it though.

comment:3 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Resolution: invalid
Status: closedreopened

I think that the releaser/packager/whatever knows the files that it has put into the ckeditor.js, so the possible solution would be to delete whatever file that it's merging into ckeditor.js and finally delete any empty folder that have remained.

Just that change would be enough to remove all the code that it isn't used and so the distribution file will be slightly smaller and people will have to handle less files in their ftps, knowing much better what they can delete and what is important (the folders that remain under plugins will be optional plugins, so if they don't want to use those commands they won't cause any trouble by deleting them). And also: when they change any file under plugins, they will be able to see the changes run, now they must be sure that it isn't a plugin that it's already merged so it isn't never loaded from the standalone instance.

couldn't this be done before waiting for the builder?

comment:4 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Discussion removed
Milestone: CKEditor 3.1
Owner: set to Wiktor Walc
Status: reopenednew

Ok Alfonso... it makes sense. After all we still don't have all necessary tools to properly customize the packaged file.

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Summary: compressed plugins folder, why?Bundled plugins should not be included in the distribution

Changed 8 years ago by Wiktor Walc

Attachment: ckreleaser.jar added

Changed 8 years ago by Wiktor Walc

Attachment: 4415.patch added

comment:6 Changed 8 years ago by Wiktor Walc

Keywords: Review? added

Use compiled ckreleaser.jar to test the patch. To test it with CKEditor, in ckreleaser.release an "ignore" instruction should be added in the "copy" section (because ckreleaser.release defines one or more packages, we need to tell CKReleaser somehow which files should be ignored):

copy :
[
	{
		source : '_source/lang',
		target : 'lang',
		minify : true
	},

	{
		source : '_source/plugins',
		target : 'plugins',
		minify : true,
		ignore : {
			sourcePackage : 'ckeditor.pack',
			files : 'packages[1].files'
		}
	},

	{
		source : '_source/themes',
		target : 'themes',
		minify : true
	}
],

Note that although compressed plugin.js files are removed, some plugin directories remain, because either they contain image files or dialogs.

I think this is fine anyway because the number of files that should be uploaded to the server have been reduced.

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

The final result is perfect really.

comment:8 Changed 8 years ago by Wiktor Walc

Resolution: fixed
Status: newclosed

Fixed with [4589], [4590].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy