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.claude/skills/typescript/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".
PositionTargetscarriespointsandanchorsalways, so readers writetargets.points, nottargets.points ?? [], and asktargets.hasAnchors. Keep an optional collection only where absence differs from empty (an option that falls back to a default, a wire field mirroring Rust'sOption<Vec<_>>), and say why in the disable comment. Enforced byshift/no-optional-collection-field. - Lookups return
T | null, neverundefined. Hold id-keyed collections inEntityMapfrom@shift/types:getreturnsnull,requirethrows a namedEntityNotFoundErrorwhen absence is a bug for that caller, andwhere(predicate)filters in insertion order. Expose indexes asReadonlyEntityMap. Enforced byshift/no-map-get-to-null;no-mutable-collection-fieldtreatsEntityMapfields likeMapfields. - 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 writingfont.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 tosource(id)) and point every caller at it. Unsure → Rust.scan:ts --kind model-rulelists model methods that compute without the engine;--kind repeated-filterlists 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 readlist[0]without a fallback. - Use the built-ins before spreading. Query an iterator directly (
map.values().filter(…).toArray();.some/.every/.findneed no array), copy-sort withtoSorted, merge sets withunion/intersection/difference, group withMap.groupBy. Enforced byshift/prefer-iterator-helpers. - A combined collection used in several places is a type.
ThemeCatalogbuilds the shipped-then-user theme list once;KerningGroups.atPositionsorts 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 byshift/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)andorderBy(...)from@shift/types,byAxis(axis)from@shift/geo, or a comparator named next to the type it orders (newestFirst,byMappingInput(axis)). Enforced byshift/no-inline-comparator. - Match typed names through
NameQuery. A search field's text becomesNameQuery.from(text)(trimmed; an empty query matches everything;{ prefix: "@" }drops a typed marker), and the list asksquery.filter(items),query.matches(item)orquery.filterBy(items, (item) => [item.displayName]), neverx.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 acomputed.
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
RecentListfrom@shift/types:of(items, rules),find,touch,update,remove,where. - Keyed lists use
upsertBy,replaceByandremoveByfrom@shift/typesinstead offindIndex, copy and assign, ormap((x) => (x.id === id ? f(x) : x)). A domain rule beyond the key gets a named function, such aswithIndependentMappingin@shift/editor/variation. - Sets in React state use
useMemberSet, never a copiedSettoggled 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) ?? nulliseditor.layerForGlyph(id, sourceId). Add the method to the owner, next to the data, and use it everywhere. - When the same
filter/findappears 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 byshift/prefer-scene-only. - State has one owner the window or session holds. A module-level
WeakMap<Editor, …>reached throughwindowTabs(editor)is a hidden global: nothing says who creates it or when it goes. Construct it where its lifetime starts (the window, the session, theEditor) 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 (
wrapIndexin@shift/geo, enforced for cyclic indexing byshift/no-manual-index-wrap).
Extensible kinds
- Kinds that packages may add are open registries, not closed unions.
SceneNodeKindsandNodeDefinitionByKindare interfaces;ShiftNode,NodeKindandNodeOfKind<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; outsidelib/nodes/, avoidnode.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, …), neveras PointId(enforced byshift/brand-cast-via-helper). Locations useasExternalAxisLocation/asDesignAxisLocation. - Parse at the boundary. Data from IPC, MCP, JSON or
localStorageisunknownuntil 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 noas never. - Make switches exhaustive. End a
switchon a union withdefault: 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 inpackages/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 writesrun.features?.[tag]. If the data has never shipped, write no migration and no legacy branch (ensurePagere-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 oneequalson the type next to its fields.
Review checklist
- No optional collection field without a reason in its disable comment.
- No
Map.get(…) ?? nullon a class's own collection; id-keyed collections areEntityMap. - No
nodesOfKind(kind)[0],const [node] = nodesOfKind(kind)ornodesOfKind(kind).length === 1. - No
?.chain across two objects where the owner could answer; nofilter/findrepeated across files. - No
[...x.values()].filter(…), no[...array].sort(…)wheretoSortedfits. - 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
switchon a union is exhaustive. - No TypeScript fallback, preview path or
WORKAROUNDthat 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 typecheckand the affected tests pass.