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

t/5: Introducing the CKEDITOR namespace, the AMD API and the Builder. #6

Merged
merged 34 commits into from Jan 19, 2015
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
1c16b97
Added the ckeditor5-core dependency.
fredck Jan 9, 2015
b44682c
Added the original RequireJS script.
fredck Jan 9, 2015
03f78c4
Added the entry point script: ckeditor.js.
fredck Jan 9, 2015
2c6e785
Added minimum configuration for RequireJS.
fredck Jan 9, 2015
a23bda8
Exposed the AMD API through the CKEDITOR namespace.
fredck Jan 9, 2015
1eddc34
Added support for "plugin!" and properly initialized the CKEDITOR mod…
fredck Jan 12, 2015
9368232
Introduced the basePath calculation, making RequireJS paths relative …
fredck Jan 12, 2015
e0ebc2c
Made the dev version of CKEDITOR override the core one through module…
fredck Jan 13, 2015
ba2f8c6
Workaround for the name conflict between the core "plugin" module and…
fredck Jan 13, 2015
130b2c7
Linting fixes.
fredck Jan 13, 2015
a3a4956
Introduced "grunt build".
fredck Jan 14, 2015
1dce371
Added the Almond source to the build.
fredck Jan 14, 2015
767f6ac
Do not initialize the dependency tree as this is out of the scope of …
fredck Jan 14, 2015
ef522dd
Shared the ckeditor5/ckeditor.js code between dev and build versions …
fredck Jan 14, 2015
255d2b5
Enabled uglify by default.
fredck Jan 14, 2015
2f578cf
Removed unnecessary comments.
fredck Jan 15, 2015
051574a
Got rid of a weird way to read a json file.
fredck Jan 15, 2015
6113982
Removed an unnecessary callback wrapping.
fredck Jan 15, 2015
c2413f0
Removed yet another unnecessary callback wrapping.
fredck Jan 15, 2015
c3fc22c
Extracted the builder logic into a class. Added minimum documentation…
fredck Jan 16, 2015
667acb1
Moved the start and end code fragments to be injected in the build to…
fredck Jan 16, 2015
76d64c6
Added documentation for the start and end build fragments.
fredck Jan 16, 2015
f4074f0
Stop the building process if the tmp folder already exists.
fredck Jan 19, 2015
9a86ebb
Exposed the `getBasePath` method to enable testing it more easily.
fredck Jan 19, 2015
10246ac
Minor fixes on style, typos, etc.
fredck Jan 19, 2015
3d4fbbe
Minor simplification.
fredck Jan 19, 2015
950871b
Fixed `@type` to `@property`.
fredck Jan 19, 2015
66297bb
Changed the way the Builder class is exported.
fredck Jan 19, 2015
49b17a2
Removed the RequireJS distro from the repo making it a npm dependency.
fredck Jan 19, 2015
8880b20
Added proper information about the build directory.
fredck Jan 19, 2015
990f135
Using console.error where appropriate.
fredck Jan 19, 2015
2c89d5d
Added reference to avoid newbies confusion.
fredck Jan 19, 2015
ef856f4
Removed an unnecessary argument passed to define().
fredck Jan 19, 2015
1a37506
Follow the naming standard.
Reinmar Jan 19, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions LICENSE.md
Expand Up @@ -24,6 +24,15 @@ Where not otherwise indicated, all CKEditor content is authored by CKSource engi
intellectual property. In some specific instances, CKEditor will incorporate work done by developers outside of CKSource
with their express permission.

(Ignore this line: %REMOVE_START%)

