Ticket #4415 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

Bundled plugins should not be included in the distribution

Reported by: alfonsoml Owned by: wwalc
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

ckreleaser.jar (1.1 MB) - added by wwalc 5 years ago.
4415.patch (53.8 KB) - added by wwalc 5 years ago.

Change History

comment:1 Changed 5 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 5 years ago by fredck

  • Status changed from new to closed
  • Resolution set to invalid

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 5 years ago by alfonsoml

  • Status changed from closed to reopened
  • Resolution invalid deleted

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 5 years ago by fredck

  • Keywords Discussion removed
  • Status changed from reopened to new
  • Owner set to wwalc
  • Milestone set to CKEditor 3.1

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

comment:5 Changed 5 years ago by fredck

  • Summary changed from compressed plugins folder, why? to Bundled plugins should not be included in the distribution

Changed 5 years ago by wwalc

Changed 5 years ago by wwalc

comment:6 Changed 5 years ago by wwalc

  • 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 5 years ago by fredck

  • Keywords Review+ added; Review? removed

The final result is perfect really.

comment:8 Changed 5 years ago by wwalc

  • Status changed from new to closed
  • Resolution set to fixed

Fixed with [4589], [4590].

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