Code style guide for CKEditor 4
Table of contents
This document is based on Idiomatic.JS. However, it contains many clarifications and rules specific for projects maintained by CKSource like CKEditor 4.
All code in any code-base should look like a single person typed it, no matter how many people contributed.
Unknown author
Rebecca Murphey
Part of being a good steward to a successful project is realizing that writing code for yourself is a Bad Idea™. If thousands of people are using your code, then write your code for maximum clarity, not your personal preference of how to get clever within the spec.
Idan Gazit
- Always use tabs. Never use spaces for indentation (for both - code and comments).
- Do not leave trailing spaces (note: empty lines should not contain any spaces).
- Always use LF line endings. Never use CRLF.
- Each file end up with an empty line character EOL.
- If your editor supports it, always work with the "show invisibles" setting turned on. The benefits of this practice are:
- Enforced consistency.
- Eliminating end of line whitespace.
- Eliminating blank line whitespace.
- Commits and diffs that are easier to read.
Related settings for Editor Config:
root = true
[*]
end_of_line = lf
insert_final_newline = true
indent_style = tab
trim_trailing_whitespace = true
if ( condition ) {
// statements
}
if ( true ) {
// statements
} else {
// statements
}
if ( true ) {
// statements
}
// Comments regarding what "else" means and does.
else {
// statements
}
while ( condition ) {
// statements
}
for ( var i = 0; i < 100; i++ ) {
// statements
}
try {
// statements
} catch ( e ) {
// statements
}// Bad
if ( true ) // statement
// Good
if ( true ) {
// statement
}
// Bad
if ( false ) // statement
else // statement
// Good
if ( false ) {
// statement
} else {
// statement
}if ( condition1 ||
condition2 ||
condition3 ) {
// statement
}Declaring one variable per line improves debugging experience. Multiple variables declared in the same line won't allow you to place breakpoint at the definition computation. The same goes to object literals.
// Bad
var foo = 'foo', bar = 'bar',
object = {
foo: 'foo', bar = 'bar' // Also bad
};
// Good
var foo = 'foo',
bar = 'bar',
object = {
foo: 'foo',
bar: 'bar'
};Using only one variable per scope (function) promotes readability and keeps your declaration list free of clutter (also saves a few keystrokes). However, if a scope have early returns statements, it will be more readable and performance wise declaring variable later, when definition is needed.
// Bad.
function foo() {
// 2 statements
var bar = '';
// 2 statements
var foo = '';
// 10 statements
var bom = '';
// some statements
}
// Good
function foo() {
var bar = '',
foo = '';
// some statements
}
// Bad
function bar( x ) {
var foo = doSomethingSlow();
if ( x < 2 ) {
return;
}
// some statements
}
// Good
function bar( x ) {
if ( x < 2 ) {
return;
}
var foo = doSomethingSlow();
// some statements
}Prefer function declaration instead of function expression to avoid issues with executing function before it has been defined.
// Good
square( 10 );
function square( number ) {
return number * number;
}
// Bad
square( 10 );
var square = function( number ) {
return number * number;
};If your function should be executed asynchronously or requires additional callback (e.g. filter function) as an function argument, pass it as the last argument in function signature. Prefer Promises for asynchronous code when possible.
// Declaration.
function square( number, callback ) {
callback( number * number );
}
// Usage.
square( 10, function( square ) {
// callback statements
} );Preferably, asynchronous code is written using Promises. It's not always possible when maintaining code, however, prefer Promise based code for new features and everythere where you have control over API structure consumer. Always use CKEDITOR.tools.promise instead of native Promise object for wider browser support.
var promise = new CKEDITOR.tools.promise( function( resolve, reject ) {
getContentFromFileServer( options, function( result ) {
if ( result.status === 200 ) {
resolve( result.data );
} else {
reject( result.error );
}
} );
} );Prefer using CKEDITOR.tools.createClass API instead of native constructor declaration.
// Good
var FooBar = CKEDITOR.tools.createClass( {
$: function( message ) {
this.message = message;
},
proto: {
greeting: function() {
console.log( this.message );
}
}
} );
var fooBar = new FooBar( 'Hello, World!' );
fooBar.greeting();
// 'Hello, World!'
// Bad
function FooBar( message ) {
this.message = message;
}
FooBar.prototype.greeting = function() {
console.log( this.message );
};
var fooBar = new FooBar( 'Hello, World!' );
fooBar.greeting();
// 'Hello, World'Exposing public API should have much higher priority in file structure than private members. It improves API discoverability and hides private members.
// Simple module.
CKEDITOR.module1 = {
pubFoo: function() {
this.privFoo();
},
pubBar: function() {
this.privBar();
},
privFoo: function() {
// ...
},
privBar: function() {
// ...
}
};
// Using module pattern.
var module = ( function() {
// Exposing public API.
return {
foo: foo,
bar: bar
}
function foo() {
bar();
}
// Private helpers.
function bar() {
// ...
}
} )();When extracting helper function, move it right after the first usage inside the nearest scope which won't affect code performance and memory usage. In most cases, you want to keep helper functions on the same scope level as a public one unless they are used somewhere else in the same file. Do not forget about public API rule where main code flow have higher priority than private helpers.
CKEDITOR.module1 = {
doSomething: function() {
// Good
foo();
bar();
// Bad thus the function object will be created every time when doSomething function executed.
function bar() {
// ...
}
}
};
CKEDITOR.module2 = {
doSomething: function() {
// Note that despite this function is used in another module later,
// it has been correctly declared as close as possible to the first usage.
foo();
}
}
// We are moving helper functions lower in correct order to keep main code flow happy.
function foo() {
baz();
quix();
}
function baz() {
// ...
}
function quix() {
// ...
}Note that the same practice applies to objects:
CKEDITOR.module1 = {
foo: function() {
this.bar();
},
bar: function() {
// ...
},
baz: function() {
// ...
}
};Configuration members exposed by CKEDITOR.config object should be moved at the end of the code file. It's adapted practice in CKEditor code base which improves configuration options discoverability.
CKEDITOR.plugins.add( 'myplugin', definition );
// ... some code goes here ...
// ... more lines of code ...
// ... just don't wake up Cthulhu ...
CKEDITOR.config.foo = 'foo';
CKEDITOR.config.bar = 'bar';Always use single quotes in JavaScript, but double quotes in HTML.
String:
typeof variable === 'string'Number:
typeof variable === 'number'Boolean:
typeof variable === 'boolean'Object:
typeof variable === 'object'Array:
// In CKEditor:
CKEDITOR.tools.isArray( arrayLikeObject )
// In other cases:
Array.isArray( arrayLikeObject )
(wherever possible)Node:
// In CKEditor:
elem.type === CKEDITOR.NODE_ELEMENT
// In other cases:
elem.nodeType === Node.ELEMENT_NODEnull:
variable === nullundefined:
variable === undefinedUse coercion with caution. In most cases prefer explicit type casting than using implicit operator coercion. Note that some practices are more common than the others, so use common sense to choose correct casting method.
Boolean coercion leveraged by double !! is widely adopted in CKEditor 4 codebase and it's one of the exception where you should use implicit coercion to its popularity.
Small explanation how double !! works:
- The first
!negates value and parses it into boolean value. - The second
!flips negation.
var string = 'some string value',
object = {
foo: 'foo'
},
number = 0,
nothing = null;
!!string
// true
!!number
// false
!!object
// true
!!nothing
// falseAvoid using unpopular coercion operators due to lacking readability. Most of them have explicit versions, which are much more readable.
Instead of bitwise NOT operator ~ prefer explicit aproach by comparing returned value.
var array = [ 'a', 'b', 'c' ];
// Bad
if ( ~array.indexOf( 'a' ) ) {
// ...
}
// Good
if ( array.indexOf( 'a' ) >= 0 ) {
// ...
}Instead of double bitwise NOT operator ~~ resulting in numerical substitution, use Math.floor or parseInt methods.
var num = 2.5;
// Bad
~~num;
// Good
parseInt( num, 10 );
// Good
Math.floor( num );Do not use empty string concatenation to convert primitive values into strings.
// Bad
number + '';
// Good
String( number );
var boolean = true;
// Bad
boolean + '';
// Good
String( boolean );Do not use numeric operators to convert primitive values into numbers.
var string = '5';
// Bad
+string;
// Good
Number( string );
var boolean = true;
// Bad
+boolean;
// Good
Number( boolean );Do not use double == to match both undefined and null values. Prefer explicit comparison instead.
// Bad
if ( value == null ) ...
// Good
if ( foo === null || foo === undefined ) ...Preserve extra caution using types coercion, where it doesn't make logical sense.
Number( [] );
// 0
[] == false
// true// When only evaluating that an array has length,
// instead of this:
if ( array.length > 0 ) ...
// ...evaluate truthiness, like this:
if ( array.length ) ...
// When only evaluating that an array is empty,
// instead of this:
if ( array.length === 0 ) ...
// ...evaluate truthiness, like this:
if ( !array.length ) ...// When only evaluating that a string is not empty,
// instead of this:
if ( string !== '' ) ...
// ...evaluate truthiness, like this:
if ( string ) ...
// When only evaluating that a string _is_ empty,
// instead of this:
if ( string === '' ) ...
// ...evaluate falsyness, like this:
if ( !string ) ...// When only evaluating that a reference is true, instead of this:
if ( foo === true ) ...
// ...evaluate like you mean it, take advantage of built in capabilities:
if ( foo ) ...
// When evaluating that a reference is false, instead of this:
if ( foo === false ) ...
// ...use negation to coerce a true evaluation
if ( !foo ) ...
// ...Be careful, this will also match: 0, '', null, undefined, NaN. If you MUST test for a boolean false, then use:
if ( foo === false ) ...In most cases Boolean evaluation for falsy reference is enough, but if you really need to check if value is not undefined or null (like when false is an expected value) preferexplicit comparison. See Null or undefined coercion.
// When evaluating that reference is null or undefined, prefer explicit comparison:
if ( foo === null || foo === undefined ) ...
// instead of == type coercion:
if ( foo == null ) ...Use loose equality operator to simplify your code when you want to leverage type coercion. Remember that strict equality comparator === is checking if both the type and the value you are comparing are the same. In a constract, loose equality operator == will try to do type coercion which may be useful in some cases when comparing values where types are less revelant than values.
false === 'false'
// false
false == 'false'
// true
// 1 === '1'
// false
// 1 == '1'
// true// Booleans:
true, false
// Truthy:
"foo", 1
// Falsy:
"", 0, null, undefined, NaN, void 0( function() {
var Module = ( function() {
var data = "secret";
return {
// This is some boolean property.
bool: true,
// Some string value.
string: 'a string',
// An array property.
array: [ 1, 2, 3, 4 ],
// An object property.
object: {
lang: 'en-Us'
},
getData: function() {
// Get the current value of `data`.
return data;
},
setData: function( value ) {
this.value = value;
}
};
} )();
// Other things might happen here.
// Expose our module to the global object.
CKEDITOR.module = Module;
} )();( function() {
var FooBar = CKEDITOR.tools.createClass( {
$: function( foo ) {
this.foo = foo;
},
proto: {
getFoo: function() {
return this.foo;
},
setFoo: function( val ) {
this.foo = val;
}
}
} );
// Expose our constructor to the global object.
CKEDITOR.fooBar = FooBar;
} )();( function() {
var FooBar = CKEDITOR.tools.createClass( {
// class definition
} );
CKEDITOR.tools.extend( FooBar.prototype, {
getFoo: function() {
return this.foo;
},
setFoo: function( val ) {
this.foo = val;
}
} );
// Expose our constructor with extended prototype to the global object.
CKEDITOR.fooBar = FooBar;
} )();You are not a human code compiler/compressor, so don't try to be one.
The following code is an example of egregious naming:
function q(s) {
return document.querySelectorAll(s);
}
var i,a=[],els=q("#foo");
for(i=0;i<els.length;i++){a.push(els[i]);}Here's the same piece of logic, but with kinder, more thoughtful naming (and a readable structure):
function query( selector ) {
return document.querySelectorAll( selector );
}
var elements = [],
matches = query( '#foo' ),
length = matches.length;
for ( var i = 0; i < length; i++ ) {
elements.push( matches[ i ] );
}Methods and functions are almost always verbs/actions in camelCase notation:
// Good
execute();
this.getNextNumber();
// Bad
this.editable();
this.status();The exception for the above are get/set methods that are used very often, where the first parameter is the thing to get/set and the second is the optional value (set with it and get without):
this.data( 'name' ); // get
this.data( 'name', 'John' ); // set
this.attr( 'title' ); // get
this.attr( 'title', 'Flying plane' ); // setProperties and variables are almost always nouns in camelCase notation:
var editor;
this.name;
this.editable;Boolean properties and variables are always prefixed by an auxiliary verb:
this.isDirty;
this.hasChildren;
this.isFake;Properties that require computation, instead of having them as getXXX() and setXXX() accessing methods, can be named as nouns, when are programmatically and conceptually simple:
editor.setStatus( 404 ); // setter
this.status; // getter
editor.setReadOnly( true ); // setter
editor.readOnly; // getterOnly use underscore _ name as an object member with private properties:
var object = {
// Public member.
foo: {},
// Also public member.
bar: {},
// Private member and everything contained inside this object is also private.
_: {}
}Avoid using underscore _ with private variables, methods and functions. Note that all undocumented members are private by default, so there is no need beeing explicit here.
// Bad
var object = {
_foo: {}
};
// Good
var object = {
foo: {}
};
// Also good, but note that explicitivness is only needed for easier distinction between public and private members.
var object = {
/*
* @private
*/
foo: {},
/*
* Some nice description here.
* @property {Object}
*/
bar: {}
};Constants are always nouns in UNDERSCORE_SNAKE_CASE notation:
// Constant variables.
var CONSTANT_VARIABLE = 999;
// Constant enum.
CKEDITOR.sth.CONSTANT_ENUM = 1;If configuration member is a part of core codebase, go with camelCaseNaming notation.
CKEDITOR.config.fooBarBaz = true;If configuration member is a part of plugin functionality, use pluginName_optionName pattern consisting of mixed camelCase plugin and option naming separated with underscore _.
CKEDITOR.config.myPlugin_myProperty = true;Constructors created with CKEDITOR.tools.createClass API are always nouns in PascalCase notation:
var FooBar = CKEDITOR.tools.createClass( {
$: function( message ) {
this.message = message;
},
proto: {
greeting: function() {
console.log( this.message );
}
}
} );Constructor functions are always nouns in PascalCase notation:
function FooBar( message ) {
this.message = message;
}
FooBar.prototype = {
greeting: function() {
console.log( this.message );
}
};But should always be exposed in camelCase notation publicly:
CKEDITOR.fooBar = FooBar;Keep this "local" to the class. This means – do not bind e.g. a function in selection.js file to editor instance and do not create a function which should be ran in editor context. This way you'll avoid confusion what this refers to.
function Selection() {
var editor = this.editor;
// Bad:
this.editor.on( 'setData', someCallback, editor );
// Still bad:
this.editor.on( 'setData', CKEDITOR.tools.bind( someCallback, editor ) );
// Good:
this.editor.on( 'setData', someCallback, this );
// Good:
this.getEditor().on( 'setData', function( evt ) {
evt.editor.doSomething();
} );
}
function someCallback() {
// Should refer to Selection, not Editor.
var editor = this.editor;
// ...
}Several prototype methods of ES 5.1 built-ins come with a special thisArg signature. CKEditor follows this practice too, which should be used whenever possible. For example eventTarget.on also accepts context as an argument.
var obj = {
f: 'foo',
b: 'bar',
q: 'qux'
};
Object.keys( obj ).forEach( function( key ) {
// |this| now refers to `obj`
console.log( this[ key ] );
}, obj ); // <-- the last arg is `thisArg`
// Prints...
// 'foo'
// 'bar'
// 'qux'thisArg can be used with most Object and Array CKEditor 4 helpers.
Beyond the generally well known use cases of call and apply, always prefer CKEDITOR.tools.bind( fn, this ) for creating BoundFunction definitions for later invocation.
function Device( opts ) {
stream.read( opts.path, CKEDITOR.tools.bind( function( value ) {
this.value = value;
}, this ) );
}Only resort to aliasing when no preferable option is available.
function Device( opts ) {
var self = this;
stream.read( opts.path, function( value ) {
self.value = value;
} );
}This section will serve to illustrate ideas and concepts that should not be considered dogma, but instead exists to encourage questioning practices in an attempt to find better ways to do common JavaScript programming tasks.
Early returns promote code readability with negligible performance difference.
// Bad:
function returnLate( range ) {
var docFragment = range.cloneContents();
if ( range.collapsed ) {
return;
}
// ...
}
// Good:
function returnEarly( range ) {
if ( range.collapsed ) {
return;
}
var docFragment = range.cloneContents();
// ...
}The basic principle here is:
Don't extend native objects and everything will be ok.
CKEditor is a widget - an application inside someone's system – so... do not touch anything except CKEDITOR.* namespace.
- Single line above the code that is subject (except for long expressions like conditions).
- Comment is a sentence - start it with capital leter, end with period.
- Use multiline comments only for API documentation.
- Comments should be almost always put above code, not below.
- Check the JSDuck documentation guide.
- You can use JSDuck documentation style for private functions, but then use single line comments.
// Some docs for private functions.
// @param {Object} foo
// @param {object} bar
// returns {Number}- Justify end line comments with tabs:
if ( some != really.complex && // Comment line 1.
condition || with && // Comment line 2.
( multiple == !lines ) ) // Comment line 3.
// Of course it's better to avoid this spaghetti at all.- Fixing a bug should result in additional ticket reference comment written as
(#1345)comment.
// (#1345)
if ( requiresPromisePolyfill() ) {
// ...
}Manual tests reproduction steps should be structured in one, ubiquitous way, to unify test files, so they are easy to read and modify. Manual test file should consist only of a single test suite. Also, avoid putting many test cases in the same file, extract common logic instead using helper methods or template files.
Example of the correct manual test:
@bender-tags: 4.14.0, feature, 999
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles
@bender-include: helpers/utils.js
1. Focus the editor.
1. Type `Hello, World!`.
1. Press `enter`.
**Expected**
* Editor is responsive.
* Typed text has been registered by the editor.
**Unexpected**
* Editor loses focus.
* Typed text is in the reverse order.You can also inline Expected/Unexpected if they are consist of the single sentence line:
@bender-tags: 4.14.0, feature, 999
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles
@bender-include: helpers/utils.js
1. Focus the editor.
1. Type `Hello, World!`.
1. Press `enter`.
**Expected** Editor is responsive.
**Unexpected** Editor is not responsive.And even skip Unexpected if Expected result is obvious enough to guess Unexpected:
@bender-tags: 4.14.0, feature, 999
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles
@bender-include: helpers/utils.js
1. Focus the editor.
1. Type `Hello, World!`.
1. Press `enter`.
**Expected** Editor is responsive.If you need to repeat some test steps for the same test case, make it obvious enough by numbering test steps instead of using 1 everywhere. But note, that such practice should be also used when test is very simple and additional test case won't add too much complexity to the test file:
@bender-tags: 4.14.0, feature, 999
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles
@bender-include: helpers/utils.js
1. Focus the editor.
2. Type `Hello, World!`.
3. Press `enter`.
**Expected** Editor is responsive.
4. Repeat 1-2.
5. Press `tab`.
**Expected** Editor is responsive.But NEVER put more than one test suite into the same manual test file:
@bender-tags: 4.14.0, feature, 999
@bender-ui: collapsed
@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles
@bender-include: helpers/utils.js
### Classic editor
1. Focus the editor.
2. Type `Hello, World!`.
3. Press `enter`.
**Expected** Editor is responsive.
### Inline editor
1. Focus the editor.
**Expected** Toolbar shows up.Extract common logic instead using helper functions for HTML files or use __template__.html feature which will share the same HTML file among manual tests in the same folder. It's better to accept some small duplication than overrun manual testing due to high test complexity.
In test scenarios indentation with tabs is preferred most of the time. However, to force bender to properly render nested lists, use spaces instead.
Principles of Writing Consistent, Idiomatic JavaScript by Rick Waldron and Contributors is licensed under a Creative Commons Attribution 3.0 Unported License.
Based on a work at github.com/rwldrn/idiomatic.js.