Software available at our [repository](https://github.com/ckeditor/ckeditor5) and developer version only:

- RequireJS (lib/requirejs) <br>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use * insteaf of -.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

Licensed under the terms of the MIT or new BSD license. <br>
Copyright (c) 2010-2014, The Dojo Foundation All Rights Reserved.

(Ignore this line: %REMOVE_END%)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove the first or last blank line inside the above block we will be able to add a blank line after it, so the next header will be visually separated from the contents of this section :P.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't understand this one. Feel free to change it yourself.

Trademarks
----------

Expand Down
105 changes: 105 additions & 0 deletions ckeditor.js
@@ -0,0 +1,105 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* global requirejs, define, require, window, document, location */

'use strict';

// This file is shared by the dev and release versions of CKEditor. It bootstraps the API.

( function( root ) {
var CKEDITOR = root.CKEDITOR = {
/**
* The full URL for the CKEditor installation directory.
*
* It is possible to manually provide the base path by setting a global variable named `CKEDITOR_BASEPATH`. This
* global variable must be set **before** the editor script loading.
*
* alert( CKEDITOR.basePath ); // e.g. 'http://www.example.com/ckeditor/'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log will be more appropriate these days :D.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

*
* @property {String}
*/
basePath: getBasePath(),

/**
* Defines an AMD module.
*
* See https://github.com/ckeditor/ckeditor5-design/wiki/AMD for more details about our AMD API.
*
* @method
* @member CKEDITOR
*/
define: define,

/**
* Retrieves one or more AMD modules.
*
* Note that the CKEditor AMD API does not download modules on demand so be sure to have their relative scripts
* available in the page.
*
* See https://github.com/ckeditor/ckeditor5-design/wiki/AMD for more details about our AMD API.
*
* @method
* @member CKEDITOR
*/
require: require
};

requirejs.config( {
// Modules are generally relative to the core project.
baseUrl: CKEDITOR.basePath + 'node_modules/ckeditor5-core/src/',

// These configurations will make no difference in the build version because the following paths will be
// already defined there.
paths: {
// Hide the core "ckeditor" under a different name.
'ckeditor-core': CKEDITOR.basePath + 'node_modules/ckeditor5-core/src/ckeditor',

// The dev version overrides for the "ckeditor" module. This is empty on release.
'ckeditor-dev': CKEDITOR.basePath + 'src/ckeditor-dev'
}
} );

// Define a new "ckeditor" module, which overrides the core one with the above and the dev stuff.
define( 'ckeditor', [ 'ckeditor-core', 'ckeditor-dev', 'utils' ], function( core, dev, utils ) {
utils.extend( core, root.CKEDITOR, ( dev || {} ) );
root.CKEDITOR = core;

return core;
} );

function getBasePath() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should be testable. We already had problem in CKEditor 4 that in order to test the base path we had to load multiple iframes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 9a86ebb.

if ( window.CKEDITOR_BASEPATH ) {
return window.CKEDITOR_BASEPATH;
}

var scripts = document.getElementsByTagName( 'script' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why so much spacing between variable declarations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.


var basePathSrcPattern = /(^|.*[\\\/])ckeditor\.js(?:\?.*|;.*)?$/i;

var path;

// Find the first script that src matches ckeditor.js.
[].some.call( scripts, function( script ) {
var match = script.src.match( basePathSrcPattern );

if ( match ) {
path = match[ 1 ];

return true;
}
} );

if ( path.indexOf( ':/' ) == -1 && path.slice( 0, 2 ) != '//' ) {
if ( path.indexOf( '/' ) === 0 ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path[ 0 ] == '/' would be shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 3d4fbbe.

path = location.href.match( /^.*?:\/\/[^\/]*/ )[ 0 ] + path;
} else {
path = location.href.match( /^[^\?]*\/(?:)/ )[ 0 ] + path;
}
}

return path;
}
} )( window );
18 changes: 18 additions & 0 deletions dev/tasks/build.js
@@ -0,0 +1,18 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* jshint node: true */

'use strict';

var Builder = require( './build/builder' );

module.exports = function( grunt ) {
grunt.registerTask( 'build', 'Build a release out of the current development code.', function() {
var done = this.async();
var builder = new Builder();
builder.build( done );
} );
};
217 changes: 217 additions & 0 deletions dev/tasks/build/builder.js
@@ -0,0 +1,217 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* jshint node: true */

'use strict';

var Builder;

/**
* A CKEditor 5 release builder.
*
* @class Builder
*/
module.exports = Builder = function( target ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's slightly better to use function declaration and then assign it to the module.exports, because it leaves the function named.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 66297bb.

/**
* The target directory where to create the build.
*
* **Warning**: if existing, this directory will be deleted before processing.
*
* @type {string}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use the @type tag.

The use of this tag is deprecated. It's only supported for backwards compatibility with old ext-doc. For new code always use the @Property tag instead.

https://github.com/senchalabs/jsduck/wiki/@type

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a talk about doc tags we can also discus if we want to stick with JSDuck, see ckeditor/ckeditor5-design#27.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 950871b.

*/
this.target = target || 'build';

/**
* The temporary directory to use for build processing.
*
* **Warning**: if existing, this directory will be deleted before processing.
*
* @type {string}
*/
this.tmp = 'tmp';

/**
* The list of tasks to be executed by the `build()` method. Each entry is an Array containing the name of the
* method inside `tasks` to execute and the message to show to the end user when executing it.
*
* @type {Array}
*/
this.taskList = [
[ 'cleanup', 'Cleaning the "' + target + '" directory...' ],
[ 'copyToTmp', 'Copying source files for manipulation...' ],
[ 'removeAmdNamespace', 'AMD cleanup...' ],
[ 'optimize', 'Creating the optimized code...' ],
[ 'cleanupTmp', 'Removing the "' + this.tmp + '" directory...' ]
];
};

Builder.prototype = {
/**
* Builds a CKEditor release based on the current development code.
*
* @param {Function} [callback] Function to be called when build finishes.
*/
build: function( callback ) {
var that = this;
var stepCounter = 0;

runNext();

function runNext() {
var next = that.taskList.shift();

if ( next ) {
stepCounter++;
console.log( stepCounter + '. ' + next[ 1 ] );
that.tasks[ next[ 0 ] ].call( that, runNext );
} else {
if ( callback ) {
callback();
}
}
}
},

/**
* Holds individual methods for each task executed by the builder.
*
* All methods here MUST be called in the builder context by using
* `builder.tasks.someMethod.call( builder, callback )`.
*/
tasks: {
/**
* Deletes the `target` and `tmp` directories.
*
* @param {Function} callback Function to be called when the task is done.
* @returns {Object} The callback returned valued.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo. Should be "value".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

*/
cleanup: function( callback ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as below - cleanUp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

var del = require( 'del' );
del.sync( this.target );
del.sync( this.tmp );

return callback();
},

/**
* Copy the local source code of CKEditor and its dependencies to the `tmp` directory for processing.
*
* @param {Function} callback Function to be called when the task is done.
* @returns {Object} The callback returned valued.
*/
copyToTmp: function( callback ) {
var ncp = require( 'ncp' ).ncp;
var path = require( 'path' );
var fs = require( 'fs' );
var tmp = this.tmp;

var deps = require( '../../../package.json' ).dependencies;

var toCopy = Object.keys( deps ).filter( function( name ) {
return name.indexOf( 'ckeditor5-' ) === 0;
} );

if ( !fs.existsSync( tmp ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the tmp/ directory is removed at the end of the task we risk removing someone's directory (with all his uncommitted work :D). Even if this is only a theoretical situation, it's a good practice IMO to abort if a directory which should not exist already exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one... fixed with f4074f0.

fs.mkdirSync( tmp );
}

function copy() {
var module = toCopy.shift();

if ( !module ) {
return callback();
}

var dest = path.join( tmp + '/', module );

if ( !fs.existsSync( dest ) ) {
fs.mkdirSync( dest );
}

// Copy the "src" directory only.
ncp( path.join( 'node_modules', module, 'src' ), path.join( dest, 'src' ), {
dereference: true
}, function( err ) {
if ( err ) {
throw( err );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw doesn't need ().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

}

copy();
} );
}

copy();
},

/**
* Removes the `CKEDITOR` namespace from AMD calls in the `tmp` copy of the source code.
*
* @param {Function} callback Function to be called when the task is done.
* @returns {Object} The callback returned valued.
*/
removeAmdNamespace: function( callback ) {
var replace = require( 'replace' );

replace( {
regex: /^\s*CKEDITOR\.(define|require)/mg,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to do this? Can't the minified code be using define and require through the CKEDITOR namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary for r.js to find dependencies. Actually, to find modules and theirs deps.

replacement: '$1',
paths: [ this.tmp ],
recursive: true,
silent: true
} );

callback();
},

/**
* Creates the optimized release version of `ckeditor.js` in the `target` directory out of the `tmp` copy of the
* source code.
*
* @param {Function} callback Function to be called when the task is done.
* @returns {Object} The callback returned valued.
*/
optimize: function( callback ) {
var requirejs = require( 'requirejs' );

var config = {
out: this.target + '/ckeditor.js',

baseUrl: this.tmp + '/ckeditor5-core/src/',
paths: {
'ckeditor': '../../../ckeditor',
'ckeditor-dev': '../../../src/ckeditor-dev',
'ckeditor-core': 'ckeditor'
},

include: [ 'ckeditor' ],
stubModules: [ 'ckeditor-dev' ],

// optimize: 'none',
optimize: 'uglify2',
preserveLicenseComments: false,
wrap: {
startFile: [ 'src/build/start.frag', require.resolve( 'almond' ) ],
endFile: 'src/build/end.frag'
}
};

requirejs.optimize( config, callback );
},

/**
* Deletes `tmp` directory.
*
* @param {Function} callback Function to be called when the task is done.
* @returns {Object} The callback returned valued.
*/
cleanupTmp: function( callback ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be cleanUpTmp. Cleanup is a noun.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 10246ac.

var del = require( 'del' );
del.sync( this.tmp );

return callback();
}
}
};
1 change: 1 addition & 0 deletions gruntfile.js
Expand Up @@ -9,6 +9,7 @@ module.exports = function( grunt ) {
// Files that will be ignored by the "jscs" and "jshint" tasks.
var ignoreFiles = [
// Automatically loaded from .gitignore. Add more if necessary.
'lib/**'
];

// Basic configuration which will be overloaded by the tasks.
Expand Down