Skip to content

FlowCrypt TypeScript Style Guide

Yan Takushevich edited this page Oct 19, 2020 · 14 revisions

The following style guide is current as of April 2019.

Also see FlowCrypt Project Structure and Overview

Preferred Syntax and TS patterns

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. (It also messes up stack traces unnecessarily)

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
+ }

Note: [].map(), [].filter(), [].find() and friends are ok, and they are especially encouraged if the result comfortably fits as a one-liner, eg const emails = contacts.map(contact => contact.email);

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)));

We put ifs in brackets. Inline ? : ifs are also encouraged.

- if (true)
-   doSomething();
- else
-   soemthingElse();
+ if (true) {
+   doSomething();
+ } else { // both brackets on the same line
+   soemthingElse();
+ }

// inline ? : expressions are engouraged if it increases brevity and stays readable
+ doSomething(condition ? someValue : otherValue)

// never ever do this, regardless how you format it. It's hard to read.
- doSomething(condition ? someValue : (otherConditioncondition ? otherValue : yetAnotherValue));

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);

Short object literals (2-3 items) should be kept on a single line.

- doSomething({
-   foo: 4,
-   bar: 5,
- });
+ doSomething({ foo: 4, bar: 5 });

Error handling

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.reportErr(err);.

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.

Rendering untrusted content

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 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.

If our linters and code checks have falsely marked a line of code as XSS vulnerability, you can put a comment next to it to silence the linter. Comments with their meanings are:

Comment Description
// xss-direct You have directly hard coded the value that is inserted into dom, therefore there is no chance of XSS. For example $('body').append('<iframe src="http://some-url.com" />');
// xss-sanitized You have already sanitized the content that you are inserting using Xss sanitizer
// xss-escaped You have carefully escaped any dynamic content that you are inserting with Xss.escape

Environment and project structure

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

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

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.

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).

we use .htm instead of .html

We enable vscode auto-format on each save. We use TSLint extension (the one from Microsoft) and the PhilHindle.errorlens extension. For all this to work, open flowcrypt-browser.code-workspace as a project (don't just import the folder without opening it through the project file).