From 39596289cd91bac9cdc0e76138292e5932425cd6 Mon Sep 17 00:00:00 2001 From: aaronjudd Date: Wed, 26 Jul 2017 15:47:50 -0700 Subject: [PATCH 1/7] Updated styleguide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - of course, we’ll have some updates to make following this! --- developer/styleguide.md | 133 +++++++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 48 deletions(-) diff --git a/developer/styleguide.md b/developer/styleguide.md index f3b1c91bc..96f48ee57 100644 --- a/developer/styleguide.md +++ b/developer/styleguide.md @@ -4,15 +4,14 @@ Our rules are similar to AirBnB or Meteor rules, [standard template of ESLint ru A couple of notable Reaction specific style rules: -* Double quoted strings -* Well spaced function -* 160 character line length -* `import` order - * React npm packages (`React`, `prop-types`, etc...) - * other npm packages - * meteor core packages - * meteor (Atmosphere) packages - * local app files +- Double quoted strings +- Well spaced function +- 160 character line length +- `import` order + - npm packages + - meteor core packages + - meteor (Atmosphere) packages + - local app files Review [Meteor Code Style](https://guide.meteor.com/code-style.html) for additional guidelines that are typical of Meteor projects. @@ -20,9 +19,9 @@ Review [Meteor Code Style](https://guide.meteor.com/code-style.html) for additio In the Reaction app root, we have Reaction specific configuration files that can be used with most editors with the appropriate tools installed. -* `.eslintrc` - [http://eslint.org/](https://eslint.org/) -* `.jsbeautifyrc` - [http://jsbeautifier.org/](https://jsbeautifier.org/) -* `.editorconfig` - [http://editorconfig.org/](https://editorconfig.org/) +- `.eslintrc` - [http://eslint.org/](https://eslint.org/) +- `.jsbeautifyrc` - [http://jsbeautifier.org/](https://jsbeautifier.org/) +- `.editorconfig` - [http://editorconfig.org/](https://editorconfig.org/) These configurations also include additional rules supporting [ES2015](https://docs.meteor.com/packages/ecmascript.html#Supported-ES2015-Features) and `React`. @@ -38,7 +37,7 @@ apm install editorconfig atom-beautify linter linter-eslint linter-markdown lint ### Lint -Reaction installs the `eslint` v3.x packages _eslint_, _eslint-plugin-react_, _babel-eslint_ from [npm](https://www.npmjs.com/) as development dependencies. +Reaction installs the `eslint` packages _eslint_, _eslint-plugin-react_, _babel-eslint_ from [npm](https://www.npmjs.com/) as development dependencies. In the Reaction app root, we have a Reaction `eslint` configuration. @@ -54,71 +53,109 @@ In our Markdown documentation, we use [remark-lint](https://github.com/wooorm/re Pull request branches are evaluated using [BitHound](https://www.bithound.io/github/reactioncommerce/reaction) for insecure dependencies and code quality. -* must pass a Linting check and _no errors_ are accepted. -* must pass a Dependency check and no priority packages are accepted. -* must pass `reaction test` +- must pass a Linting check and _no errors_ are accepted. +- must pass a Dependency check and no priority packages are accepted. +- must pass `reaction test` In many cases, documentation updates can be required as well. Pull requests are submitted to a peer code review process before acceptance. -### File Naming Conventions +### Naming Conventions -In general we use hyphens (-) and camelCase for folder names, and camelCase alone for file names. Underscores are not to be used for file or folder names unless expressly required. Be aware that not all operating systems are case sensitive, so it's not ok to have two files named the same with differing case. +In general we use hyphens (-) for folder names. Underscores are not to be used for file or folder names unless expressly required. Be aware that not all operating systems are case sensitive, so it's not ok to have two files named the same with differing case. To prevent this, we recommend not using camelCasing when naming folders or files. -#### Folder Names +As a convention of there are multiple files that make up a functionality, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the seperator. **Good** -* Folder names for packages must contain only lowercased alpha numeric characters and may be hyphenated if joining more than one word -* Folder names all normal directories must start with a lowercased letter and may camel cased if joining more than one word - +```sh +/container/settings.js +/container/settings.html +/container/ ``` -// Packages in /imports/plugins -ui-grid/ -example-paymentmethod/ -social/ -taxes-avalara/ -// All other folder names everywhere -addressBook/ +or +```sh +/settings-container.js +/settings-container.html ``` **Bad** -* Package name should contain hyphens to make it easier to read. -* Underscores are not to be used unless expressly required. +```sh +/container/settings-containers.js +/container/settingsContainer.html +settingsContainer.js +``` + +#### Folders +**Good** + +- Folder names for packages should contain only lowercased alpha numeric characters and are **hyphenated** if joining more than one word + +```sh + // Packages in /imports/plugins + ui-grid/ + example-paymentmethod/ + social/ + taxes-avalara/ ``` -reactionpackagename/ -address_book/ + +**Bad** + +- Package name should contain hyphens to make it easier to read. +- Underscores are not to be used unless expressly required. +- Folder names should not be camelCased. + +```sh + reactionpackagename/ + address_book/ ``` -#### File Names +#### Files **Good** -* File names must start with a lowercased letter and may be camel cased if joining more than one word. -* File names may contain multiple (.) characters as needed +- File names should contain only lowercased alpha numeric characters and are **hyphenated** if joining more than one word +- File names may contain multiple (.) characters as needed +- File names can use hyphens but should avoid camelCase naming. +```sh + bootstrap.rtl.js + index.js ``` -settingsContainer.js -publishContainer.js -addressBook.js -bootstrap.rtl.js -index.js -// This is an exception as it's part of Meteor's naming convention -addressBook.app-test.js +**Bad** + +- Underscores and camelCased are not to be used unless expressly required; + +```sh + settingsContainer.js + publishContainer.js + addressBook.js + settings_container.js + publish-container.js + address_book.js ``` -**Bad** +#### Packages -* Hyphens and underscores are not to be used unless expressly required; such is the case with Meteor for `*.app-test.js` files. Or for migration files to make the filename more readable. +We suggest that package folders follow a `-` format. + +**Good** +```sh +/imports/plugins/custom/payments-custom-provider +/imports/plugins/included/shipping-provider ``` -settings_container.js -publish-container.js -address_book.js + +Registry package names should preferably use some organizational namespacing as a prefix. + +```js +reaction-paypal +reaction-google-analytics +yourorg-your-package ``` From e0da73cdc88085e85806fcadb05ab1529ab5d00e Mon Sep 17 00:00:00 2001 From: aaronjudd Date: Wed, 26 Jul 2017 15:48:08 -0700 Subject: [PATCH 2/7] Add code review docs - start a list of code review requirements --- developer/code-reviews.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 developer/code-reviews.md diff --git a/developer/code-reviews.md b/developer/code-reviews.md new file mode 100644 index 000000000..2d05e031d --- /dev/null +++ b/developer/code-reviews.md @@ -0,0 +1,21 @@ +# Code Review + +Code contributions are reviewed by the Reaction core and community contributors. +All merge requests are reviewed and approved by at least one core contributor before merge. + +Here are is a brief checklist of common items reviewed in every merge. + +- Code style guide adhered to and **linted** (`eslint`, 0 errors, no ignores) +- i18n keys on all text +- i18n values added to appropriate `en.json` +- Translations are updated (LingoHub) +- Naming conventions +- does not introduce new Atmosphere or Meteor dependencies +- jsDoc (API documentation) + + - every file should have a summary + - every method should a summary + +- sufficient inline commenting + - explains functionality to someone NOT reading any other docs + - enough detail to assistance full documentation to expand upon From efe945bdda14a40d30e0369d5de8f9543e94faed Mon Sep 17 00:00:00 2001 From: aaronjudd Date: Mon, 31 Jul 2017 09:49:27 -0700 Subject: [PATCH 3/7] Updated review/styleguide --- developer/code-reviews.md | 5 +++-- developer/styleguide.md | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/developer/code-reviews.md b/developer/code-reviews.md index 2d05e031d..b2b96aae0 100644 --- a/developer/code-reviews.md +++ b/developer/code-reviews.md @@ -14,8 +14,9 @@ Here are is a brief checklist of common items reviewed in every merge. - jsDoc (API documentation) - every file should have a summary - - every method should a summary + - every method should have a summary + - links to source documentation where appropriate - sufficient inline commenting - explains functionality to someone NOT reading any other docs - - enough detail to assistance full documentation to expand upon + - enough detail so the detailed docs can expand upon comments diff --git a/developer/styleguide.md b/developer/styleguide.md index 96f48ee57..90e321734 100644 --- a/developer/styleguide.md +++ b/developer/styleguide.md @@ -5,8 +5,9 @@ Our rules are similar to AirBnB or Meteor rules, [standard template of ESLint ru A couple of notable Reaction specific style rules: - Double quoted strings -- Well spaced function -- 160 character line length +- Well spaced functions +- Spaces around brackets +- 120 character line length - `import` order - npm packages - meteor core packages @@ -65,7 +66,7 @@ Pull requests are submitted to a peer code review process before acceptance. In general we use hyphens (-) for folder names. Underscores are not to be used for file or folder names unless expressly required. Be aware that not all operating systems are case sensitive, so it's not ok to have two files named the same with differing case. To prevent this, we recommend not using camelCasing when naming folders or files. -As a convention of there are multiple files that make up a functionality, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the seperator. +As a convention, if there are multiple files that provide functionality that is broken down across multiple files, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the seperator. **Good** From b8c2e0f9cb137c12b144c83f50f97ed5c632cd14 Mon Sep 17 00:00:00 2001 From: machiko Date: Fri, 29 Sep 2017 11:41:32 -0700 Subject: [PATCH 4/7] Update code-review Add details on what happens post-merge, how triage happens, and exact commands to run before you make a PR. --- developer/code-reviews.md | 58 +++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/developer/code-reviews.md b/developer/code-reviews.md index b2b96aae0..c4dfd1e46 100644 --- a/developer/code-reviews.md +++ b/developer/code-reviews.md @@ -1,22 +1,46 @@ -# Code Review +# Code review process -Code contributions are reviewed by the Reaction core and community contributors. -All merge requests are reviewed and approved by at least one core contributor before merge. +Code contributions are reviewed by the Reaction core and community contributors. All pull requests are reviewed and approved by at least one core contributor before merge. -Here are is a brief checklist of common items reviewed in every merge. +Here's what to expect when you make a pull request to Reaction: -- Code style guide adhered to and **linted** (`eslint`, 0 errors, no ignores) -- i18n keys on all text -- i18n values added to appropriate `en.json` -- Translations are updated (LingoHub) -- Naming conventions -- does not introduce new Atmosphere or Meteor dependencies -- jsDoc (API documentation) +## Automated pull request checks - - every file should have a summary - - every method should have a summary - - links to source documentation where appropriate +As soon as pull requests are pushed, [BitHound](https://www.bithound.io/github/reactioncommerce/reaction), [CircleCI](https://circleci.com/gh/reactioncommerce/reaction) and [Synk](https://snyk.io/) run automated tests. All checks should pass successfully to ensure: -- sufficient inline commenting - - explains functionality to someone NOT reading any other docs - - enough detail so the detailed docs can expand upon comments +- All linting rules pass withour errors or ignores. +- Dependencies are not failing. +- All tests pass. +- No new security vulnerabilities are introduced. + +You can run these checks yourself by running these commands: +- `eslint .` for linting +- `reaction test` for all tests + +## Core team pull request review + +Every Monday, the Core team triages all new pull requests. The Core team reviews code quality rules including: + +- No new Atmosphere or Meteor dependencies are introduced. +- No hard-coded copy: All copy and alerts should have i18n keys and values. +- Updated LingoHub translations. +- All new methods and files have jsdoc summaries, as outlined in [JSDoc guide](https://github.com/reactioncommerce/reaction-jsdoc#how-to-write-docs). +- All folders, variables, method names follow naming conventions, outlined in [Reaction style guide](/developer/styleguide.md). + +The Core team also encourages in-line commenting. Use comments to: +- Explain functionality to someone new to the code +- Link to any external documentation + + +## Getting feedback early and often + +Want to get feedback on your pull request before it's ready for merging? + +Push up the branch and add `[WIP]` for Work in Progress to the title and ask a question in GitHub. + +## Congrats! It's approved and merged. What's next? + +Once a pull request goes through both the automated and Core team reviews, it's ready to be merged. Here are some things you may want to consider after that: + +- If your pull request referenced an issue, close that issue. +- Does your new feature require new user documentation or developer documentation? Make an issue for that in [reaction-docs](https://github.com/reactioncommerce/reaction-docs/issues). From e672d9a17d40e19ec98037a644211aa58f81d4a1 Mon Sep 17 00:00:00 2001 From: machiko Date: Fri, 29 Sep 2017 12:34:10 -0700 Subject: [PATCH 5/7] Update style guide with JS recommendations from team --- developer/styleguide.md | 230 +++++++++++++++++++++++++--------------- 1 file changed, 142 insertions(+), 88 deletions(-) diff --git a/developer/styleguide.md b/developer/styleguide.md index d755d5708..f04295721 100644 --- a/developer/styleguide.md +++ b/developer/styleguide.md @@ -1,75 +1,95 @@ -# Style Guide +# Style guide -Our rules are similar to AirBnB or Meteor rules, [standard template of ESLint rules](https://www.npmjs.com/package/eslint-config-airbnb), although we have tweaked them a little with what is working best for us. +As a community, Reaction follows guidelines for JavaScript style and syntax conventions, along with guidelines for naming variables, methods and filenames. -A couple of notable Reaction specific style rules: +## Syntax and style conventions -- Double quoted strings -- Well spaced functions -- Spaces around brackets -- 120 character line length +Our rules are similar to [Airbnb JavaScript Style Guide](https://github.com/airbnb/javascript) and [Meteor Code Style](https://guide.meteor.com/code-style.html), [standard template of ESLint rules](https://www.npmjs.com/package/eslint-config-airbnb), with a few custom Reaction-specific rules: + +- Double-quoted strings +- Well-spaced functions +- Spaces around brackets +- 120 character line-length - `import` order - React npm packages (`React`, `prop-types`, etc...) - other npm packages - - meteor core packages - - meteor (Atmosphere) packages + - Meteor core packages + - Meteor (Atmosphere) packages - local app files -Review [Meteor Code Style](https://guide.meteor.com/code-style.html) for additional guidelines that are typical of Meteor projects. +Reaction uses various linting libraries to automate style checking. Find the exact rules in the code: +- [`.eslintrc`](https://github.com/reactioncommerce/reaction/blob/master/.eslintrc) - [ESLint](http://eslint.org) checks JavaScript style, including [ES2015](https://docs.meteor.com/packages/ecmascript.html#Supported-ES2015-Features), React and Babel. +- [`.jsbeautifyrc`](https://github.com/reactioncommerce/reaction/blob/master/.jsbeautifyrc) - [JS Beautifier](jsbeautifier.org) automates code formatting +- [`.editorconfig`](https://github.com/reactioncommerce/reaction/blob/master/.editorconfig) - [Editor Config](https://editorconfig.org/) standardizes file formatting + +To see the rules in action, run `eslint .` from the command line or use [ESLint code editor tools](https://eslint.org/docs/user-guide/integrations). -## Editor Configuration +💡 **ProTip!** If you're using [Atom](https://atom.io/), like the Core team, you can install all the necessary tools in one line: `apm install editorconfig atom-beautify linter linter-eslint linter-markdown linter-jsonlint linter-docker` -In the Reaction app root, we have Reaction specific configuration files that can be used with most editors with the appropriate tools installed. +## Naming conventions -- `.eslintrc` - [http://eslint.org/](https://eslint.org/) -- `.jsbeautifyrc` - [http://jsbeautifier.org/](https://jsbeautifier.org/) -- `.editorconfig` - [http://editorconfig.org/](https://editorconfig.org/) +### Avoid camel-casing files and folders -These configurations also include additional rules supporting [ES2015](https://docs.meteor.com/packages/ecmascript.html#Supported-ES2015-Features) and `React`. +Not all operating systems are case sensitive, so avoid having two files named the same with differing case. Instead of camel-casing, use hyphens. Underscores are not to be used for file or folder names unless expressly required. -### Atom +Names of folders should be: +- Lowercased +- Alphanumeric characters +- Hyphenated to join more words +- Avoid camelCase, undescore_casing -We've been using the Open Source [Atom](https://atom.io/) editor for some time now and can recommend it. +File names follow the same conventions as folder names, and also allow for names to contain multiple (.) characters as needed. -Setting up Atom is a quick command where we use the shell command `apm` to install the same [Atom linting packages](https://atom.io/users/AtomLinter) as we use for Reaction. +**Do** ```sh -apm install editorconfig atom-beautify linter linter-eslint linter-markdown linter-jsonlint linter-docker + ui-grid/ + example-payment-method/ + social/ ``` -### Lint - -Reaction installs the `eslint` packages _eslint_, _eslint-plugin-react_, _babel-eslint_ from [npm](https://www.npmjs.com/) as development dependencies. - -In the Reaction app root, we have a Reaction `eslint` configuration. - -`.eslintrc` - [http://eslint.org/](https://eslint.org/) +```sh + bootstrap.rtl.js + index.js +``` -For more details, see the [ESLint Getting Started Guide](http://eslint.org/docs/user-guide/getting-started). +**Don't** -#### Markdown +```sh + reactionpackagename/ + address_book/ + addressBook/ +``` -In our Markdown documentation, we use [remark-lint](https://github.com/wooorm/remark-lint) to ensure consistent Markdown styling. +```sh + settingsContainer.js + addressBook.js + address_book.js +``` -### Pull Requests +### Create strong package names -Pull request branches are evaluated using [BitHound](https://www.bithound.io/github/reactioncommerce/reaction) for insecure dependencies and code quality. -- must pass a Linting check and _no errors_ are accepted. -- must pass a Dependency check and no priority packages are accepted. -- must pass `reaction test` +We suggest that package folders follow a `-` format. -In many cases, documentation updates can be required as well. +```sh +/imports/plugins/custom/payments-custom-provider +/imports/plugins/included/shipping-provider +``` -Pull requests are submitted to a peer code review process before acceptance. +Registry package names should preferably use some organizational namespacing as a prefix. -### Naming Conventions +```js +reaction-paypal +reaction-google-analytics +yourorg-your-package +``` -In general we use hyphens (-) for folder names. Underscores are not to be used for file or folder names unless expressly required. Be aware that not all operating systems are case sensitive, so it's not ok to have two files named the same with differing case. To prevent this, we recommend not using camelCasing when naming folders or files. +### Group related files together into folders -As a convention, if there are multiple files that provide functionality that is broken down across multiple files, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the seperator. +As a convention, if there are multiple files that provide functionality that is broken down across multiple files, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the separator. -**Good** +**Do** ```sh /container/settings.js @@ -84,7 +104,7 @@ or /settings-container.html ``` -**Bad** +**Don't** ```sh /container/settings-containers.js @@ -92,74 +112,108 @@ or settingsContainer.js ``` -#### Folders +## JavaScript style recommendations -**Good** +### Variables -- Folder names for packages should contain only lowercased alpha numeric characters and are **hyphenated** if joining more than one word +#### Naming variables -```sh - // Packages in /imports/plugins - ui-grid/ - example-paymentmethod/ - social/ - taxes-avalara/ -``` +Variable names should be: -**Bad** +- Meaningful. Avoid single-letter variables. +- Use `error`, instead of `e` or `err` +- Variables that contains a boolean value should use a `isThisTrue` style. +- Variables that return arrays should be plural. -- Package name should contain hyphens to make it easier to read. -- Underscores are not to be used unless expressly required. -- Folder names should not be camelCased. +Publication names should: -```sh - reactionpackagename/ - address_book/ -``` +- Use TitleCase -#### Files +#### Working with variables -**Good** +- Use `const` except when a variable is being reassigned. +## Methods -- File names should contain only lowercased alpha numeric characters and are **hyphenated** if joining more than one word -- File names may contain multiple (.) characters as needed -- File names can use hyphens but should avoid camelCase naming. +#### Method names should: +- Methods names should use a verb/noun style when possible, e.g. `checkConfiguration` or `addToCart`. +- Methods that return a single value should be singular. +- Methods whose main purpose is to return a value should use the get prefix. -```sh - bootstrap.rtl.js - index.js +- Avoid ternary operators and one-line `if` statements + +#### Use parenthesis for clarity + +- Use parenthesis around all method arguments. + +**Don't** +``` +cartItems.find(item => item.status === picked) ``` -**Bad** +**Do** +``` +cartItems.find((item) => item.status === picked) +``` -- Underscores and camelCased are not to be used unless expressly required; +- When specifying a callback, always use the parenthesis to indicate the argument being accepted over the bare arrow-function form. This way, it's clear it's not another argument to the original function: -```sh - settingsContainer.js - publishContainer.js - addressBook.js - settings_container.js - publish-container.js - address_book.js +**Don't** ``` +thisMethodTakesACallback(arg1, arg2, result => { + doStuff +}); +``` +**Do** -#### Packages +``` +thisMethodTakesACallback(arg1, arg2, (result) => { + dostuff(); +}); +``` -We suggest that package folders follow a `-` format. +#### Avoid multi-line shorthand arrow functions -**Good** +If you have to break your arrow function into multiple lines, use curly brackets and a return rather than trying to break the shorthand arrow function into multiple lines. -```sh -/imports/plugins/custom/payments-custom-provider -/imports/plugins/included/shipping-provider + +**Don't** + ``` +cartItems.find( + (item) => item.status === status + ) ``` -Registry package names should preferably use some organizational namespacing as a prefix. +**Do** + ``` +cartItems.find((item) => { + return item.status === status; +}) +``` -```js -reaction-paypal -reaction-google-analytics -yourorg-your-package + +#### Working with collections + +Be explicit in querying: + +**Don't** +``` +Products.findOne("abc123") +``` +**Do** +``` +Products.findOne({ _id: "abc123" }) ``` + +#### Using lodash and ES2015 + +Use native ES6 elements over lodash whenever there is a 1-for-1 replacement. Here are some examples of lodash methods that can be replaced with native elements: + +- Replace `_.map`, with `Array.prototype.map` +- Replace `_.reduce`, with `Array.prototype.reduce` +- Replace `_.filter`, with `Array.prototype.filter` +- Replace `_.each`, `_.forEach`, with `Array.prototype.forEach` +- Replace `_.isArray`, with `Array.isArray` +- Replace `_.extend` with `Object.assign` + From 190b0d95c525e7320dacd030ed9c01a47fe3def7 Mon Sep 17 00:00:00 2001 From: machiko Date: Fri, 29 Sep 2017 12:38:56 -0700 Subject: [PATCH 6/7] Update styleguide.md --- developer/styleguide.md | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/developer/styleguide.md b/developer/styleguide.md index f04295721..18bee7d86 100644 --- a/developer/styleguide.md +++ b/developer/styleguide.md @@ -69,8 +69,7 @@ File names follow the same conventions as folder names, and also allow for names ### Create strong package names - -We suggest that package folders follow a `-` format. +Name package folders in this format: `-` ```sh /imports/plugins/custom/payments-custom-provider @@ -114,9 +113,9 @@ settingsContainer.js ## JavaScript style recommendations -### Variables +## Variables -#### Naming variables +### Naming variables Variable names should be: @@ -129,22 +128,22 @@ Publication names should: - Use TitleCase -#### Working with variables +### Working with variables - Use `const` except when a variable is being reassigned. ## Methods -#### Method names should: +### Method names should: - Methods names should use a verb/noun style when possible, e.g. `checkConfiguration` or `addToCart`. - Methods that return a single value should be singular. - Methods whose main purpose is to return a value should use the get prefix. - Avoid ternary operators and one-line `if` statements -#### Use parenthesis for clarity +### Use parenthesis for clarity -- Use parenthesis around all method arguments. +- Use parenthesis around all method arguments. **Don't** ``` @@ -164,7 +163,7 @@ thisMethodTakesACallback(arg1, arg2, result => { doStuff }); ``` -**Do** +**Do** ``` thisMethodTakesACallback(arg1, arg2, (result) => { @@ -173,7 +172,7 @@ thisMethodTakesACallback(arg1, arg2, (result) => { ``` -#### Avoid multi-line shorthand arrow functions +### Avoid multi-line shorthand arrow functions If you have to break your arrow function into multiple lines, use curly brackets and a return rather than trying to break the shorthand arrow function into multiple lines. @@ -192,8 +191,7 @@ cartItems.find((item) => { }) ``` - -#### Working with collections +### Working with collections Be explicit in querying: @@ -206,7 +204,7 @@ Products.findOne("abc123") Products.findOne({ _id: "abc123" }) ``` -#### Using lodash and ES2015 +### Replacing lodash with ES2015 Use native ES6 elements over lodash whenever there is a 1-for-1 replacement. Here are some examples of lodash methods that can be replaced with native elements: @@ -216,4 +214,3 @@ Use native ES6 elements over lodash whenever there is a 1-for-1 replacement. Her - Replace `_.each`, `_.forEach`, with `Array.prototype.forEach` - Replace `_.isArray`, with `Array.isArray` - Replace `_.extend` with `Object.assign` - From 345a9ed2c6d03ef82580fd6316bc9c55ce62d488 Mon Sep 17 00:00:00 2001 From: machiko Date: Wed, 4 Oct 2017 06:41:20 -0700 Subject: [PATCH 7/7] Update styleguide.md --- developer/styleguide.md | 196 +++++++++++++++++++++++++--------------- 1 file changed, 123 insertions(+), 73 deletions(-) diff --git a/developer/styleguide.md b/developer/styleguide.md index 18bee7d86..d7acac37c 100644 --- a/developer/styleguide.md +++ b/developer/styleguide.md @@ -1,23 +1,34 @@ -# Style guide +# Coding style guide -As a community, Reaction follows guidelines for JavaScript style and syntax conventions, along with guidelines for naming variables, methods and filenames. +As a community, Reaction follows guidelines for code style and naming conventions for variables, methods and filenames. The guide also includes tips on working with libraries in Reaction, like React, MongoDB, lodash and more. + +## On this page + +- [General rules](#syntax-and-style-conventions) +- [Naming files and folders](#naming-conventions) +- [Packages](#working-with-packages) +- [Variables](#variables) +- [Methods](#methods) +- [MongoDB](#working-with-collections) +- [Native JavaScript](#using-native-javascript) +- [React](#working-with-react) ## Syntax and style conventions Our rules are similar to [Airbnb JavaScript Style Guide](https://github.com/airbnb/javascript) and [Meteor Code Style](https://guide.meteor.com/code-style.html), [standard template of ESLint rules](https://www.npmjs.com/package/eslint-config-airbnb), with a few custom Reaction-specific rules: -- Double-quoted strings -- Well-spaced functions -- Spaces around brackets +- Always double-quote strings +- Give methods space +- Add spaces around brackets - 120 character line-length -- `import` order +- `import` in order: - React npm packages (`React`, `prop-types`, etc...) - other npm packages - Meteor core packages - Meteor (Atmosphere) packages - local app files -Reaction uses various linting libraries to automate style checking. Find the exact rules in the code: +Other Reaction-specific rules are checked using various linting libraries. Find all the rules in the code: - [`.eslintrc`](https://github.com/reactioncommerce/reaction/blob/master/.eslintrc) - [ESLint](http://eslint.org) checks JavaScript style, including [ES2015](https://docs.meteor.com/packages/ecmascript.html#Supported-ES2015-Features), React and Babel. - [`.jsbeautifyrc`](https://github.com/reactioncommerce/reaction/blob/master/.jsbeautifyrc) - [JS Beautifier](jsbeautifier.org) automates code formatting - [`.editorconfig`](https://github.com/reactioncommerce/reaction/blob/master/.editorconfig) - [Editor Config](https://editorconfig.org/) standardizes file formatting @@ -28,65 +39,57 @@ To see the rules in action, run `eslint .` from the command line or use [ESLint ## Naming conventions -### Avoid camel-casing files and folders +### Use hyphens to separate words in names -Not all operating systems are case sensitive, so avoid having two files named the same with differing case. Instead of camel-casing, use hyphens. Underscores are not to be used for file or folder names unless expressly required. +File and directory names should be hyphenated. Not all operating systems are case sensitive, so avoid having two files named the same with differing case. -Names of folders should be: +Names of folders and files should be: - Lowercased - Alphanumeric characters - Hyphenated to join more words -- Avoid camelCase, undescore_casing - -File names follow the same conventions as folder names, and also allow for names to contain multiple (.) characters as needed. +- Avoid `camelCase`, `undescore_casing` +- Files may use multiple `.` as needed **Do** ```sh - ui-grid/ - example-payment-method/ - social/ -``` - -```sh - bootstrap.rtl.js - index.js + /ui-grid/ + /example-payment-method/ + /social/ + /bootstrap.rtl.js + /index.js ``` **Don't** ```sh - reactionpackagename/ - address_book/ - addressBook/ + /reactionpackagename/ + /address_book/ + /addressBook/ + /settingsContainer.js + /addressBook.js + /address_book.js ``` -```sh - settingsContainer.js - addressBook.js - address_book.js -``` +## Working with packages ### Create strong package names -Name package folders in this format: `-` +Namespace package folders in this format: `-` or `-` ```sh -/imports/plugins/custom/payments-custom-provider -/imports/plugins/included/shipping-provider -``` - -Registry package names should preferably use some organizational namespacing as a prefix. - -```js -reaction-paypal -reaction-google-analytics -yourorg-your-package +/imports/plugins/custom/payments-authnet +/imports/plugins/included/connectors-shopify +/imports/plugins/custom/connectors-magento +/reaction-paypal/ +/yourorg-your-package/ ``` ### Group related files together into folders -As a convention, if there are multiple files that provide functionality that is broken down across multiple files, these files should be grouped within a folder. If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the separator. +If there are multiple files that provide functionality that is broken down across multiple files, these files should be grouped within a folder. + +If the files are stand-alone and the name needs to represent functionality, you can use a hyphen for the separator. **Do** @@ -113,56 +116,50 @@ settingsContainer.js ## JavaScript style recommendations -## Variables +### Variables -### Naming variables +#### Naming variables Variable names should be: - Meaningful. Avoid single-letter variables. - Use `error`, instead of `e` or `err` -- Variables that contains a boolean value should use a `isThisTrue` style. +- Variables that contain a boolean value should use a `isThisTrue` style. - Variables that return arrays should be plural. Publication names should: - Use TitleCase -### Working with variables +#### Working with variables - Use `const` except when a variable is being reassigned. -## Methods +### Methods + +#### Naming methods -### Method names should: - Methods names should use a verb/noun style when possible, e.g. `checkConfiguration` or `addToCart`. - Methods that return a single value should be singular. -- Methods whose main purpose is to return a value should use the get prefix. +- Methods whose main purpose is to return a value should use the `get` prefix, e.g. `getShop`. +- Avoid ternary operators and one-line `if` statements (except in React componenets) -- Avoid ternary operators and one-line `if` statements - -### Use parenthesis for clarity +#### Working with methods - Use parenthesis around all method arguments. -**Don't** -``` -cartItems.find(item => item.status === picked) -``` - **Do** ``` cartItems.find((item) => item.status === picked) ``` -- When specifying a callback, always use the parenthesis to indicate the argument being accepted over the bare arrow-function form. This way, it's clear it's not another argument to the original function: - **Don't** ``` -thisMethodTakesACallback(arg1, arg2, result => { - doStuff -}); +cartItems.find(item => item.status === picked) ``` + +- When specifying a callback, always use the parenthesis to indicate the argument being accepted over the bare arrow-function form. This way, it's clear it's not another argument to the original function: + **Do** ``` @@ -171,18 +168,14 @@ thisMethodTakesACallback(arg1, arg2, (result) => { }); ``` - -### Avoid multi-line shorthand arrow functions - -If you have to break your arrow function into multiple lines, use curly brackets and a return rather than trying to break the shorthand arrow function into multiple lines. - - **Don't** - ``` -cartItems.find( - (item) => item.status === status - ) ``` +thisMethodTakesACallback(arg1, arg2, result => { + doStuff +}); +``` + +- When breaking arrow functions into multiple lines, use curly brackets and a `return`: **Do** ``` @@ -191,6 +184,13 @@ cartItems.find((item) => { }) ``` +**Don't** +``` +cartItems.find( + (item) => item.status === status + ) +``` + ### Working with collections Be explicit in querying: @@ -204,9 +204,11 @@ Products.findOne("abc123") Products.findOne({ _id: "abc123" }) ``` -### Replacing lodash with ES2015 +### Using native JavaScript + +Use native JavaScript over lodash whenever there is a suitable replacement. -Use native ES6 elements over lodash whenever there is a 1-for-1 replacement. Here are some examples of lodash methods that can be replaced with native elements: +Here are some examples of lodash methods that can be replaced with native elements: - Replace `_.map`, with `Array.prototype.map` - Replace `_.reduce`, with `Array.prototype.reduce` @@ -214,3 +216,51 @@ Use native ES6 elements over lodash whenever there is a 1-for-1 replacement. Her - Replace `_.each`, `_.forEach`, with `Array.prototype.forEach` - Replace `_.isArray`, with `Array.isArray` - Replace `_.extend` with `Object.assign` + +### Working with React + +Use the return shorthand with arrow functions in React components: + +**Don't** +``` +const MyComponent = ({ title, content }) => { + return ( +
+

{title}

+
{content}/
+
+ ); +} +``` + +**Do** +``` +const MyComponent = ({ title, content }) => ( +
+

{title}

+
{content}/
+
+); +``` + +When iterating over arrays in a component: + +**Do** + +``` +const MyThings = ({ things }) => ( +
    + {things.map((thing) =>
  • {thing}
  • )} +
+); + +// or + +const MyThings = ({ things }) => ( +
    + {things.map((thing) => ( +
  • {thing}
  • + ))} +
+); +```