Skip to content

Coding Style

Dan Connolly edited this page Dec 8, 2022 · 53 revisions

Our best-practices and guidelines include:

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.
  • HACK acknowledges it's not pretty but doesn't recommend changing it.

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

Module systems

Agoric SDK and most Dapps use Node.js modules (NESM).

Each package.json has properties that indicate whether it is using Node.js modules (NESM) or emulated modules using standardthings/esm, which we call RESM for short, since the invocation is node -r esm entrypoint.js. 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. A RESM package will also have -r esm in any scripts, a "ava": {"require": ["esm"]} directive, and a dependency or dev-dependency on "esm". A RESM 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. Published versions of esm can only import other RESM packages, transitively.

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.

Method Naming guide

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