Changes between Version 15 and Version 16 of WebInspectorCodingStyleGuide


Ignore:
Timestamp:
Aug 24, 2015 10:03:29 AM (9 years ago)
Author:
BJ Burg
Comment:

Update style guide

Legend:

Unmodified
Added
Removed
Modified

  • WebInspectorCodingStyleGuide

    v15 v16  
    88* Indent with 4 spaces
    99* The opening bracket `'{'` after a named, non-inlined function goes on the next line. Anywhere else, the opening bracket `'{'` stays on the same line.
    10 * Style for object literals is: `{key1: value1, key2: value2}`.
     10* Style for object literals is: `{key1: value1, key2: value2}`. When key and variable names coincide, use the syntax {expression} rather than {expression: expression}.
    1111* Add new lines before and after different tasks performed in the same function.
    1212* Else-blocks should share a line with leading } or }).
    1313* Long promise chains should place `.then()` blocks on a new line.
    1414* Calling a constructor with no arguments should have no parenthesis `'()'`. eg. `var map = new Map;`
    15 * Inline anonymous functions, especially if they don't need a bound `this`-object (using `.bind()`). Example:
    16 {{{
    17     this.requestRootDOMNode(function(rootNode) {
    18         ...
    19     });
    20 }}}
     15* Put anonymous functions inline if they are not used as a subroutine.
     16* Use arrow functions when possible, unless it makes code less readable. See below for examples.
    2117
    2218== Naming things
    … …  
    2925== API preferences
    3026
    31 * Consider using [http://people.mozilla.org/~jorendorff/es6-draft.html#sec-keyed-collection `Map` and `Set` collections] instead of plain objects.
    32 * Consider using `hsla()' over hex or RGB colors in CSS.
    33 * Use `for..of` syntax when performing small actions on each element. Use `forEach` when the function body is longer. Use a classical for loop when doing index math.
     27* Use [http://people.mozilla.org/~jorendorff/es6-draft.html#sec-keyed-collection `Map` and `Set` collections] instead of plain objects if the key values are unknown or not monotonic (i.e., frequently added then removed).
     28* Use `hsla()' over hex or RGB colors in CSS.
     29* Use `for..of` syntax when performing actions on each element. Use `forEach` when chaining methods in a functional style. Use a classical for loop when doing index math.
    3430* When using `forEach` or `map`, supply the `this`-object as the optional second parameter rather than binding it.
    35 * In promise chains, assign `const instance = this;' and capture a reference to `instance` to avoid noisy calls to Function.prototype.bind().
     31* In promise chains, use arrow functions for lexical `this`, rather than assigning `const instance = this;' or `.bind`ing every function's `this`-argument.
     32* Use destructuring assignment when digging values out of a JSON object or "args" object.
     33* Use default parameters when it makes sense.
     34* Use `super` to make calls to base class (possibly overridden) methods.
    3635
    3736== Layering and abstractions
    … …  
    4039* Avoid accessing *View classes from *Manager or *Object classes. This is a layering violation that prevents writing tests for models.
    4140* Avoid storing DOM elements in *Manager or *Object classes. (see above.)
    42 * Avoid using Inspector TypeBuilders outside of InspectorAgent classes. We want to isolate protocol considerations from other functionality in JavaScriptCore and WebCore.
     41* In the backend, avoid using Inspector TypeBuilders outside of InspectorAgent classes. We want to isolate protocol considerations from other functionality in JavaScriptCore and WebCore.
    4342
    4443== Understanding and Using Promises
    … …  
    48471. Become __fulfilled__ by a **value**
    49482. Become __rejected__ with an **Error instance** or by throwing an exception
     49
     50A promise that is eiher fulfilled or rejected is said to be __settled__. A promise that has not settled is said to be __pending__.
    5051
    5152And, if you have a correctly implemented `then()` function, then fulfillment and rejection will compose just like their synchronous counterparts, with fulfillments flowing up a compositional chain, but being interrupted at any time by a rejection that is only handled by someone who declares they are ready to handle it.
    … …  
    5960* Use `Promise.all()` with `map()` to process an array of asynchronous work in parallel. Use `Promise.all()` with `reduce()` to sequence an array asynchronous work.
    6061* If a result may be a promise or an actual value, wrap the value in a promise, e.g., `Promise.resolve(val)`
    61 * Use `.catch()` at the end of a chain to perform error handling. Most promise chains should have a catch block to avoid dropping errors.
     62* Use `.catch()` at the end of a chain to perform error handling. '''Most promise chains should have a catch block to avoid dropping errors'''.
    6263* To reject a promise, throw an `Error` instance or call the `reject` callback with an `Error` instance.
    6364* A `.catch()` block is considered resolved if it does not re-throw an `Error` instance. Re-throw if you want to log an error message and allow other parts of a chain (i.e, an API client) to handle an error condition.
    6465* Don't directly pass a promise's `resolve` function to `Object.addEventListener`, as it will leak the promise if the event never fires. Instead, use a single-fire `WebInspector.EventListener` object defined outside of the promise chain and connect it inside a `.then()` body. Inside the `.catch` block, disconnect the `EventListener` if necessary.
    6566* For APIs that return promises, document what the fulfilled value will be, if any. Example: `createSession() // --> (sessionId)`
     67
     68== Arrow Functions
     69
     70Arrow functions simplify a common use of anonymous functions by providing a shorter syntax, lexical binding of `this` and `arguments`, and implicit return. While this new syntax enables new levels of terse code, we must take care to keep our code readable.
     71
     72=== Implicit return
     73
     74Arrow functions with one expression have an implicit return. All of these are equivalent (modulo `this` binding, arguments, constructor usage, etc.):
     75
     76{{{
     771   let foo = val => val;
     782   let foo = (val) => val
     793   let foo = (val) => val;
     804   let foo = (val) => { return value++; }
     815   let foo = function doStuff(val) { return value++; }
     82}}}
     83
     84Never use option (1), because it is a special case that only applies when the function has one argument, reducing predictability.
     85
     86In cases where the return value is used and the expression is a constant ("foo"), a variable (foo), or a member (this.foo), use option (2) (with braces if the arrow function is inlined).
     87
     88In cases where the expression computes a value (a + 42) or performs a side effect (++a), prefer option (4).
     89In some sense, curly braces are a signpost to the effect of "careful, we do actual work here".
     90
     91GOOD:
     92
     93{{{
     94setTimeout(() => { testRunner.notifyDone(); }, 0)
     95}}}
     96
     97BAD:
     98
     99{{{
     100setTimeout(() => { testRunner.notifyDone() }, 0); // return value not used
     101}}}
     102
     103=== When not to arrow
     104
     105When assigning a function to a subclass prototype (in the old way of setting up classes), always use the normal function syntax, to avoid breaking subclasses who use a different 'this' binding. Note that arrow functions are NOT acceptable for assigning functions to singleton objects like WebInspector, since the captured lexical `this` is typically the global object.
     106
     107GOOD:
     108
     109{{{
     110Base.prototype.compute = function(a, b, c) { ... }
     111Foo.prototype.compute = function(a, b, c) { Base.prototype.compute.call(this, a, b, c); }
     112
     113WebInspector.UIString = function(format, args) { ... }
     114}}}
     115
     116BAD:
     117
     118{{{
     119Base.prototype.compute = (a, b, c) => { ... }
     120Foo.prototype.compute = (a, b, c) => { Base.prototype.compute.call(this, a, b, c); }
     121
     122WebInspector.UIString = (format, args) => { ... } // this will be window.
     123}}}
     124
     125Also use the normal function syntax when naming an anonymous function improves readability of the code. In this case, use Function.prototype.bind or assign the arrow function into a local variable first.
     126
     127GOOD:
     128
     129{{{
     130Promise.resolve()
     131    .then(function resolved(value) { ... },
     132        function rejected(value) { ... });
     133}}}
     134
     135BAD:
     136
     137{{{
     138Promise.resolve()
     139    .then((value) => { ... },
     140        (value) => { ... })
     141}}}
    66142
    67143== New class skeleton
    … …  
    75151       super();
    76152       this._propertyName = param;
     153    }
     154
     155    // Static
     156    computeBestWidth(things)
     157    {
     158        ....
     159        return 3.14159;
    77160    }
    78161
    … …  
    115198
    116199}}}
    117 
    118 == Old class skeleton
    119 
    120 Some existing Inspector object classes have not been converted to ES6 classes. The should conform to the following format:
    121 
    122 {{{
    123 WebInspector.NewObjectType = function()
    124 {
    125     WebInspector.Object.call(this);
    126 
    127     this._propertyName = ...;
    128 }
    129 
    130 WebInspector.NewObjectType.Event = {
    131     PropertyWasChanged: "new-object-type-property-was-changed"
    132 };
    133 
    134 WebInspector.NewObjectType.prototype = {
    135     constructor: WebInspector.NewObjectType,
    136     __proto__: WebInspector.Object.prototype,
    137 
    138     // Public
    139 
    140     get propertyName()
    141     {
    142         return this._propertyName;
    143     },
    144 
    145     set propertyName(value)
    146     {
    147         this._propertyName = value;
    148         this.dispatchEventToListeners(WebInspector.NewObjectType.Event.PropertyWasChanged);
    149     },
    150 
    151     publicMethod: function()
    152     {
    153         /* public methods called outside the class */
    154     }
    155 
    156     // Protected
    157 
    158     handleEvent: function(event)
    159     {
    160         /* delegate methods, event handlers, and overrides. */
    161     },
    162 
    163     // Private
    164 
    165     _privateMethod: function()
    166     {
    167         /* private methods are underscore prefixed */
    168     }
    169 };
    170 }}}