MozillaWiki

DevTools/CodingStandards: Difference between revisions

Page Discussion

DevTools/CodingStandards (view source)

Revision as of 18:07, 3 August 2018

3,508 bytes added ,  3 August 2018
Mark outdated
(Mark outdated)
 
(24 intermediate revisions by 11 users not shown)
Line 1: Line 1:
{{outdated}}
The DevTools JS code base is quite large, and a lot of different people contribute to it all the time, so it's important that a set of standards be shared when coding so that the code is consistent, written in a predictable style that also hopefully helps avoid common mistakes.
The DevTools JS code base is quite large, and a lot of different people contribute to it all the time, so it's important that a set of standards be shared when coding so that the code is consistent, written in a predictable style that also hopefully helps avoid common mistakes.


Line 9: Line 11:
* Learn more [http://eslint.org/ about ESLint],
* Learn more [http://eslint.org/ about ESLint],
* learn how to [http://eslint.org/docs/user-guide/integrations integrate ESLint in your code editor],
* learn how to [http://eslint.org/docs/user-guide/integrations integrate ESLint in your code editor],
* or [http://eslint.org/docs/user-guide/command-line-interface run it on the command line]. Note that ESLint has been integrated to our build system and can therefore be run on the command line with <code>./mach eslint</code>.
* or [http://eslint.org/docs/user-guide/command-line-interface run it on the command line]. Note that ESLint has been integrated to our build system and has some Mozilla specific plugins so it should always be run from the command line using <code>./mach eslint</code>.
 
Our ESLint rule set is meant to be used with ESLint 3.5.0.


Our ESLint rule set is meant to be used with ESLint 1.10.3 at the moment, and is not compatible with ESLint 2.0.0.
The main rule set is defined in <code>./devtools/.eslintrc</code>.


=== Installing ESLint and Recommended plugins ===
=== Installing ESLint and Recommended plugins ===
Line 18: Line 22:
<code>./mach eslint --setup</code>
<code>./mach eslint --setup</code>


'''NOTE:''' If you are not an administrator you either need to use an administrator account or use sudo depending on your operating system.
ESLint, eslint-plugin-html, eslint-plugin-mozilla and eslint-plugin-react will be automatically downloaded and installed.
 
ESLint, eslint-plugin-mozilla and eslint-plugin-react will be automatically downloaded and installed.


=== Running ESLint from the command line ===
=== Running ESLint from the command line ===
Line 49: Line 51:
* make sure to configure SublimeLinter with the <code>--no-ignore</code> option so that errors are also shown for source files that are ignored. To do this, open the SublimeLinter user configuration at: Preferences / Package Settings / SublimeLinter / Settings - User, and add <code>"args": "--no-ignore"</code> to the eslint linter object.
* make sure to configure SublimeLinter with the <code>--no-ignore</code> option so that errors are also shown for source files that are ignored. To do this, open the SublimeLinter user configuration at: Preferences / Package Settings / SublimeLinter / Settings - User, and add <code>"args": "--no-ignore"</code> to the eslint linter object.


Once done, open the mozilla project in SublimeText and open any JS file in either <code>/browser/devtools</code> or <code>/toolkit/devtools</code> (the 2 main directories for DevTools code). You can then trigger the linter via the contextual menu (right click on the file itself) or with a keyboard shortcut (ctrl+option+L on Mac).
You will also need to point SublimeLinter at the local eslint installation by setting the path to whatever <code>./mach eslint --setup</code> gives you when you run it (include a trailing slash but remove the eslint binary filename) e.g.
<code>
    NOTE: Your local eslint binary is at
          /some-project-path/node_modules/.bin/eslint
 
    "paths": {
        "linux": [],
        "osx": [
            "/some-project-path/node_modules/.bin"
        ],
        "windows": [
            "C:\\some-project-path\\node_modules\\.bin"
        ]
    },
</code>
 
Once done, open the mozilla project in SublimeText and open any JS file in the <code>/devtools</code> directory. You can then trigger the linter via the contextual menu (right click on the file itself) or with a keyboard shortcut (ctrl+option+L on Mac).


You should see errors and warnings in the gutter as shown in the screenshot below. You can also see all errors listed with ctrl+option+A, and individual errors by clicking in the gutter marker.
You should see errors and warnings in the gutter as shown in the screenshot below. You can also see all errors listed with ctrl+option+A, and individual errors by clicking in the gutter marker.
Line 59: Line 77:
* First, install the flycheck package (flymake doesn't support ESLint yet).  You can get flycheck from the [https://marmalade-repo.org/ marmalade] or [http://stable.melpa.org/#/ melpa-stable] repositories.
* First, install the flycheck package (flymake doesn't support ESLint yet).  You can get flycheck from the [https://marmalade-repo.org/ marmalade] or [http://stable.melpa.org/#/ melpa-stable] repositories.


* Tell flycheck to disable jslint, and enable flycheck in your javascript mode.  This snippet assumes the built-in javascript mode, but with minor changes (just the name of the hook) should work fine with js2-mode as well:
* Tell flycheck to disable jslint, and enable flycheck in your javascript mode.  Some care is needed to find the eslint installed in the source tree.  This snippet assumes the built-in javascript mode, but with minor changes (just the name of the hook) should work fine with js2-mode as well:
  <nowiki>
  <nowiki>
(defun my-js-mode-hacks ()
(defun my-js-mode-hacks ()
  (setq-local mode-name "JS")
   ;; Set this locally so that the head.js rule continues to work
   ;; Set this locally so that the head.js rule continues to work
   ;; properly.  In particular for a mochitest we want to preserve the
   ;; properly.  In particular for a mochitest we want to preserve the
Line 68: Line 87:
     (let ((base (file-name-nondirectory (buffer-file-name))))
     (let ((base (file-name-nondirectory (buffer-file-name))))
       (when (string-match "^\\([a-z]+_\\)" base)
       (when (string-match "^\\([a-z]+_\\)" base)
(setq-local flycheck-temp-prefix (match-string 1 base)))))
(setq-local flycheck-temp-prefix (match-string 1 base))))
    (let ((base-dir (locate-dominating-file (buffer-file-name)
    ".eslintignore")))
      (when base-dir
(let ((eslint (expand-file-name
      "tools/lint/eslint/node_modules/.bin/eslint" base-dir)))
  (when (file-exists-p eslint)
    (setq-local flycheck-javascript-eslint-executable eslint))))))
   (flycheck-mode 1))
   (flycheck-mode 1))
(require 'flycheck)
(require 'flycheck)
Line 77: Line 103:


* flycheck puts its bindings on <code>C-c !</code> by default, so use <code>C-c ! C-h</code> to see what is available.  There are key bindings to list all the errors and to move through the errors, among other things.
* flycheck puts its bindings on <code>C-c !</code> by default, so use <code>C-c ! C-h</code> to see what is available.  There are key bindings to list all the errors and to move through the errors, among other things.
* To make sure flycheck is finding eslint, open a .js file and run M-x flycheck-verify-setup. It should show the path to your eslint installation.
=== Running ESLint in Atom ===
From the root of the project type:
<code>./mach eslint --setup</code>
Install the [https://atom.io/packages/linter-eslint linter-eslint] package v.8.00 or above. Then go to the package settings and enable the following options:
[[File:Eslint-atom-settings.png|651px|linter-eslint settings in Atom]]
Once done, you should see errors and warnings as shown in the screenshot below.
[[File:Eslint-atom.png|651px|ESLint in Atom]]
=== Running ESLint in ViM ===
If you don't use Syntastic yet, the instructions here should get you started: https://wiki.mozilla.org/WebExtensions/Hacking#Vim
Alternatively, if you do use Syntastic already, add this to your <code>.vimrc</code> to get ESLint working where the path contains <code>mozilla-central</code> (adjust the path to reflect the one in your computer):
<code>
autocmd FileType javascript,html
    \ if stridx(expand("%:p"), "/mozilla-central/") != -1 |
    \    let b:syntastic_checkers = ['eslint'] |
    \    let b:syntastic_eslint_exec = '/path/to/mozilla-central/tools/lint/eslint/node_modules/.bin/eslint' |
    \    let b:syntastic_html_eslint_args = ['--plugin', 'html'] |
    \ endif
</code>
You probably need to close and reopen ViM for the changes to take effect. Then, open any file and try to edit it to cause an error, then save it. If all goes well, you will get some distinctive arrows pointing to the error. Hovering with the mouse will produce a sort of tooltip with more information about the error.
=== Running ESLint in Visual Studio Code ===
From the root of the project type:
<code>./mach eslint --setup</code>
Install the [https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint dbaeumer.vscode-eslint] package. Then go to the package settings and set the following option:
<code>"eslint.nodePath": "tools/lint/eslint/node_modules/.bin"</code>
Once done, you should see errors and warnings as shown in the screenshot below.
[[File:Vscode-eslint.png|600px|ESLint in VS Code]]


=== Getting Rid of ESLint Errors ===
=== Getting Rid of ESLint Errors ===
Line 96: Line 168:
The primary source for the style guide is the [https://dxr.mozilla.org/mozilla-central/source/devtools/.eslintrc .eslintrc]; but for quick reference here are some of the main code style rules:
The primary source for the style guide is the [https://dxr.mozilla.org/mozilla-central/source/devtools/.eslintrc .eslintrc]; but for quick reference here are some of the main code style rules:


* file references to browser globals such as <code>window</code> and <code>document</code> need <code>/* eslint-env browser */</code> at the top of the file
* file references to browser globals such as <code>window</code> and <code>document</code> need <code>/* eslint-env browser */</code> at the top of the file,
* lines should be 80 characters maximum (go longer if wrapping hurts readability),
* lines should be 90 characters maximum,
* indent with 2 spaces (no tabs!),
* indent with 2 spaces (no tabs!),
* <code>camelCasePlease</code>,
* <code>camelCasePlease</code>,
* don't open braces on the next line,
* don't open braces on the next line,
* don't name function expressions: <code>let o = { doSomething: function doSomething() {} };</code>,
* don't name function expressions: <code>let o = { doSomething: function doSomething() {} };</code>,
* use a space before opening paren for anonymous functions, but don't use one for named functions:
** anonymous functions: <code>function () {}</code>
** named functions: <code>function foo() {}</code>
** anonymous generators: <code>function* () {}</code>
** named generators: <code>function* foo() {}</code>
* aim for short functions, 24 lines max (ESLint has a rule that checks for function complexity too),
* aim for short functions, 24 lines max (ESLint has a rule that checks for function complexity too),
* aArguments aAre the aDevil (don't use them please),
* aArguments aAre the aDevil (don't use them please),
Line 135: Line 212:
* A global comment at the very top of a file explaining what the file is about and the major types/classes/functions it contains is a good idea for quickly browsing code.
* A global comment at the very top of a file explaining what the file is about and the major types/classes/functions it contains is a good idea for quickly browsing code.
* If you are forced to employ some kind of hack in your code, and there's no way around it, then add a comment that explains the hack and why it is needed. The reviewer is going to ask for one anyway.
* If you are forced to employ some kind of hack in your code, and there's no way around it, then add a comment that explains the hack and why it is needed. The reviewer is going to ask for one anyway.
* Bullet points in comments should use stars aligned with the first comment to format each point
<code><pre>
// headline comment
// * bullet point 1
// * bullet point 2
</pre></code>


== Asynchronous Code ==
== Asynchronous Code ==
Line 154: Line 237:
=== Components ===
=== Components ===
* Default to creating components as [https://facebook.github.io/react/docs/reusable-components.html#stateless-functions stateless function components].  
* Default to creating components as [https://facebook.github.io/react/docs/reusable-components.html#stateless-functions stateless function components].  
* If you need local state or lifecycle methods, use `React.createClass` instead of functions.
* If you need local state or lifecycle methods, use <code>React.createClass</code> instead of functions.
* Use React.DOM to create native elements. Assign it to a variable named `dom`, and use it like `dom.div({}, dom.span({}))`. You may also destructure specific elements directly: `const { div, ul } = React.DOM`.
* Use React.DOM to create native elements. Assign it to a variable named <code>dom</code>, and use it like <code>dom.div({}, dom.span({}))</code>. You may also destructure specific elements directly: <code>const { div, ul } = React.DOM</code>.


=== PropTypes ===
=== PropTypes ===
* Use [https://facebook.github.io/react/docs/reusable-components.html#prop-validation PropTypes] to define the expected properties of your component. Each directly accessed property (or child of a property) should have a corresponding PropType.
* Use [https://facebook.github.io/react/docs/reusable-components.html#prop-validation PropTypes] to define the expected properties of your component. Each directly accessed property (or child of a property) should have a corresponding PropType.
* Use isRequired for any required properties.
* Use <code>isRequired</code> for any required properties.
* Place the propTypes definition at the top of the component. If using a stateless function component, place it above the declaration of the function.
* Place the propTypes definition at the top of the component. If using a stateless function component, place it above the declaration of the function.
* Where the children property is used, consider [http://www.mattzabriskie.com/blog/react-validating-children validating the children].
* Where the children property is used, consider [http://www.mattzabriskie.com/blog/react-validating-children validating the children].
Digitarald
Confirmed users
45

edits

Retrieved from "https://wiki.mozilla.org/Special:MobileDiff/1122898...1198411"