Code style guide for CKEditor
This document is based on Idiomatic.JS. However, it contains many clarifications and rules specific for projects maintained by CKSource like CKEditor.
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
- Whitespace
- Beautiful Syntax
- Type Checking (Courtesy jQuery Core Style Guidelines)
- Conditional Evaluation
- Practical Style
- Naming
- Misc
- Native & Host Objects
- Comments
- One Language Code
- Always use tabs. Never use spaces for indentation (for both - code and comments).
- Tab is 4 spaces.
- Do not leave trailing spaces (note: empty lines should not contain any spaces).
- Always use LF line endings. Never use CRLF.
- 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 Sublime users:
{
// Defaults:
"tab_size": 4,
"translate_tabs_to_spaces": false,
// Additional settings:
"default_line_ending": "unix",
"trim_trailing_white_space_on_save": true
}
-
A. Parens, Braces, Linebreaks
// 2.A.1.1 // Use whitespace to promote readability 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 }
B. Assignments, Declarations, Functions (Named, Expression, Constructor)
// 2.B.1.1 // Variables var foo = 'bar', num = 1, undef; // Literal notations: var array = [], object = {}; // 2.B.1.2 // Using only one `var` per scope (function) promotes readability // and keeps your declaration list free of clutter (also saves a few keystrokes). // Comment on this. var foo = '', bar = '', // Comment on this. quux; // 2.B.1.3 // Try to group `var` statements. Preferably in the beginning // of their respective scope (function), but also don't go crazy with this. // Avoid declaring new variable every few lines of code. // In case of long functions make sure to declare all common variables // (like loop counters, `el`, `range`, etc.) in the beginning of scope. // Same goes for const and let from ECMAScript 6. // Bad. function foo() { // 2 statements var bar = ''; // 2 statements var foo = ''; // 50 statements var bom = ''; // some statements } // Good function foo() { var bar = '', foo = ''; // 50 statements var bom = ''; // Assuming that bom isn't needed before, there's no sense // in grouping it with the first group. // some statements } // 2.B.2.1 // Named Function Declaration function foo( arg1, argN ) { } // Usage. foo( arg1, argN ); // 2.B.2.2 // Named Function Declaration. function square( number ) { return number * number; } // Usage. square( 10 ); // Really contrived continuation passing style. function square( number, callback ) { callback( number * number ); } square( 10, function( square ) { // callback statements } ); // 2.B.2.3 // Function Expression var square = function( number ) { // Return something valuable and relevant. return number * number; }; // 2.B.2.4 // Constructor Declaration function FooBar( options ) { this.options = options; } // Usage. var fooBar = new FooBar( { a: 'alpha' } ); fooBar.options; // -> { a: 'alpha' }
C. Quotes
Always use single quotes in JavaScript, but double quotes in HTML.
-
A. Actual Types
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:
elem.nodeType == 1null:
variable === nullnull or undefined:
variable == nullundefined:
Global Variables: typeof variable == 'undefined' // Or: variable === undefined Properties: object.prop === undefined 'prop' in object !object.prop // If you know what you do.B. Coerced Types
Use
+,!!and+ ''for type coercion whenever possible. However, your code should not look like assembly or CoffeeScript so do not overuse tricks.Here are some common cases along with coercions:
// 3.B.2.1 var number = 1, string = '1', bool = false; number; // 1 number + ''; // '1' string; // '1' +string; // 1 +string++; // 1 string; // 2 bool; // false +bool; // 0 bool + ''; // 'false'
// 3.B.2.2 var number = 1, string = '1', bool = true; string === number; // false string === number + ''; // true +string === number; // true bool === number; // false +bool === number; // true bool === string; // false bool === !!string; // true
// 3.B.2.3 var array = [ 'a', 'b', 'c' ]; !!~array.indexOf( 'a' ); // true !!~array.indexOf( 'b' ); // true !!~array.indexOf( 'c' ); // true !!~array.indexOf( 'd' ); // false // Note that the above should be considered "unnecessarily clever" // Prefer the obvious approach of comparing the returned value of // indexOf, like: if ( array.indexOf( 'a' ) >= 0 ) { // ... }
// 3.B.2.4 var num = 2.5; parseInt( num, 10 ); // Is the same as... ~~num; num >> 0; num >>> 0; // All result in 2. // Keep in mind however, that negative numbers will be treated differently... var neg = -2.5; parseInt( neg, 10 ); // Is the same as... ~~neg; neg >> 0; // All result in -2. // However... neg >>> 0; // Will result in 4294967294.
-
// 4.1.1 // When only evaluating that an array has length, // instead of this: if ( array.length > 0 ) ... // ...evaluate truthiness, like this: if ( array.length ) ... // 4.1.2 // When only evaluating that an array is empty, // instead of this: if ( array.length === 0 ) ... // ...evaluate truthiness, like this: if ( !array.length ) ... // 4.1.3 // When only evaluating that a string is not empty, // instead of this: if ( string !== '' ) ... // ...evaluate truthiness, like this: if ( string ) ... // 4.1.4 // When only evaluating that a string _is_ empty, // instead of this: if ( string === '' ) ... // ...evaluate falsyness, like this: if ( !string ) ... // 4.1.5 // 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 ) ... // 4.1.6 // 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 ) ... // 4.1.7 // When only evaluating a ref that might be null or undefined, // but NOT false, '' or 0, instead of this: if ( foo === null || foo === undefined ) ... // ...take advantage of == type coercion, like this: if ( foo == null ) ... // Remember, using == will match a `null` to BOTH `null` and `undefined` // but not `false`, '' or 0 null == undefined
ALWAYS evaluate for the best, most accurate result - the above is a guideline, not a dogma.
// 4.2.1 // Type coercion and evaluation notes // Use === only if you have to - for example when you're dealing // with user input or you need to distinguish between null and undefined. // You can safely use == in most cases, especially // when function's input is documented. // It will be caller's fault if he passes wrong arguments. // 4.2.2 // Booleans, Truthies & Falsies // Booleans: true, false // Truthy: "foo", 1 // Falsy: "", 0, null, undefined, NaN, void 0
-
// 5.1.1 // A Practical Module ( 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 ) { // Set the value of `data` and return it. return ( data = value ); } }; } )(); // Other things might happen here. // Expose our module to the global object. CKEDITOR.module = Module; } )();
// 5.2.1 // A Practical Constructor ( function() { function Ctor( foo ) { this.foo = foo; return this; } CKEDITOR.tools.extend( Ctor.prototype, { getFoo: function() { return this.foo; }; setFoo: function( val ) { return ( this.foo = val ); }; } ); // Expose our constructor to the global object. CKEDITOR.ctor = Ctor; } )();
-
A. You are not a human code compiler/compressor, so don't try to be one.
The following code is an example of egregious naming:
// 6.A.1.1 // Example of code with poor names 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):
// 6.A.2.1 // Example of code with improved names function query( selector ) { return document.querySelectorAll( selector ); } var id = 0, elements = [], matches = query( '#foo' ), length = matches.length; for ( ; id < length; id++ ) elements.push( matches[ id ] );
A few additional naming pointers:
// 6.A.3.1 // Naming strings `dog` is a string // 6.A.3.2 // Naming arrays `dogs` is an array of `dog` strings // 6.A.3.3 // Naming functions, objects, instances, etc camelCase; function and var declarations // 6.A.3.4 // Naming constructors, prototypes, etc. // constructor function PascalCase; // but camel case when making public: CKEDITOR.sth.camelCase; // 6.A.3.6 // From the Google Closure Library Style Guide functionNamesLikeThis; variableNamesLikeThis; ConstructorNamesLikeThis; EnumNamesLikeThis; methodNamesLikeThis; SYMBOLIC_CONSTANTS_LIKE_THIS;
B. Faces of
thisKeep
this"local" to the class. This means – do not bind e.g. a function inselection.jsfile to editor instance and do not create a function which should be ran in editor context. This way you'll avoid confusion whatthisrefers to.// 6.B.1 function Selection() { var editor = this.editor; // Bad: editor.on( 'setData', someCallback, editor ); // Still bad: editor.on( 'setData', CKEDITOR.tools.bind( someCallback, editor ) ); // Good: editor.on( 'setData', someCallback, this ); // Good: this.getEditor().on( 'setData', function( evt ) { evt.editor.doSomething(); } ); // Good too: var that = this; editor.on( 'setData', function( evt ) { editor.setCurrentSelection( that ); } ); } function someCallback() { // Should refers to Selection, not Editor. this; }
C. Use
thisArgSeveral prototype methods of ES 5.1 built-ins come with a special
thisArgsignature, which should be used whenever possible. CKEditor follows this practice too, so for exampleeventTarget.onalso accepts context as an argument.// 6.C.1 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'
thisArgcan be used withArray.prototype.every,Array.prototype.forEach,Array.prototype.some,Array.prototype.map,Array.prototype.filter. -
A. Early returns promote code readability with negligible performance difference
// 7.A.1.1 // Bad: function returnLate( foo ) { var ret; if ( foo ) { ret = 'foo'; } else { ret = 'quux'; } return ret; } // Good: function returnEarly( foo ) { if ( foo ) { return 'foo'; } return 'quux'; } // Bad: function doSthComplex( foo, bar ) { if ( foo ) { // do something if ( bar ) { // do something } else { // do something else } } } // Good: function doSthComplex( foo, bar ) { if ( !foo ) return; // do something if ( !bar ) { // do something else return; } // do something }
B. (CKEditor only) Objects minify poorly, closures very well
Avoid excessing usage of objects, when functional programming may be used as well. Properties and method names unlike private variables and function names cannot be shortened by minifier.
You can play with online Google Closure Compiler to test different approaches. Make sure to check the gzipped size which matters the most.
-
The basic principle here is:
To reinforce this concept, please watch the following presentation:
<iframe src="http://blip.tv/play/g_Mngr6LegI.html" width="480" height="346" frameborder="0" allowfullscreen></iframe>http://blip.tv/jsconf/jsconf2011-andrew-dupont-everything-is-permitted-extending-built-ins-5211542
But 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.
-
Check the JSDuck documentation guide.
-
You can use JSDuck documentation style for private functions, but then use single line comments.
-
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.
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.
