DXOS Code Style
Authoring conventions for the DXOS monorepo. The always-on subset lives in
AGENTS.md; this skill holds the detail. Use @dxos/echo as the reference
implementation for the namespace-export pattern.
Casts — fix the type at its source
Do NOT cast to silence a build error; fix the type where it originates.
- "Cast" means
as T, as any, as unknown as T, non-null !, or a widened /
any signature added to silence a type error. as const is NOT a cast — it
narrows a literal rather than bypassing the checker, and is always acceptable
(no comment or justification needed).
- Default: fix the type at its source (inference, signature, generic), not the
call site that surfaced the error. A red typecheck during a refactor is a
finding, not an obstacle to paper over.
- Casts are only acceptable at genuine type-system boundaries (external / untyped
data, deliberate coercions), and must carry a concise comment saying why no
typed alternative exists.
- Before every commit/PR, audit your diff for new casts:
git diff origin/main | grep -nE '\bas (any|unknown|[A-Z])|as unknown as'.
Justify or remove each; do not defer to review.
- Casts accumulate fastest during large codemods — treat each as a deliberate
decision, never an autopilot stopgap.
- Use @dxos/util
trim to create multi-line strings (e.g., prompts).
Unhandled errors — surface, don't suppress
Unhandled errors and rejections are findings; surface them and fix the root
cause. Do NOT silence them to make a suite go green.
- Never set
dangerouslyIgnoreUnhandledErrors: true in any vitest config
(shared vite.base.config.ts or a per-package config). It lets a run exit 0
despite unhandled rejections, hiding real failures — including the teardown
races it is usually reached for. It has slipped in before; keep it out.
- A flaky teardown rejection (e.g. vitest worker rpc closing while a console log
is pending, browser/birpc close races) is a bug to fix at its source — close
the resource, await the pending work, or disable what leaks — not to blanket-
ignore. If a specific, understood signature must be tolerated, filter exactly
it via
onUnhandledError (returning false only for that case) so every other
unhandled error stays fatal; never widen that to all errors.
- Same rule for a swallowed
unhandledRejection handler, an empty .catch() on
real work, and a blanket try/catch that drops the error — handle or rethrow.
Comments — say why, once
A comment earns its place by stating the why the code can't — the constraint,
the non-obvious consequence, the reason a reader would otherwise "fix" it wrong.
The code already says what it does; a comment that restates that is noise.
- One load-bearing clause. Not a multi-sentence essay. If you're explaining a
mechanism, state the constraint in a sentence and stop — resist narrating each
step, the alternatives you rejected, or how it used to work. This applies
hardest to JSDoc on a new abstraction, where the instinct to over-explain peaks.
- Line-count is itself a signal. If a comment runs past ~2 lines, that length
is almost never buying explanatory power proportional to its size — it's
restating what a competent reader already infers from the code, the variable
names, or the fact that it's test/fixture scaffolding. Compress to the one
clause a reader couldn't get any other way, or delete outright. A correct but
low-stakes "why" (this is a toolkit stub, this layer is a noop) does not
justify three sentences of scene-setting — the reader can see it's a stub.
- Test/fixture code gets a lower comment bar, not a higher one: "this is a
minimal/fake X for testing Y" is exactly what the surrounding
describe
block, filename, and variable names (TestToolkit, *LayerNoop,
scripted*) already communicate — restating it in prose adds nothing.
- Never narrate history or the conversation ("previously X, now Y", "as
requested", "changed to…"). State the current invariant as if it always was.
- Delete a comment that a competent reader gets from the code itself. Prefer a
clearer name or signature over a comment that compensates for an unclear one.
- End with a period. JSDoc public functions.
- Before every commit/PR, audit the comments in your diff:
git diff origin/main | grep -nE '^\+\s*(//|\*|/\*)'. Re-read each added line
and cut it to its load-bearing clause — or delete it. A verbose comment is the
autopilot default; conciseness is the deliberate pass. Do not defer to review.
A comment that survives your own audit deserves a second look — try
deleting it first, and keep only words that don't come back on their own
from re-reading the code.
// ✅ why the code can't be the obvious thing, in one clause:
// Seeded across an identity change so the view refreshes in place, not to empty.
// ❌ restates the code / multi-sentence narration / history:
// Set displayItems to initialItems if it has items, otherwise the empty array.
// We used to reset here but that flashed empty, so now we hold the previous page
// and only replace it once the new query delivers its own results, which means…
// ❌ correct but over-explained test scaffolding — the name/context already says this:
// A minimal echo tool: the deterministic developer code the loop invokes when the
// (scripted) model emits a tool call. Its handler runs for real, so a genuine
// tool-call → result → continue cycle is exercised without any live model.
const TestToolkit = Toolkit.make(Tool.make('Echo', { ... }));
// ✅ same fact, one clause, or just delete it and let the name carry it:
// Real handler, so tool-call → result → continue is a genuine cycle, not a mock.
const TestToolkit = Toolkit.make(Tool.make('Echo', { ... }));
Namespace-export packages
Packages are increasingly organized as namespace exports. Modules have
capital-case names and are re-exported as namespaces:
src/
Foo.ts
Bar.ts
errors.ts
index.ts
testing/
index.ts
internal/
foo.ts
bar.ts
baz.ts
// index.ts
export * as Foo from './Foo';
export * as Bar from './Bar';
export * from './errors';
// Foo.ts
// @import-as-namespace
export const
export const two = 2;
export const func: {
(a: string): number;
(a: number): string;
} = (a) => {
return a;
};
- The
@import-as-namespace linter directive marks a file as a namespace export.
- Internal code is hidden in
internal/, which is not exported.
testing/ and errors.ts are the exceptions (exported directly).
- For a namespace file, avoid prefixing top-level types with the namespace name —
inside
Foo.ts prefer Manager, Service, Options over FooManager,
FooService, FooOptions (callers see Foo.Manager either way).
Internal module imports
For @dxos/echo-style entrypoints importing src/internal/<Module>/: import the
capitalized internal barrel as a lowercase *Internal namespace —
import * as objInternal from './internal/Obj',
import * as queryInternal from './internal/Query'. Do not deep-import
submodules (./internal/Obj/atoms, ./internal/Ref/ref, etc.); re-export needed
symbols from the module's index.ts instead. The top-level ./internal barrel is
for cross-cutting re-exports only — prefer the per-module barrel when a single
entrypoint owns the dependency. Atom factories inside internal modules use the
makeAtom name (not make) to avoid clashing with public make APIs.
Types and signatures
Class member ordering
Consider: static fields → public readonly → public mutable → private readonly
(incl. constructor-injected) → private mutable → constructor → public methods →
private methods. Within each group, rank properties roughly most-important to
least — "further up the stack" (closer to public API), required over optional,
readonly over mutable.
Testing
- Place tests near modules as
module.test.ts. Use vitest with describe /
test (not it); prefer test('foo', ({ expect }) => ...).
- Prefer extending existing test suites over creating new ones. Look for a
suite that already covers the area before adding a file. A small number of
cohesive suites beats many fragmented ones.
- Test at the level that is naturally the public API. Exercise the seam
consumers actually use (exported surface, a service/manager's public methods),
not private internals. This keeps tests resilient to refactors and documents
real usage.
- Prefer a unified
TestLayer for all tests rather than one per test.
TestLayer(opts?) can be parametrized so tests configure it.
- Place test layer, configuration, and main definitions at the top of the suite;
helpers at the bottom.
- Never wrap an official API in a trivial local helper. A one-liner like
const makeBody = (text: string) => Obj.make(Body, { text }) renames the API
rather than removing duplication: the reader has to jump to the definition to
see what is under test, and several of them turn a suite into an ad-hoc DSL.
Inline the real call — Obj.make(Body, { text: 'x' }) — so the API being
exercised stays visible next to the assertion. A helper earns its place only
when it composes several calls or encodes a non-obvious setup sequence.
- Avoid sleep and polling. Use events and
TestClock instead.
1---2name: dxos-code-style3description: DXOS TypeScript authoring conventions. Use when writing or refactoring code — namespace-export packages, internal module imports, class member ordering, options-bag types, function overloads, the no-cast rule, the no-suppressing-unhandled-errors rule, the comment rule (say why, once), and test structure.4---56# DXOS Code Style78Authoring conventions for the DXOS monorepo. The always-on subset lives in9`AGENTS.md`; this skill holds the detail. Use `@dxos/echo` as the reference10implementation for the namespace-export pattern.1112## Casts — fix the type at its source1314Do NOT cast to silence a build error; fix the type where it originates.1516- "Cast" means `as T`, `as any`, `as unknown as T`, non-null `!`, or a widened /17 `any` signature added to silence a type error. `as const` is NOT a cast — it18 narrows a literal rather than bypassing the checker, and is always acceptable19 (no comment or justification needed).20- Default: fix the type at its source (inference, signature, generic), not the21 call site that surfaced the error. A red typecheck during a refactor is a22 finding, not an obstacle to paper over.23- Casts are only acceptable at genuine type-system boundaries (external / untyped24 data, deliberate coercions), and must carry a concise comment saying why no25 typed alternative exists.26- **Before every commit/PR**, audit your diff for new casts:27 `git diff origin/main | grep -nE '\bas (any|unknown|[A-Z])|as unknown as'`.28 Justify or remove each; do not defer to review.29- Casts accumulate fastest during large codemods — treat each as a deliberate30 decision, never an autopilot stopgap.31- Use @dxos/util `trim` to create multi-line strings (e.g., prompts).3233## Unhandled errors — surface, don't suppress3435Unhandled errors and rejections are findings; surface them and fix the root36cause. Do NOT silence them to make a suite go green.3738- **Never set `dangerouslyIgnoreUnhandledErrors: true`** in any vitest config39 (shared `vite.base.config.ts` or a per-package config). It lets a run exit 040 despite unhandled rejections, hiding real failures — including the teardown41 races it is usually reached for. It has slipped in before; keep it out.42- A flaky teardown rejection (e.g. vitest worker rpc closing while a console log43 is pending, browser/birpc close races) is a bug to fix at its source — close44 the resource, await the pending work, or disable what leaks — not to blanket-45 ignore. If a specific, understood signature must be tolerated, filter exactly46 it via `onUnhandledError` (returning `false` only for that case) so every other47 unhandled error stays fatal; never widen that to all errors.48- Same rule for a swallowed `unhandledRejection` handler, an empty `.catch()` on49 real work, and a blanket try/catch that drops the error — handle or rethrow.5051## Comments — say why, once5253A comment earns its place by stating the _why_ the code can't — the constraint,54the non-obvious consequence, the reason a reader would otherwise "fix" it wrong.55The code already says _what_ it does; a comment that restates that is noise.5657- **One load-bearing clause.** Not a multi-sentence essay. If you're explaining a58 mechanism, state the constraint in a sentence and stop — resist narrating each59 step, the alternatives you rejected, or how it used to work. This applies60 hardest to JSDoc on a new abstraction, where the instinct to over-explain peaks.61- **Line-count is itself a signal.** If a comment runs past ~2 lines, that length62 is almost never buying explanatory power proportional to its size — it's63 restating what a competent reader already infers from the code, the variable64 names, or the fact that it's test/fixture scaffolding. Compress to the one65 clause a reader couldn't get any other way, or delete outright. A correct but66 low-stakes "why" (this is a toolkit stub, this layer is a noop) does not67 justify three sentences of scene-setting — the reader can see it's a stub.68- Test/fixture code gets a **lower** comment bar, not a higher one: "this is a69 minimal/fake X for testing Y" is exactly what the surrounding `describe`70 block, filename, and variable names (`TestToolkit`, `*LayerNoop`,71 `scripted*`) already communicate — restating it in prose adds nothing.72- Never narrate history or the conversation ("previously X, now Y", "as73 requested", "changed to…"). State the current invariant as if it always was.74- Delete a comment that a competent reader gets from the code itself. Prefer a75 clearer name or signature over a comment that compensates for an unclear one.76- End with a period. JSDoc public functions.77- **Before every commit/PR**, audit the comments in your diff:78 `git diff origin/main | grep -nE '^\+\s*(//|\*|/\*)'`. Re-read each added line79 and cut it to its load-bearing clause — or delete it. A verbose comment is the80 autopilot default; conciseness is the deliberate pass. Do not defer to review.81 **A comment that survives your own audit deserves a second look** — try82 deleting it first, and keep only words that don't come back on their own83 from re-reading the code.8485```ts86// ✅ why the code can't be the obvious thing, in one clause:87// Seeded across an identity change so the view refreshes in place, not to empty.8889// ❌ restates the code / multi-sentence narration / history:90// Set displayItems to initialItems if it has items, otherwise the empty array.91// We used to reset here but that flashed empty, so now we hold the previous page92// and only replace it once the new query delivers its own results, which means…9394// ❌ correct but over-explained test scaffolding — the name/context already says this:95// A minimal echo tool: the deterministic developer code the loop invokes when the96// (scripted) model emits a tool call. Its handler runs for real, so a genuine97// tool-call → result → continue cycle is exercised without any live model.98const TestToolkit = Toolkit.make(Tool.make('Echo', { ... }));99100// ✅ same fact, one clause, or just delete it and let the name carry it:101// Real handler, so tool-call → result → continue is a genuine cycle, not a mock.102const TestToolkit = Toolkit.make(Tool.make('Echo', { ... }));103```104105## Namespace-export packages106107Packages are increasingly organized as namespace exports. Modules have108capital-case names and are re-exported as namespaces:109110```text111src/112 Foo.ts113 Bar.ts114 errors.ts115 index.ts116 testing/117 index.ts118 internal/119 foo.ts120 bar.ts121 baz.ts122```123124```ts125// index.ts126export * as Foo from './Foo';127export * as Bar from './Bar';128export * from './errors';129```130131```ts132// Foo.ts133134// @import-as-namespace135export const one = 1;136export const two = 2;137export const func: {138 (a: string): number;139 (a: number): string;140} = (a) => {141 return a;142};143```144145- The `@import-as-namespace` linter directive marks a file as a namespace export.146- Internal code is hidden in `internal/`, which is not exported.147- `testing/` and `errors.ts` are the exceptions (exported directly).148- For a namespace file, avoid prefixing top-level types with the namespace name —149 inside `Foo.ts` prefer `Manager`, `Service`, `Options` over `FooManager`,150 `FooService`, `FooOptions` (callers see `Foo.Manager` either way).151152### Internal module imports153154For `@dxos/echo`-style entrypoints importing `src/internal/<Module>/`: import the155capitalized internal barrel as a lowercase `*Internal` namespace —156`import * as objInternal from './internal/Obj'`,157`import * as queryInternal from './internal/Query'`. Do not deep-import158submodules (`./internal/Obj/atoms`, `./internal/Ref/ref`, etc.); re-export needed159symbols from the module's `index.ts` instead. The top-level `./internal` barrel is160for cross-cutting re-exports only — prefer the per-module barrel when a single161entrypoint owns the dependency. Atom factories inside internal modules use the162`makeAtom` name (not `make`) to avoid clashing with public `make` APIs.163164## Types and signatures165166- Common suffix for constructor option-bag types is `Options` (e.g.167 `SpawnOptions`, `ManagerImplOptions`) — pick this over `Opts` / `Props` /168 `Config`.169- Keep React component Props types immediately before the component function.170- Take an options object when a constructor or function has more than a few171 readonly props, especially when several are optional or share a logical group.172- For exported functions with multiple overloads, declare them as `const` with173 the overload signatures inline in the type annotation rather than174 `export function` with repeated declarations:175 ```ts176 export const myFn: {177 <T extends Foo>(a: T): Bar<T>;178 (a: string): Bar<any>;179 } = (a): Bar<unknown> => { ... };180 ```181182## Class member ordering183184Consider: static fields → public readonly → public mutable → private readonly185(incl. constructor-injected) → private mutable → constructor → public methods →186private methods. Within each group, rank properties roughly most-important to187least — "further up the stack" (closer to public API), required over optional,188readonly over mutable.189190## Testing191192- Place tests near modules as `module.test.ts`. Use vitest with `describe` /193 `test` (not `it`); prefer `test('foo', ({ expect }) => ...)`.194- **Prefer extending existing test suites over creating new ones.** Look for a195 suite that already covers the area before adding a file. A small number of196 cohesive suites beats many fragmented ones.197- **Test at the level that is naturally the public API.** Exercise the seam198 consumers actually use (exported surface, a service/manager's public methods),199 not private internals. This keeps tests resilient to refactors and documents200 real usage.201- Prefer a unified `TestLayer` for all tests rather than one per test.202 `TestLayer(opts?)` can be parametrized so tests configure it.203- Place test layer, configuration, and main definitions at the top of the suite;204 helpers at the bottom.205- **Never wrap an official API in a trivial local helper.** A one-liner like206 `const makeBody = (text: string) => Obj.make(Body, { text })` renames the API207 rather than removing duplication: the reader has to jump to the definition to208 see what is under test, and several of them turn a suite into an ad-hoc DSL.209 Inline the real call — `Obj.make(Body, { text: 'x' })` — so the API being210 exercised stays visible next to the assertion. A helper earns its place only211 when it composes several calls or encodes a non-obvious setup sequence.212- Avoid sleep and polling. Use events and `TestClock` instead.