1# Catapult Style guide 2 3## Base style guide 4 5Unless stated below, we follow the conventions listed in the [Chromium style 6guide](https://www.chromium.org/developers/coding-style) and [Google JavaScript 7style guide](http://google.github.io/styleguide/javascriptguide.xml). 8 9## Files 10File names `should_look_like_this.html`. 11 12Keep to one concept per file, always. In practice, this usually means one 13component or class per file, but can lead to multiple if they’re small and 14closely related. If you can, group utility functions into a static class to 15clarify their relationship, e.g. `base/statistics.html`. 16 17``` 18<!-- tracing/model/point.html --> 19<script> 20‘use strict’; 21 22tr.exportTo(‘tr.model’, function() { 23 function Point() {} 24 25 return { 26 Point: Point 27 }; 28}); 29</script> 30``` 31 32The exception to this rule is when there are multiple small, related classes or 33methods. In this case, a file may export multiple symbols: 34 35``` 36<!-- tracing/base/dom_helpers.html --> 37<script> 38‘use strict’; 39 40tr.exportTo(‘tr.ui.b’, function() { 41 function createSpan() { // … } 42 function createDiv() { // … } 43 function isElementAttached(element) { // … } 44 45 return { 46 createSpan: createSpan, 47 createDiv: createDiv, 48 isElementAttached: isElementAttached 49 }; 50}); 51</script> 52``` 53 54Any tests for a file should be in a file with the same name as the 55implementation file, but with a trailing `_test`. 56 57``` 58touch tracing/model/access_point.html 59touch tracing/model/access_point_test.html 60``` 61## Namespacing and element names 62 63All symbols that exist in the global namespace should be exported using the 64`exportTo` method. 65 66Exported package names show the file’s location relative to the root `tracing/` 67directory. These package names are abbreviated, usually with a 1 or 2 letter 68abbreviation - just enough to resolve naming conflicts. All files in the same 69directory should share the same package. 70 71``` 72<!-- tracing/base/units/generic_table.html → 73tr.exportTo(‘tr.b.u’, function() { 74 // ... 75}); 76``` 77 78Polymer element names should use the convention 79`hyphenated-package-name-element-name`. 80 81``` 82<!-- tracing/ui/analysis/counter_sample_sub_view.html --> 83<polymer-element name='tr-ui-a-counter-sample-sub-view'> 84 ... 85</polymer-element> 86``` 87 88## Classes and objects 89 90Classes should expose public fields only if those fields represent a part of the 91class’s public interface. 92 93All fields should be initialized in the constructor. Fields with no reasonable 94default value should be initialized to undefined. 95 96Do not set defaults via the prototype chain. 97 98``` 99function Line() { 100 // Good 101 this.yIntercept_ = undefined; 102} 103 104Line.prototype = { 105 // Bad 106 xIntercept_: undefined, 107 108 109 set slope(slope) { 110 // Bad: this.slope_ wasn’t initialized in the constructor. 111 this.slope_ = slope; 112 }, 113 114 set yIntercept() { 115 // Good 116 return this.yIntercept_; 117 } 118}; 119``` 120 121## Polymer elements 122The `<script>` block for the Polymer element can go either inside or outside of 123the element’s definition. Generally, the block outside is placed outside when 124the script is sufficiently complex that having 2 fewer spaces of indentation 125would make it more readable. 126 127``` 128<polymer-element name="tr-bar"> 129 <template><div></div></template> 130 <script> 131 // Can go here... 132 </script> 133</polymer-element> 134 135<script> 136'use strict'; 137(function(){ // Use this if you need to define constants scoped to that element. 138Polymer('tr-bar’, { 139 // ... or here. 140}); 141})(); 142</script> 143``` 144 145Style sheets should be inline rather than in external .css files. 146 147``` 148<polymer-element name="tr-bar"> 149 <style> 150 #content { 151 display: flex; 152 } 153 </style> 154 <template><div id=”content”></div></template> 155</polymer-element> 156``` 157 158## `undefined` and `null` 159Prefer use of `undefined` over `null`. 160 161``` 162function Line() { 163 // Good 164 this.yIntercept_ = undefined; 165 // Bad 166 this.slope = null; 167} 168``` 169 170## Tests 171UI element tests that make sure that an element is instantiable should have 172names that start with “`instantiate`”. These tests should, as a general rule, 173should not make assertions. 174 175## ES6 features 176 177**Use of all ES6 features is currently prohibited.** However, we're currently working to allow them. 178 179| Feature | Status | 180|---------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------| 181| [Arrows](https://github.com/lukehoban/es6features#arrows) | [Testing in progress](https://github.com/catapult-project/catapult/issues/2165) | 182| [Classes](https://github.com/lukehoban/es6features#classes) | To be discussed | 183| [Enhanced object literals](https://github.com/lukehoban/es6features#enhanced-object-literals) | To be discussed | 184| [Template strings](https://github.com/lukehoban/es6features#template-strings) | To be discussed | 185| [Destructuring](https://github.com/lukehoban/es6features#destructuring) | To be discussed | 186| [Default, rest, and spread](https://github.com/lukehoban/es6features#default--rest--spread) | To be discussed | 187| [`let` and `const`](https://github.com/lukehoban/es6features#let--const) | To be discussed | 188| [Iterators and `for...of`](https://github.com/lukehoban/es6features#iterators--forof) | To be discussed | 189| [Generators](https://github.com/lukehoban/es6features#generators) | To be discussed | 190| [Unicode](https://github.com/lukehoban/es6features#unicode) | To be discussed | 191| [Modules](https://github.com/lukehoban/es6features#modules) | To be discussed | 192| [Module loaders](https://github.com/lukehoban/es6features#module-loaders) | To be discussed | 193| [`Map`, `Set`, `Weakmap`, and `Weakset`](https://github.com/lukehoban/es6features#map--set--weakmap--weakset) | To be discussed | 194| [Proxies](https://github.com/lukehoban/es6features#proxies) | To be discussed | 195| [Symbols](https://github.com/lukehoban/es6features#symbols) | To be discussed | 196| [Subclassable Built-ins](https://github.com/lukehoban/es6features#subclassable-built-ins) | To be discussed | 197| [Promises](https://github.com/lukehoban/es6features#promises) | To be discussed | 198| [`Math`, `Number`, `String`, `Array`, and `Object` APIs](https://github.com/lukehoban/es6features#math--number--string--array--object-apis) | To be discussed | 199| [Binary and octal literals](https://github.com/lukehoban/es6features#binary-and-octal-literals) | To be discussed | 200| [Reflect API](https://github.com/lukehoban/es6features#reflect-api) | To be discussed | 201| [Tail calls](https://github.com/lukehoban/es6features#tail-calls) | To be discussed | 202 203### Possible feature statuses 204 - **Approved**: this feature is approved for general use. 205 - **Testing in progress**: there's agreement that we should use this feature, but we still need to make sure that it's safe. "Testing in progress" statuses should link to a Catapult bug thread tracking the testing. 206 - **Discussion in progress**: there's not yet agreement that we should use this feature. "Discussion in progress" statuses should link to a Catapult bug thread about whether the feature should be used. 207 - **To be discussed**: this feature hasn't been discussed yet. 208 209Use of an ES6 features shouldn't be considered until that feature is supported in both Chrome stable and [our current version of D8](/third_party/vinn/third_party/v8/README.chromium). 210 211If you see that Catapult’s version of D8 is behind Chrome stable's, use 212[this script](/third_party/vinn/bin/update_v8) to update it. 213 214## Workarounds have bugs for removal: Avoid defensive programming 215 216We should never silently eat an unexpected condition. When such a condition 217occur we should ensure to output the clearest possible warning or a catastrophic 218error if progress cannot continue. If fixing the problem is hard and a simple 219patch would allow someone to keep working on a feature, then it is OK to submit 220this patch at the express condition that: 221 222 1. An issue is created to track the problem. 223 2. The defensive patch is wrapped in a `// TODO` linking to that issue. 224 3. The todo and defensive patch are removed after the problem is fixed. 225 226## Issues 227 228Issues should either: 229 230 * Not have a BUG= tag 231 * Have a BUG=catapult:#123 bug referring to issue 123 in our github tracker. 232 * Have a BUG=chromium:456 bug referring to issue 456 in the chromium tracker. 233