Skip to content

Coding Style

Jump to bottom
Richard Gibson edited this page Jul 3, 2024 · 53 revisions

Our best-practices and guidelines include:

  cond || Fail`complaint`;

over

  assert(cond, X`complaint`);

See also: issues labelled code-style.

Code comments

Try to write code that makes clear what it does without comments. Of course comments often make the code more clear or give helpful context about history and patterns.

We use JSDoc with Typescript extensions so that the types written in comments are checked statically.

We also have several codes for kinds of comments,

  • TODO notes something to come back and do. It's best to file an issue and link it so we can track it getting done.
  • FIXME notes something that is broken that needs fixing.
  • XXX acknowledges some tech debt. It's not broken and doesn't need to be tracked as something to change, but it should not be emulated.
  • UNTIL specifies a blocking issue.
  • HACK acknowledges it's not pretty but doesn't recommend changing it.

Using issue links helps us to keep the comments in sync with issue tracking. Whereas a link in TODO or FIXME is for making a change at the comment site, UNTIL means indicates what issue is blocking an improvement.

vs-code eslint plugin flakiness

where to put this? under eslint, vs-code, or other?

symptoms:

[Error - 11:42:15 AM] ESLint stack trace:
[Error - 11:42:15 AM] Error: Failed to load config "@endo" to extend from.

treatment: restart the eslint plug-in:

  • Command palette (Ctrl + P), "Developer: reload window", or
  • quit and restart vs-code

background: #3355

Modules

Module system emulation and version

Agoric SDK and most dapps use Node.js modules (NESM). Some dapps still use an emulator we have deprecated and call RESM (because of the node -r esm invocation).

To migrate from RESM to NESM, a package must use the Agoric fork of esm, like "esm": "agoric-labs/esm#Agoric-built" or "resolutions": {"**/esm": "agoric-labs/esm#Agoric-built"} to be able to import from NESM-migrated packages. This is because of these mutually exclusive constraints:

  • Modules in a NESM package should have a "type": "module" property in package.json, and
  • Modules in a RESM package must not have a "type": "module" property in their package.json on Node.js 12 or newer.

Published versions of esm can only import other RESM packages, transitively.

A RESM package will also have -r esm in any scripts, a "ava": {"require": ["esm"]} directive, and a dependency or dev-dependency on "esm". These must be removed.

Endo also is an emulator of ESM but is more faithful to Node.js. Endo does not yet support imports in package.json, but does support exports. Consequently, both Node and Endo can enforce which modules are public. For portability with other tools (those that do not support exports), all of a package’s public API must be exported from either its main module or a top-level module mentioned in exports.

Consider a package that exports a platform-agnostic OCap feature and its types. It also incidentally exports an adapter/attenuator for Node.js.

Users can import:

import fs from 'node:fs';
import net from 'node:net';
import crypto from 'node:crypto';
import { makeFeature } from '@agoric/feature';
import { makeFeaturePowers } from '@agoric/feature/node-powers.js';

const powers = makeFeaturePowers({ fs, net, crypto });
/** @type {import('@agoric/feature').Delegate} */
const delegate = {};
const options = {};
const feature = makeFeature(powers, delegate, options);

The feature has this structure:

- package.json
- types.d.ts
- node-powers.js
- src/index.js
- src/types.js
- src/node-powers.js

To make the explicitly public modules addressable in Node, craft a suitable package.json:

{
  "name": "@agoric/feature",
  "main": "./src/index.js",
  "exports": {
    ".": "./src/index.js",
    "./node-powers.js": "./node-powers.js",
    "./package.json": "./package.json"
  }
}

TypeScript does not recognize the "exports" field in package.json, so no special treatment is necessary for types.js.

We prefer in most cases to have all substantial code in the src directory, so the public ./node-powers.js is a façade for ./src/node-pwoers.js:

export { makeFeaturePowers } from './src/node-powers.js';

Rules for module import specifiers

The following guidance applies to NESM packaging, and is stricter than RESM, which is a thin veneer over Node.js’s implementation of CommonJS.

  • All import specifiers within the same package are merely paths to the exact file, must have an extension, and must begin with ./ or ../ to distinguish them from specifiers for modules in other packages.
  • Any specifier that is the exact name of another package does not need an extension. The reference points, effectively, at package.json which must have a main property that points to an exact file with an extension.
  • Any other external specifier needs to have an extension.

The motivation behind the rules is that ESM is intended to be more suitable for direct use on the web, which means that ESM does not have a “search path”, won’t issue a sequence of HTTP requests until the response is not 404. It will only try one URL for each module specifier. That disqualifies old tricks like probing the extension (is there a .node DLL? is there a .js file?) or probing for name (if name is a file) or name/index (if name is a directory).

These rules are not exhaustive. Node.js ESM introduces the properties exports and imports in package.json that provide module aliases externally or internally to a package. When using exports, another package cannot reach for an arbitrary module in another package. There must be an alias, so packages can determine their own API surface and often provide better names, like @agoric/zoe/contract-support, which would not need an extension. The sensible convention going forward, after RESM is fully exorcised, would be that all external module specifiers are aliases and should not have extensions. We’re not there yet.

Importing platform / host / built-in modules

Use a platform prefix to import modules hosted by the platform:

import fs from 'node:fs';

For consistency with our OCap discipline, any module that imports a platform module, like node:fs, must be in a file that indicates the platform dependency in its name, like feature-node.js. A usage pattern that emerges from this constraint is the separation of a program into layers, like:

  • feature.js: a feature that receives all its platform capabilities by injection of platform-agnostic powers. At this layer, we strive to use idiomatic JavaScript features to model platform capabilities. For example, we would use async iterators for streams and use promises and delegate objects rather than the web’s EventTarget or Node.js EventEmitter,
  • feature-node-powers.js or feature-web-powers.js: provides a function that takes platform capabilities (like Node.js module objects), then attenuates (makes them hardened OCaps, often with less authority) and adapts (makes them platform-agnostic) and produces the powers expected by feature.js,
  • feature-node.js or feature-web.js: if necessary, a small stub that imports and combines feature.js and feature-{platform}-powers.js. This is useful for shell scripts.

Function naming guide

Functions that return iterators or async iterators (i.e., generator functions) should be named like generateFoo, and the "generate" prefix should be reserved for such functions.

See Method Naming Structure for an explanation of our distinction between makeFoo(), createFoo(), getFoo(), makeFooKit(), and provideFoo().

todo

  • emphasize Jessie; e.g. objects-as-closures over class
    • identify code not intended to be written in Jessie (@erights Jun 8)
  • CONTRIBUTING.md : move to or cite from
  • github PR template : move / copy to or cite from
  • care with interleaving points: "The hazard is that, when both writing and reading a program, especially when reading other people's programs, an explicit await is not salient enough for people to understand that there's a turn boundary there, and that arbitrary turns interleave at that point, invalidating stateful assumptions." -- erights Apr 2020
  • exceptions are not for flow control (or "errors are not normal API" or something like that)
Clone this wiki locally