From 045a6133f63b99058cbeff334b060490776fe913 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Thu, 25 May 2017 15:22:54 -0800 Subject: [PATCH 01/18] Copy ember styleguide from dockyard (copied from https://github.com/DockYard/styleguides/blob/master/engineering/ember.md) --- ember.md | 441 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 441 insertions(+) create mode 100644 ember.md diff --git a/ember.md b/ember.md new file mode 100644 index 0000000..2e23d6e --- /dev/null +++ b/ember.md @@ -0,0 +1,441 @@ +# Ember.js Style Guide + +## Table Of Contents + +* [General](#general) +* [Organizing your modules](#organizing-your-modules) +* [Models](#models) +* [Controllers](#controllers) +* [Templates](#templates) +* [Routing](#routing) +* [Ember data](#ember-data) + + +## General + +### Import what you use, do not use globals + +For Ember Data, we should import `DS` from `ember-data`, and then +destructure our desired modules. +For Ember, use destructuring [as Ember's internal organization is +not intuitive and difficult to grok, and we should wait until Ember has been +correctly modularized.](https://github.com/ember-cli/ember-cli-shims/issues/53) + +[Here is the RFC on ES2015 modules](https://github.com/emberjs/rfcs/pull/68). + +Once Ember has officially adopted shims, we will prefer shims over +destructuring. + +```javascript +// Good + +import Ember from 'ember'; +import DS from 'ember-data'; + +const { + computed, + computed: { alias } +} = Ember; + +const { + Model, + attr +} = DS; + +export default Model.extend({ + firstName: attr('string'), + lastName: attr('string'), + + surname: alias('lastName'), + + fullName: computed('firstName', 'lastName', function() { + // Code + }) +}); + +// Bad + +export default DS.Model.extend({ + firstName: DS.attr('string'), + lastName: DS.attr('string'), + + surname: Ember.computed.alias('lastName'), + + fullName: Ember.computed('firstName', 'lastName', { + get() { + // Code + }, + + set() { + // Code + } + }) +}); +``` + +### Don't use Ember's prototype extensions + +Avoid Ember's `Date`, `Function` and `String` prototype extensions. Prefer the +corresponding functions from the `Ember` object. + +Preferably turn the prototype extensions off by updating the +`EmberENV.EXTEND_PROTOTYPES` setting in your `config/environment` file. + +```javascript +module.exports = function(environment) { + var ENV = { + EmberENV: { + EXTEND_PROTOTYPES: { + Date: false, + Function: false, + String: false + } + } +``` + +```javascript +// Good + +export default Model.extend({ + hobbies: w('surfing skateboarding skydiving'), + fullName: computed('firstName', 'lastName', function() { ... }), + didError: on('error', function() { ... }) +}); + +// Bad + +export default Model.extend({ + hobbies: 'surfing skateboarding skydiving'.w(), + fullName: function() { ... }.property('firstName', 'lastName'), + didError: function() { ... }.on('error') +}); +``` + +### Use `get` and `set` + +Calling `someObj.get('prop')` couples your code to the fact that +`someObj` is an Ember Object. It prevents you from passing in a +POJO, which is sometimes preferable in testing. It also yields a more +informative error when called with `null` or `undefined`. + +Although when defining a method in a controller, component, etc. you +can be fairly sure `this` is an Ember Object, for consistency with the +above, we still use `get`/`set`. + +```js +// Good +import Ember from 'ember'; + +const { get, set } = Ember; + +set(this, 'isSelected', true); +get(this, 'isSelected'); + +// Bad + +this.set('isSelected', true); +this.get('isSelected'); +``` + +### Use brace expansion + +This allows much less redundancy and is easier to read. + +Note that **the dependent keys must be together (without space)** for the brace expansion to work. + +```js +// Good +fullName: computed('user.{firstName,lastName}', { + // Code +}) + +// Bad +fullName: computed('user.firstName', 'user.lastName', { + // Code +}) +``` + +## Organizing your modules + +Ordering a module's properties in a predictable manner will make it easier to +scan. + +1. __Plain properties__ + + Start with properties that configure the module's behavior. Examples are + `tagName` and `classNames` on components and `queryParams` on controllers and + routes. Followed by any other simple properties, like default values for properties. + +2. __Single line computed property macros__ + + E.g. `alias`, `sort` and other macros. Start with service injections. If the + module is a model, then `attr` properties should be first, followed by + `belongsTo` and `hasMany`. + +3. __Multi line computed property functions__ + +4. __Lifecycle hooks__ + + The hooks should be chronologically ordered by the order they are invoked in. + +5. __Functions__ + + Public functions first, internal functions after. + +6. __Actions__ + +```js +export default Component.extend({ + // Plain properties + tagName: 'span', + + // Single line CP + post: alias('myPost'), + + // Multiline CP + authorName: computed('author.{firstName,lastName}', function() { + // code + }), + + // Lifecycle hooks + didReceiveAttrs() { + this._super(...arguments); + // code + }, + + // Functions + someFunction() { + // code + }, + + actions: { + someAction() { + // Code + } + } +}); +``` + +### Override init + +Rather than using the object's `init` hook via `on`, override init and +call `_super` with `...arguments`. This allows you to control execution +order. [Don't Don't Override Init](https://dockyard.com/blog/2015/10/19/2015-dont-dont-override-init) + +## Models + +### Organization + +Models should be grouped as follows: + +* Attributes +* Associations +* Computed Properties + +Within each section, the attributes should be ordered alphabetically. + +```js +// Good + +import Ember from 'ember'; +import DS from 'ember-data'; + +const { computed } = Ember; + +const { + Model, + attr, + hasMany +} = DS; + +export default Model.extend({ + // Attributes + firstName: attr('string'), + lastName: attr('string'), + + // Associations + children: hasMany('child'), + + // Computed Properties + fullName: computed('firstName', 'lastName', function() { + // Code + }) +}); + +// Bad + +import Ember from 'ember'; +import DS from 'ember-data'; + +const { computed } = Ember; + +const { + Model, + attr, + hasMany +} = DS; + +export default Model.extend({ + children: hasMany('child'), + firstName: attr('string'), + lastName: attr('string'), + + fullName: computed('firstName', 'lastName', function() { + // Code + }) +}); + +``` + +## Controllers + +### Define query params first + +For consistency and ease of discover, list your query params first in +your controller. These should be listed above default values. + +### Alias your model + +It provides a cleaner code to name your model `user` if it is a user. It +is more maintainable, and will fall in line with future routable +components + +```javascript +export default Controller.extend({ + user: alias('model') +}); +``` + +## Templates + +### Do not use partials + +Always use components. Partials share scope with the parent view, use +components will provide a consistent scope. + +### Don't yield `this` + +Use the hash helper to yield what you need instead. + +```hbs +{{! Good }} +{{yield (hash thing=thing action=(action "action"))}} + +{{! Bad }} +{{yield this}} +``` + +### Use components in `{{#each}}` blocks + +Contents of your each blocks should be a single line, use components +when more than one line is needed. This will allow you to test the +contents in isolation via unit tests, as your loop will likely contain +more complex logic in this case. + +```hbs +{{! Good }} +{{#each posts as |post|}} + {{post-summary post=post}} +{{/each}} + +{{! Bad }} +{{#each posts as |post|}} +
+ +

{{post.title}}

+

{{post.summar}}

+
+{{/each}} +``` + +### Always use the `action` keyword to pass actions. + +Although it's not strictly needed to use the `action` keyword to pass on +actions that have already been passed with the `action` keyword once, +it's recommended to always use the `action` keyword when passing an action +to another component. This will prevent some potential bugs that can happen +and also make it more clear that you are passing an action. + +```hbs +{{! Good }} +{{edit-post post=post deletePost=(action deletePost)}} + +{{! Bad }} +{{edit-post post=post deletePost=deletePost}} +``` + +### Ordering static attributes, dynamic attributes, and action helpers for HTML elements + +Ultimately, we should make it easier for other developers to read templates. +Ordering attributes and then action helpers will provide clarity. + +```hbs +{{! Bad }} + + +``` + +```hbs +{{! Good }} + + +``` + +## Routing + +### Route naming +Dynamic segments should be underscored. This will allow Ember to resolve +promises without extra serialization +work. + +```js +// good + +this.route('foo', { path: ':foo_id' }); + +// bad + +this.route('foo', { path: ':fooId' }); +``` + +[Example with broken +links](https://ember-twiddle.com/0fea52795863b88214cb?numColumns=3). + +### Perform all async actions required for the page to load in route `model` hooks + +The model hooks are async hooks, and will wait for any promises returned +to resolve. An example of this would be models needed to fill a drop +down in a form, you don't want to render this page without the options +in the dropdown. A counter example would be comments on a page. The +comments should be fetched along side the model, but should not block +your page from loading if the required model is there. + +## Ember Data + +### Be explicit with Ember Data attribute types + +Even though Ember Data can be used without using explicit types in +`attr`, always supply an attribute type to ensure the right data +transform is used. + +```javascript +// Good + +export default Model.extend({ + firstName: attr('string'), + jerseyNumber: attr('number') +}); + +// Bad + +export default Model.extend({ + firstName: attr(), + jerseyNumber: attr() +}); +``` From c91e7dc4df5e1dac9d028db9601925266ec14155 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Thu, 25 May 2017 15:23:37 -0800 Subject: [PATCH 02/18] Add Ember guidelines to README --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index c9ad210..3c1de99 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ - [Ruby coding guidelines](ruby.md) [original source](https://github.com/bbatsov/ruby-style-guide) +- [Ember coding guidelines](ember.md) + [original source](https://github.com/DockYard/styleguides/blob/master/engineering/ember.md) - [ES6 coding guidelines](es6.md) [original source](https://github.com/elierotenberg/coding-styles) - [PostgreSQL coding guidelines](postgres.md) From d696030de75c148d2ca27ad14828b39255444d0f Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Thu, 25 May 2017 16:05:30 -0800 Subject: [PATCH 03/18] Update dockyard emample to be a little more in line with our style --- ember.md | 164 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 90 insertions(+), 74 deletions(-) diff --git a/ember.md b/ember.md index 2e23d6e..e609bc1 100644 --- a/ember.md +++ b/ember.md @@ -10,7 +10,6 @@ * [Routing](#routing) * [Ember data](#ember-data) - ## General ### Import what you use, do not use globals @@ -21,8 +20,6 @@ For Ember, use destructuring [as Ember's internal organization is not intuitive and difficult to grok, and we should wait until Ember has been correctly modularized.](https://github.com/ember-cli/ember-cli-shims/issues/53) -[Here is the RFC on ES2015 modules](https://github.com/emberjs/rfcs/pull/68). - Once Ember has officially adopted shims, we will prefer shims over destructuring. @@ -34,23 +31,19 @@ import DS from 'ember-data'; const { computed, - computed: { alias } } = Ember; const { - Model, - attr + attr, } = DS; -export default Model.extend({ +export default DS.Model.extend({ firstName: attr('string'), lastName: attr('string'), - surname: alias('lastName'), - fullName: computed('firstName', 'lastName', function() { // Code - }) + }), }); // Bad @@ -59,17 +52,9 @@ export default DS.Model.extend({ firstName: DS.attr('string'), lastName: DS.attr('string'), - surname: Ember.computed.alias('lastName'), - - fullName: Ember.computed('firstName', 'lastName', { - get() { - // Code - }, - - set() { - // Code - } - }) + fullName: Ember.computed('firstName', 'lastName', function _fullName() { + // Code + }), }); ``` @@ -78,28 +63,19 @@ export default DS.Model.extend({ Avoid Ember's `Date`, `Function` and `String` prototype extensions. Prefer the corresponding functions from the `Ember` object. -Preferably turn the prototype extensions off by updating the -`EmberENV.EXTEND_PROTOTYPES` setting in your `config/environment` file. - -```javascript -module.exports = function(environment) { - var ENV = { - EmberENV: { - EXTEND_PROTOTYPES: { - Date: false, - Function: false, - String: false - } - } -``` - ```javascript // Good +const { + String: { w }, + computed, + on, +} = Ember; + export default Model.extend({ hobbies: w('surfing skateboarding skydiving'), - fullName: computed('firstName', 'lastName', function() { ... }), - didError: on('error', function() { ... }) + fullName: computed('firstName', 'lastName', function _fullName() { ... }), + didError: on('error', function _didError() { ... }) }); // Bad @@ -128,13 +104,16 @@ import Ember from 'ember'; const { get, set } = Ember; -set(this, 'isSelected', true); -get(this, 'isSelected'); +set(object, 'isSelected', true); +get(object, 'isSelected'); +get(this, 'isPending'); -// Bad +// Ok +this.get('isPending'); -this.set('isSelected', true); -this.get('isSelected'); +// Bad +object.set('isSelected', true); +object.get('isSelected'); ``` ### Use brace expansion @@ -145,16 +124,61 @@ Note that **the dependent keys must be together (without space)** for the brace ```js // Good -fullName: computed('user.{firstName,lastName}', { +fullName: computed('user.{firstName,lastName}', function _fullName{ // Code }) // Bad -fullName: computed('user.firstName', 'user.lastName', { +fullName: computed('user.firstName', 'user.lastName', function _fullName{ // Code }) ``` +### Trailing Commas + +Trailing commas must be used for multiline literals, and must not be used for single-line literals. + +```js +// good +const { get, computed } = Ember; +const { + get, + computed, +} = Ember; + +export default Ember.Object.extend({ + foo() { + }, + + actions: { + bar() { + }, + }, +}); +``` + +*Trailing commas are easier to maintain; one can add or remove properties without having to maintain the commas. Also produces cleaner git diffs.* + +### All functions must be named + +Any function declared with the `function` keyword must have a name. Functions declared with the es6 short "fat arrow" syntax do not need a name. + +```js +// good + +things.map((thing) => get(thing, 'name')); + +fullName: computed('firstName', 'lastName', function _fullName() { + // Code +}), + +// bad + +fullName: computed('firstName', 'lastName', function() { + // Code +}), +``` + ## Organizing your modules Ordering a module's properties in a predictable manner will make it easier to @@ -193,7 +217,7 @@ export default Component.extend({ post: alias('myPost'), // Multiline CP - authorName: computed('author.{firstName,lastName}', function() { + authorName: computed('author.{firstName,lastName}', function _authorName() { // code }), @@ -211,8 +235,8 @@ export default Component.extend({ actions: { someAction() { // Code - } - } + }, + }, }); ``` @@ -243,12 +267,11 @@ import DS from 'ember-data'; const { computed } = Ember; const { - Model, attr, - hasMany + hasMany, } = DS; -export default Model.extend({ +export default DS.Model.extend({ // Attributes firstName: attr('string'), lastName: attr('string'), @@ -257,9 +280,9 @@ export default Model.extend({ children: hasMany('child'), // Computed Properties - fullName: computed('firstName', 'lastName', function() { + fullName: computed('firstName', 'lastName', function _fullName() { // Code - }) + }), }); // Bad @@ -270,19 +293,18 @@ import DS from 'ember-data'; const { computed } = Ember; const { - Model, attr, - hasMany + hasMany, } = DS; -export default Model.extend({ +export default DS.Model.extend({ children: hasMany('child'), firstName: attr('string'), lastName: attr('string'), - fullName: computed('firstName', 'lastName', function() { + fullName: computed('firstName', 'lastName', function _fullName() { // Code - }) + }), }); ``` @@ -301,17 +323,18 @@ is more maintainable, and will fall in line with future routable components ```javascript -export default Controller.extend({ - user: alias('model') +export default Ember.Controller.extend({ + user: computed.readOnly('model'), }); ``` ## Templates -### Do not use partials +### Avoid using partials -Always use components. Partials share scope with the parent view, use -components will provide a consistent scope. +Use components. Partials share scope with the parent view, which makes it hard +to follow the context. Components will provide a consistent scope. Only use +partials if you really, really need the shared scope. ### Don't yield `this` @@ -407,15 +430,6 @@ this.route('foo', { path: ':fooId' }); [Example with broken links](https://ember-twiddle.com/0fea52795863b88214cb?numColumns=3). -### Perform all async actions required for the page to load in route `model` hooks - -The model hooks are async hooks, and will wait for any promises returned -to resolve. An example of this would be models needed to fill a drop -down in a form, you don't want to render this page without the options -in the dropdown. A counter example would be comments on a page. The -comments should be fetched along side the model, but should not block -your page from loading if the required model is there. - ## Ember Data ### Be explicit with Ember Data attribute types @@ -429,13 +443,15 @@ transform is used. export default Model.extend({ firstName: attr('string'), - jerseyNumber: attr('number') + jerseyNumber: attr('number'), }); // Bad export default Model.extend({ firstName: attr(), - jerseyNumber: attr() + jerseyNumber: attr(), }); ``` + +An exception is in cases where a transform is not desired. From 7a07c859886140c2c4531d079ffaf8a51737b260 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Thu, 25 May 2017 16:20:33 -0800 Subject: [PATCH 04/18] Explain named functions better --- ember.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ember.md b/ember.md index e609bc1..6ab9170 100644 --- a/ember.md +++ b/ember.md @@ -163,6 +163,10 @@ export default Ember.Object.extend({ Any function declared with the `function` keyword must have a name. Functions declared with the es6 short "fat arrow" syntax do not need a name. +This helps with debugging, as the name of the function shows up in the backtrace instead of `(anonymous function)`. + +When the function is used as part of a macro to define a property, we have a convention of using the property name prefixed with an underscore. + ```js // good From 7e2b0b07d450f1c640d6f4416301a7cbf86e9c31 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Thu, 25 May 2017 16:21:00 -0800 Subject: [PATCH 05/18] Add some more trailing commas to demonstrate consistency --- ember.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ember.md b/ember.md index 6ab9170..973fddd 100644 --- a/ember.md +++ b/ember.md @@ -126,12 +126,12 @@ Note that **the dependent keys must be together (without space)** for the brace // Good fullName: computed('user.{firstName,lastName}', function _fullName{ // Code -}) +}), // Bad fullName: computed('user.firstName', 'user.lastName', function _fullName{ // Code -}) +}), ``` ### Trailing Commas From f02e2ac333a664c9fcbc804a3b2581899341c066 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Fri, 26 May 2017 14:23:00 -0800 Subject: [PATCH 06/18] Resolve some of @andreafrost's concerns --- ember.md | 46 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/ember.md b/ember.md index 973fddd..7defa95 100644 --- a/ember.md +++ b/ember.md @@ -29,19 +29,15 @@ destructuring. import Ember from 'ember'; import DS from 'ember-data'; -const { - computed, -} = Ember; +const { computed } = Ember; -const { - attr, -} = DS; +const { attr } = DS; export default DS.Model.extend({ firstName: attr('string'), lastName: attr('string'), - fullName: computed('firstName', 'lastName', function() { + fullName: computed('firstName', 'lastName', function _fullName() { // Code }), }); @@ -72,7 +68,7 @@ const { on, } = Ember; -export default Model.extend({ +export default DS.Model.extend({ hobbies: w('surfing skateboarding skydiving'), fullName: computed('firstName', 'lastName', function _fullName() { ... }), didError: on('error', function _didError() { ... }) @@ -80,7 +76,7 @@ export default Model.extend({ // Bad -export default Model.extend({ +export default DS.Model.extend({ hobbies: 'surfing skateboarding skydiving'.w(), fullName: function() { ... }.property('firstName', 'lastName'), didError: function() { ... }.on('error') @@ -139,7 +135,7 @@ fullName: computed('user.firstName', 'user.lastName', function _fullName{ Trailing commas must be used for multiline literals, and must not be used for single-line literals. ```js -// good +// Good const { get, computed } = Ember; const { get, @@ -168,7 +164,7 @@ This helps with debugging, as the name of the function shows up in the backtrace When the function is used as part of a macro to define a property, we have a convention of using the property name prefixed with an underscore. ```js -// good +// Good things.map((thing) => get(thing, 'name')); @@ -176,7 +172,7 @@ fullName: computed('firstName', 'lastName', function _fullName() { // Code }), -// bad +// Bad fullName: computed('firstName', 'lastName', function() { // Code @@ -220,7 +216,7 @@ export default Component.extend({ // Single line CP post: alias('myPost'), - // Multiline CP + // Multi line CP authorName: computed('author.{firstName,lastName}', function _authorName() { // code }), @@ -260,8 +256,6 @@ Models should be grouped as follows: * Associations * Computed Properties -Within each section, the attributes should be ordered alphabetically. - ```js // Good @@ -269,11 +263,7 @@ import Ember from 'ember'; import DS from 'ember-data'; const { computed } = Ember; - -const { - attr, - hasMany, -} = DS; +const { attr, hasMany } = DS; export default DS.Model.extend({ // Attributes @@ -295,11 +285,7 @@ import Ember from 'ember'; import DS from 'ember-data'; const { computed } = Ember; - -const { - attr, - hasMany, -} = DS; +const { attr, hasMany } = DS; export default DS.Model.extend({ children: hasMany('child'), @@ -422,12 +408,10 @@ promises without extra serialization work. ```js -// good - +// Good this.route('foo', { path: ':foo_id' }); -// bad - +// Bad this.route('foo', { path: ':fooId' }); ``` @@ -445,14 +429,14 @@ transform is used. ```javascript // Good -export default Model.extend({ +export default DS.Model.extend({ firstName: attr('string'), jerseyNumber: attr('number'), }); // Bad -export default Model.extend({ +export default DS.Model.extend({ firstName: attr(), jerseyNumber: attr(), }); From cc172ee099a67bb7709a73e375694dd1a8c6b83e Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Fri, 26 May 2017 14:43:00 -0800 Subject: [PATCH 07/18] Change language to prefer to be a little more permissive --- ember.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ember.md b/ember.md index 7defa95..2500108 100644 --- a/ember.md +++ b/ember.md @@ -338,7 +338,7 @@ Use the hash helper to yield what you need instead. {{yield this}} ``` -### Use components in `{{#each}}` blocks +### Prefer components in `{{#each}}` blocks Contents of your each blocks should be a single line, use components when more than one line is needed. This will allow you to test the From 18f440aa9f5ed961ee6d6c902e697b17decd50c6 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Fri, 26 May 2017 14:46:07 -0800 Subject: [PATCH 08/18] Rearrange hbs examples --- ember.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ember.md b/ember.md index 2500108..073bae2 100644 --- a/ember.md +++ b/ember.md @@ -382,12 +382,6 @@ and also make it more clear that you are passing an action. Ultimately, we should make it easier for other developers to read templates. Ordering attributes and then action helpers will provide clarity. -```hbs -{{! Bad }} - - -``` - ```hbs {{! Good }} @@ -400,6 +394,12 @@ Ordering attributes and then action helpers will provide clarity. ``` +```hbs +{{! Bad }} + + +``` + ## Routing ### Route naming From 46f90ae61d680a69ace392828ebf224e1f88fa52 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Fri, 26 May 2017 14:48:20 -0800 Subject: [PATCH 09/18] Clarify module organization example --- ember.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/ember.md b/ember.md index 073bae2..b843f34 100644 --- a/ember.md +++ b/ember.md @@ -209,7 +209,7 @@ scan. 6. __Actions__ ```js -export default Component.extend({ +export default DS.Component.extend({ // Plain properties tagName: 'span', @@ -218,18 +218,22 @@ export default Component.extend({ // Multi line CP authorName: computed('author.{firstName,lastName}', function _authorName() { - // code + // Code }), // Lifecycle hooks didReceiveAttrs() { this._super(...arguments); - // code + // Code }, // Functions someFunction() { - // code + // Code + }, + + _privateInternalFunction() { + // Code }, actions: { @@ -238,6 +242,12 @@ export default Component.extend({ }, }, }); + +// Utility functions should go after the class + +function _someUtilityFunction() { + // Code +} ``` ### Override init From 8858cbfc4c1f6ab7179e9970b5a7b027d8dd6afd Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Mon, 29 May 2017 09:01:52 -0800 Subject: [PATCH 10/18] Start a new section for ordering es6 modules --- ember.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/ember.md b/ember.md index b843f34..7e48c38 100644 --- a/ember.md +++ b/ember.md @@ -181,6 +181,41 @@ fullName: computed('firstName', 'lastName', function() { ## Organizing your modules +### ES6 module files + +1. __Framework imports__ + + First, import Ember and ember addons. + +2. __Internal imports__ + + Then, separated by a blank line, import internals. + +3. __Destructure constants__ + +4. __Object definition__ + +5. __Utility functions__ + + After the object definition, at the bottom of the file, place any private utility functions. + +``` +import Ember from 'ember'; + +import x from 'fern/utils/x'; + +const { computed } = Ember; + +export default Ember.Object.extend({ + // all the properties, this is where the list we're talking about goes +}); + +function _someUtilityFunction() { +} +``` + +### Ember Object Properties + Ordering a module's properties in a predictable manner will make it easier to scan. From 7f4582c1a041437e23cf159d1436baef49fd0afe Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 09:38:11 -0700 Subject: [PATCH 11/18] Add a few more details to the new organization section --- ember.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ember.md b/ember.md index 7e48c38..5efbe98 100644 --- a/ember.md +++ b/ember.md @@ -193,16 +193,27 @@ fullName: computed('firstName', 'lastName', function() { 3. __Destructure constants__ + For example: + + ```js + const { computed } = Ember; + ``` + + There are [more details about destructuring constants above](#import-what-you-use-do-not-use-globals). + 4. __Object definition__ + This is the main body of the file. For details on how to order properties within the object definition, see the next section ([Ember Object Properties](#ember-object-properties)). + 5. __Utility functions__ After the object definition, at the bottom of the file, place any private utility functions. ``` import Ember from 'ember'; +import { task } from 'ember-concurrency'; -import x from 'fern/utils/x'; +import openDrawer from 'fern/utils/open-drawer'; const { computed } = Ember; @@ -244,7 +255,7 @@ scan. 6. __Actions__ ```js -export default DS.Component.extend({ +export default Ember.Component.extend({ // Plain properties tagName: 'span', From 011259461b75fcffcaae86873ee5aba192813b69 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 09:57:19 -0700 Subject: [PATCH 12/18] Add section on readOnly vs oneWay vs alias --- ember.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/ember.md b/ember.md index 5efbe98..c941873 100644 --- a/ember.md +++ b/ember.md @@ -130,6 +130,22 @@ fullName: computed('user.firstName', 'user.lastName', function _fullName{ }), ``` +### Prefer `readOnly` for aliases + +Use the following preference order when using a computed property macro to alias an attribute in an Ember object: + +1. `readOnly` + + This is the strictest of the three and should therefore be the default. It allows aliasing the property and issues an exception when a `set` is attempted. + +2. `oneWay` + + This allows the property to be `set` but does not propogate that new value to the aliased property. Use `oneWay` when the alias is just a default value or when injecting a value is needed for testing. + +3. `alias` + + Use `alias` if and only if you need `set`s to propogate. However, before doing so, consider if updating the value upstream could use [Data Down Actions Up](http://www.samselikoff.com/blog/data-down-actions-up/) instead of a two-way binding. + ### Trailing Commas Trailing commas must be used for multiline literals, and must not be used for single-line literals. @@ -238,7 +254,7 @@ scan. 2. __Single line computed property macros__ - E.g. `alias`, `sort` and other macros. Start with service injections. If the + E.g. `readOnly`, `sort` and other macros. Start with service injections. If the module is a model, then `attr` properties should be first, followed by `belongsTo` and `hasMany`. @@ -260,7 +276,7 @@ export default Ember.Component.extend({ tagName: 'span', // Single line CP - post: alias('myPost'), + post: readOnly('myPost'), // Multi line CP authorName: computed('author.{firstName,lastName}', function _authorName() { From 4eefd1849439957e8c0fb9bce6705eb22f28742f Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 12:27:29 -0700 Subject: [PATCH 13/18] Add section on line length --- ember.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ember.md b/ember.md index c941873..6004af6 100644 --- a/ember.md +++ b/ember.md @@ -449,6 +449,13 @@ and also make it more clear that you are passing an action. {{edit-post post=post deletePost=deletePost}} ``` +### Line length + +* Prefer multi-line expression for HTML elements and components with more than 2 attributes +* Always ensure that HTML elements/components fit within an 96 character line + and prefer 80 characters; if not, expand to multiline +* In all cases, optimize for readability, even in violation of the first 2 points; when it doubt, ask a friend + ### Ordering static attributes, dynamic attributes, and action helpers for HTML elements Ultimately, we should make it easier for other developers to read templates. From ce36b665330cd90f1452414b8c0bf5f972fc23b9 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 12:44:06 -0700 Subject: [PATCH 14/18] Add example component wrapping style --- ember.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ember.md b/ember.md index 6004af6..1592d62 100644 --- a/ember.md +++ b/ember.md @@ -456,6 +456,23 @@ and also make it more clear that you are passing an action. and prefer 80 characters; if not, expand to multiline * In all cases, optimize for readability, even in violation of the first 2 points; when it doubt, ask a friend +### Component wrapping style + +Like this: + +```hbs +{{#power-select + selected=adAccount + options=accounts + searchField="name" + placeholder="-" + onchange=(action 'selectAccount') + as |account| +}} + {{account.name}} ({{account.account_id}}) +{{/power-select}} +``` + ### Ordering static attributes, dynamic attributes, and action helpers for HTML elements Ultimately, we should make it easier for other developers to read templates. From cbcbcdc3df795dde44c203b245a22010b5942420 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 12:48:11 -0700 Subject: [PATCH 15/18] Small edits for consistancy --- ember.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ember.md b/ember.md index 1592d62..c5a7dfb 100644 --- a/ember.md +++ b/ember.md @@ -120,12 +120,12 @@ Note that **the dependent keys must be together (without space)** for the brace ```js // Good -fullName: computed('user.{firstName,lastName}', function _fullName{ +fullName: computed('user.{firstName,lastName}', function _fullName() { // Code }), // Bad -fullName: computed('user.firstName', 'user.lastName', function _fullName{ +fullName: computed('user.firstName', 'user.lastName', function _fullName() { // Code }), ``` @@ -461,6 +461,8 @@ and also make it more clear that you are passing an action. Like this: ```hbs +{{! Good }} + {{#power-select selected=adAccount options=accounts From b457f54377caa7a5339f5c0974d3a6cedf8aa087 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 12:53:05 -0700 Subject: [PATCH 16/18] Update HTML example for consistency --- ember.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ember.md b/ember.md index c5a7dfb..dce9ab7 100644 --- a/ember.md +++ b/ember.md @@ -487,7 +487,8 @@ Ordering attributes and then action helpers will provide clarity. data-auto-id="click-me" name="wonderful-button" disabled={{isDisabled}} - onclick={{action click}}> + onclick={{action click}} +> Click me ``` From aa2e81e25de8e392aba3aa8aa23f7f579eac70f4 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 13:21:28 -0700 Subject: [PATCH 17/18] Be more consistent with gfmd language indicators --- ember.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ember.md b/ember.md index dce9ab7..d64874e 100644 --- a/ember.md +++ b/ember.md @@ -23,7 +23,7 @@ correctly modularized.](https://github.com/ember-cli/ember-cli-shims/issues/53) Once Ember has officially adopted shims, we will prefer shims over destructuring. -```javascript +```js // Good import Ember from 'ember'; @@ -59,7 +59,7 @@ export default DS.Model.extend({ Avoid Ember's `Date`, `Function` and `String` prototype extensions. Prefer the corresponding functions from the `Ember` object. -```javascript +```js // Good const { @@ -225,7 +225,7 @@ fullName: computed('firstName', 'lastName', function() { After the object definition, at the bottom of the file, place any private utility functions. -``` +```js import Ember from 'ember'; import { task } from 'ember-concurrency'; @@ -384,7 +384,7 @@ It provides a cleaner code to name your model `user` if it is a user. It is more maintainable, and will fall in line with future routable components -```javascript +```js export default Ember.Controller.extend({ user: computed.readOnly('model'), }); @@ -525,7 +525,7 @@ Even though Ember Data can be used without using explicit types in `attr`, always supply an attribute type to ensure the right data transform is used. -```javascript +```js // Good export default DS.Model.extend({ From fea5ea91f6893c2ef919cbc7f2eda4c333d6e545 Mon Sep 17 00:00:00 2001 From: Amiel Martin Date: Tue, 30 May 2017 13:28:55 -0700 Subject: [PATCH 18/18] Add section on destructuring line length --- ember.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ember.md b/ember.md index d64874e..ba8da10 100644 --- a/ember.md +++ b/ember.md @@ -54,6 +54,12 @@ export default DS.Model.extend({ }); ``` +### Destructured constants line length + +* Prefer splitting a destructure in to multiple lines when there are 3 or more attributes +* Always split a destructure in to mulitple lines when it has more than one level of nesting +* In all cases, optimize for readability, even in violation of the first 2 points; when it doubt, ask a friend + ### Don't use Ember's prototype extensions Avoid Ember's `Date`, `Function` and `String` prototype extensions. Prefer the @@ -456,7 +462,7 @@ and also make it more clear that you are passing an action. and prefer 80 characters; if not, expand to multiline * In all cases, optimize for readability, even in violation of the first 2 points; when it doubt, ask a friend -### Component wrapping style +### Component line wrapping style Like this: