Skip to content

FlowCrypt TypeScript Style Guide

Tom J edited this page Feb 2, 2019 · 14 revisions

The following style guide is current as of January 2019.

  1. Do not modify standard JS constructors such as Array, Object, etc unless you are very sure and have discussed this with @tomholub

  2. Shims are OK but generally discouraged, except when there is potential to use the benefits throughout the app

  3. When introducing new libraries, please notify & discuss with @tomholub. In particular, clear licensing is very important (MIT, Apache, BSD preferred). Please take care to find a small library with few dependencies. As a company, we are required to inform some of our customers every time we add a 3rd party library, so this cannot be taken lightly. Once a library has been selected, it should be included in lib/ as a single, UNminified file, and checked into the repo. Our repo follows a batteries-included approach when it comes to 3rd party code that we distribute with our own code.

  4. All imports should be done using ES6 import statement, except when not possible (some 3rd party libraries, in which case a html <script> tag will be used on appropriate htm pages).

  5. we use .htm instead of .html

  6. We do not use functions for looping. In particular, we never ever use .each nor $.each() because it's hard to return from inside .each. Since .each can't be used in all cases, and consistency matters, we never use .each.

const arr = ['a', 'b', 'c'];
const obj = { bar: 1, baz: 2 };

- arr.each((x, i) => console.info(x, 'whatnot'));
+ for(const val of arr) { // iterate over array values using OF keyword
+   console.info(val, 'whatnot');
+ }

- arr.each((val, i) => console.info(i, 'whatnot'));
+ for(const i in arr) { // iterate over array indexes using IN keyword
+   console.info(i, 'whatnot');
+ }

- $.each(obj, (name, value) => console.info(name, value));
+ for(const name of Object.keys(obj)) { // iterate over Object names using OF Object.keys(...)
+   console.info(name, obj[name]); // Do not use .hasOwnProperty
+ }

- $.each(obj, (name, value) => console.info(value));
+ for(const value of Object.values(obj)) { // iterate over Object names using OF Object.values(...)
+   console.info(value); // Do not use .hasOwnProperty
+ }
  1. We enable tscode auto-format on each save. We use tslint and make sure it's set up and functioning in our IDE.

  2. We use async and await. We do not use promises except when necessary. We do not use callbacks except when absolutely necessary. If a legacy 3rd party library works with callbacks, we create a promisified convenience method that we can await, such as

bark = (v) => new Promise((reject, resolve) => lib.bark(v, (err, res) => err ? reject(err) : resolve(res)));
  1. Error handling is done with try / catch and async/await. We should think very long and very hard before preferring x.catch(err => ...); over simply await x; (one exception - when we need to fire off a few network requests as the page is loading, and want the page to render before loading is done, such as the settings.htm page). Inside the catch block, you may want to differentiate between different AJAX errors using if(Api.err.isWhateverType(err)). Exceptions that we cannot categorize should be reported before showing the error to the user as follows: Catch.handleErr(err);.

  2. We use provided methods to handle exceptions that happen when handling DOM events. Example:

- $('#download').click(handleDownloadButtonClicked);
+ $('#download').click(Ui.event.handle(handleDownloadButtonClicked));

This way unexpected exceptions get reported automatically. We still have to make sure to inform the user that there was an error.

  1. We always prefer to destructure awaited object using ES6 syntax over using brackets.
- const key = (await readArmoredKey(armored)).keys[0];
+ const { keys: [key] } = await readArmoredKey(armored);
  1. We always, always, always use the Xss class to sanitize strings that may contain untrusted content before rendering them, or render directly using the Xss class. From the perspective of the extension, results from our own API are considered untrusted and care should be taken before rendering them.
- $('.button').html(content);  // you just pwned our users...
+ Xss.sanitizeRender('.button', content);

To decide which sanitization method to use, follow these steps:

  • if the content is not meant to contain HTML (error or response messages from API, usernames, email address, etc):
    • if you are rendering it, use $('.button').text(content); // safe
    • if you are passing it along and the result is expected to eventually end up being rendered, escape it using Xss.escape(content)
  • If the content is meant to contain HTML and it includes a any amount of untrusted content, render it using one of Xss.sanitizeRender, Xss.sanitizeAppend, Xss.sanitizePrepend, Xss.sanitizeReplace.
  • any method that renders one of its arguments and cannot sanitize contents (because it expects internally generated code that would have been stripped away, such as iframes), should be named with DANGEROUS or DANGEROUSLY.
- renderSomeContent = (html) => $('body').html(html);
+ renderSomeContent_DANGEROUSLY = (html) => $('body').html(html);
  • Anytime you are not sure, consult @tomholub. This is crucial to get right.