typescript

Canonical rules for writing and reviewing TypeScript in Shift's packages and desktop app. Use whenever you add or change `.ts`/`.tsx` code that defines a domain type, a lookup by id, a collection fiel

SKILL.md

GitHub

/typescript — How TypeScript is written in Shift

TypeScript makes { points?: … }, ?. and ?? [] free to write, so a vague type never pays for itself; every caller does. An audit found 350 ?? null / ?? [] fallbacks, about 70 hand-written "take the first item" sequences and 95 calls that filtered the scene again for what they wanted. Each was a caller making up for a type or owner that did not answer its question. The rules below put the answer on the type, once.

The rule

When callers repeat the same fallback, filter or chain, the type they are reading is missing a method or is shaped wrong. Fix the type, then call it by name.

Collections

  • A collection field is never optional. An empty collection means "none". PositionTargets carries points and anchors always, so readers write targets.points, not targets.points ?? [], and ask targets.hasAnchors. Keep an optional collection only where absence differs from empty (an option that falls back to a default, a wire field mirroring Rust's Option<Vec<_>>), and say why in the disable comment. Enforced by shift/no-optional-collection-field.
  • Lookups return T | null, never undefined. Hold id-keyed collections in EntityMap from @shift/types: get returns null, require throws a named EntityNotFoundError when absence is a bug for that caller, and where(predicate) filters in insertion order. Expose indexes as ReadonlyEntityMap. Enforced by shift/no-map-get-to-null; no-mutable-collection-field treats EntityMap fields like Map fields.
  • Ask first: does Rust already compute this, or could another consumer need the same answer? Rust means shift-font and its libraries (fontc, fea-rs, fontdrasil, harfrust, line breaking); the other consumers are the CLI, export, the window's engine, the utility process, the SDK and agents. If yes, it has one implementation, in Rust, and every consumer calls it: expose it from shift-font through the engine and make the TypeScript method a one-line call (Kerning.between → engine.kerningRun; shaping → harfrust through the shaping module; feature compiling → fea-rs; interpolation → fontdrasil). Never write it in TypeScript "for now": two copies drift. If no (a lookup by id or name, a count, stored order, reactivity, UI state), the TypeScript model owns it, beside its siblings: before writing font.sources.find(({ name }) => name === n) outside the owner, look for the query there (Font.entryForName, recordForId, source(id), layerIdFor), and if none exists add it (Font.sourceNamed(name) next to source(id)) and point every caller at it. Unsure → Rust. scan:ts --kind model-rule lists model methods that compute without the engine; --kind repeated-filter lists predicates repeated across files.
  • A fallback, preview or workaround is still a copy. "Previews lay out as typed", "re-derive screen order by sorting x", "a feature applies when its override or default isn't off", "segment graphemes with Intl.Segmenter": each is a second rule beside Rust's that drifts the first time Rust changes (RTL, ligatures, required features). When Rust's answer lacks a field TypeScript needs, add the field in Rust.
// Before: TypeScript rebuilds screen order Rust already computed (visual_order).
const onScreen = stops.toSorted((a, b) => a.x - b.x);
const applies = (tag: string) => (run.features[tag] ?? defaultOf(tag)) !== "off";

// After: Rust returns the order and the answer (fields to add there); TypeScript reads them.
const onScreen = layout.stopsInScreenOrder;
const applies = engine.featureApplies(run, tag);
  • Prove "at least one" in the type. When a list is never empty, type it readonly [T, ...T[]] and read list[0] without a fallback.
  • Use the built-ins before spreading. Query an iterator directly (map.values().filter(…).toArray(); .some/.every/.find need no array), copy-sort with toSorted, merge sets with union/intersection/difference, group with Map.groupBy. Enforced by shift/prefer-iterator-helpers.
  • A combined collection used in several places is a type. ThemeCatalog builds the shipped-then-user theme list once; KerningGroups.atPosition sorts once per change. Spread to build a value once, not at every read.
  • Pass a maybe-value straight through. Write x: v === undefined ? undefined : f(v); never spread conditionally to leave a key out, and don't add a helper function to build it. Where a stored shape must tell a missing key from an undefined one, keep the spread with a disable comment giving the reason. Enforced by shift/no-conditional-spread.

Order and subsets

A sort or filter written at a call site means the collection is missing a method. When an order is part of the domain (kerning groups by name, recent documents newest first), the owning type keeps it, and callers read it already ordered. When a subset has a name ("the external axes", "the root contours", "the one glyph node"), the owner exposes it, as a computed when it is reactive (see signals), so no caller filters.

// Before: every caller sorts and filters, each with its own inline comparator.
const groups = groupsById
  .values()
  .filter((group) => group.position === position)
  .toArray()
  .sort((a, b) => a.name.localeCompare(b.name));
const axes = useAxes().filter((axis) => axis.role === "external");

// After: the owners hold the order and the subset.
const groups = kerning.groups.atPosition(position); // Map.groupBy over toSorted(byName), once per change
const axes = useExternalAxes(); // font.externalAxesCell, a computed
  • Name every order. Pass a named comparator to sort/toSorted: ascending, descending, alphabetical, byName, comparing(key, order) and orderBy(...) from @shift/types, byAxis(axis) from @shift/geo, or a comparator named next to the type it orders (newestFirst, byMappingInput(axis)). Enforced by shift/no-inline-comparator.
  • Match typed names through NameQuery. A search field's text becomes NameQuery.from(text) (trimmed; an empty query matches everything; { prefix: "@" } drops a typed marker), and the list asks query.filter(items), query.matches(item) or query.filterBy(items, (item) => [item.displayName]), never x.name.toLowerCase().includes(lower).
  • Sort once, where the collection changes, not at every read. An immutable owner sorts in its constructor (KerningGroups); a reactive owner sorts in a computed.

List and set invariants

When an edit to a list or set must re-establish an invariant (unique, ordered, capped, keyed), the invariant belongs to a type or named function, not to each call site.

// Before: every edit filters, re-adds, sorts and slices to keep the list valid.
const remaining = entries.filter((entry) => !sameDocument(entry, visit));
entries = [{ ...visit, openedAt }, ...remaining].sort(newestFirst).slice(0, LIMIT);

// After: the list keeps one entry per document, newest first, capped.
entries = entries.touch(visit, (previous) => ({
  ...previous,
  ...visit,
  openedAt,
}));
  • Recency lists use RecentList from @shift/types: of(items, rules), find, touch, update, remove, where.
  • Keyed lists use upsertBy, replaceBy and removeBy from @shift/types instead of findIndex, copy and assign, or map((x) => (x.id === id ? f(x) : x)). A domain rule beyond the key gets a named function, such as withIndependentMapping in @shift/editor/variation.
  • Sets in React state use useMemberSet, never a copied Set toggled in each handler.
  • Stored JSON lists read through parseEach(schema, text), which keeps each valid entry and reads anything malformed as empty.

Owners answer questions

  • When a ?. chain crosses two objects, the first object is missing a method. editor.glyphForId(id)?.layerForSource(sourceId) ?? null is editor.layerForGlyph(id, sourceId). Add the method to the owner, next to the data, and use it everywhere.
  • When the same filter/find appears in two places, it becomes a method on the type that owns the collection: scene.only(kind) for "the one node of this kind" (null with none or several), scene.first(kind) for "the first in scene order". Enforced by shift/prefer-scene-only.
  • State has one owner the window or session holds. A module-level WeakMap<Editor, …> reached through windowTabs(editor) is a hidden global: nothing says who creates it or when it goes. Construct it where its lifetime starts (the window, the session, the Editor) and pass it down.
// Before
const windowTabsByEditor = new WeakMap<Editor, WindowTabs>();
export function windowTabs(editor: Editor): WindowTabs { … }

// After: the window creates its tabs and provides them.
const tabs = new WindowTabs(editor); // in the window's setup, disposed with it
  • Prefer a method or a small class over a free function whose first parameter is the domain object. A free function is right for pure numeric helpers with no owner (wrapIndex in @shift/geo, enforced for cyclic indexing by shift/no-manual-index-wrap).

Extensible kinds

  • Kinds that packages may add are open registries, not closed unions. SceneNodeKinds and NodeDefinitionByKind are interfaces; ShiftNode, NodeKind and NodeOfKind<K> derive from them, and a package registers a kind by declaration merging on @shift/editor/types.
  • Ask a definition what a node offers instead of switching on built-in kinds. Commands reach text through NodeDefinition.textItems, so a new kind works with select-all without editing it. Inside one definition a discriminated union is right; outside lib/nodes/, avoid node.kind === "glyph" where a capability would do.

Types

  • Brand values in one place. Mint or convert ids with the as<Brand> helpers from @shift/types (asPointId, asGlyphId, …), never as PointId (enforced by shift/brand-cast-via-helper). Locations use asExternalAxisLocation / asDesignAxisLocation.
  • Parse at the boundary. Data from IPC, MCP, JSON or localStorage is unknown until a zod schema or a type guard accepts it; derive the TypeScript type from the schema (z.input<typeof schema>) instead of writing an interface beside it. Never cast untrusted data.
  • Check tables with satisfies. const capabilities = { … } satisfies Record<string, CapabilityDefinition> checks every entry and keeps literal key types, so dispatch needs no as never.
  • Make switches exhaustive. End a switch on a union with default: assertNever(value), so a new member fails to compile at every switch that forgot it.
  • Never re-declare a generated shape. Import it from @shift/types; derive views in packages/types/src/domain.ts.
  • A new field on a stored record gets its default at the boundary, or no migration at all. If the record is ever read back from disk, IPC or localStorage, give the field a default in its schema, so an older record reads as complete and no caller writes run.features?.[tag]. If the data has never shipped, write no migration and no legacy branch (ensurePage re-parenting nodes "written before pages existed"); say in the PR that it is unpersisted.
// Before: older runs have no `features`; `run.features[tag]` throws on them.
interface TextRunRecord { glyphs: GlyphId[]; features: Record<string, FeatureSetting> }

// After: one schema, the default stated once.
const textRunRecord = z.object({
  glyphs: z.array(glyphIdSchema),
  features: z.record(z.string(), featureSetting).default({}),
});
  • Sameness belongs to the type. A hand-rolled deep-equal, or a field-by-field a.tag === b.tag && a.name === b.name && … in a caller, silently goes stale when a field is added and costs a walk on every run. Compare an identity or revision Rust already gives (the result changed iff its input revision did), or put one equals on the type next to its fields.

Review checklist

  • No optional collection field without a reason in its disable comment.
  • No Map.get(…) ?? null on a class's own collection; id-keyed collections are EntityMap.
  • No nodesOfKind(kind)[0], const [node] = nodesOfKind(kind) or nodesOfKind(kind).length === 1.
  • No ?. chain across two objects where the owner could answer; no filter/find repeated across files.
  • No [...x.values()].filter(…), no [...array].sort(…) where toSorted fits.
  • No inline comparator in sort/toSorted; no sort or named filter repeated at call sites where the owner could keep it.
  • No as <Brand>; no cast of untrusted data; no hand-written interface duplicating a zod schema or a generated type.
  • Every switch on a union is exhaustive.
  • No TypeScript fallback, preview path or WORKAROUND that decides what a Rust rule decides.
  • No module-level WeakMap<Editor, …> holding state.
  • Every new field on a stored record has a schema default, or the PR says the record is never persisted.
  • No hand-rolled deep or field-by-field equality outside the type it compares.
  • pnpm lint:check, pnpm typecheck and the affected tests pass.