From aa8318d513712e8245e62786df09d19848069f21 Mon Sep 17 00:00:00 2001 From: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:19:12 -1000 Subject: [PATCH 1/2] Back unbacked ui tokens with a theme-aware default layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tokens.bgSubtle / fgFaint / borderStrong referenced CSS vars (--color-background-tertiary, --color-text-tertiary, --color-border-secondary) that no host injected, so they always resolved to their hardcoded light fallbacks — white-on-white surfaces, invisible borders, and illegible faint text whenever a host signaled dark. A var() fallback is a static literal that can't branch on theme, so the fix is a definition layer, not a smarter fallback. - Add theme-defaults.ts: a neutral default theme backing every color var the token contract references, in both light and dark. Applied to :root beneath the host's variables via a single shared applyThemeVariables() path, so host (brand) values still win and any omitted var resolves to a theme-correct neutral default. Works against an incomplete host, a standalone connect() widget, or a third-party host. - Replace the three near-duplicate inline injectors in core.ts, connect.ts, and with applyThemeVariables(mode, hostVars). - Make the preview harnesses (vite plugin host + preview server) inject the three vars in both themes and drop a duplicate --color-text-primary key. - Regression guard in tokens.test.ts: every color var a token references must be backed in both light and dark, with no dead defaults and light defaults equal to the token fallbacks. --- CHANGELOG.md | 13 ++++ package.json | 2 +- src/__tests__/ui/tokens.test.ts | 56 ++++++++++++++ src/connect.ts | 20 ++--- src/core.ts | 25 +++--- src/preview/server.ts | 28 ++++--- src/react/provider.tsx | 10 +-- src/theme-defaults.ts | 133 ++++++++++++++++++++++++++++++++ src/vite/plugin.ts | 14 ++-- 9 files changed, 246 insertions(+), 55 deletions(-) create mode 100644 src/theme-defaults.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3617a17..e374cf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/). +## [0.10.2] - 2026-06-23 + +Fixes three `ui` tokens that were theme-blind in dark mode. `tokens.bgSubtle`, `tokens.fgFaint`, and `tokens.borderStrong` reference CSS vars (`--color-background-tertiary`, `--color-text-tertiary`, `--color-border-secondary`) that no host injected, so they always resolved to their hardcoded **light** fallbacks — rendering white-on-white surfaces, invisible borders, and illegible faint text whenever a host signaled dark. This hit the SDK's own components (Card, ListRow, Prose, Table, Badge, Button, Avatar, SearchField, SegmentedControl, EmptyState, StatusDot) and any app pairing one of these with a theme-aware token. A `var()` fallback is a static literal that can't branch on theme, so the fix is a theme-aware default layer, not a smarter fallback. + +### Fixed + +- The SDK now ships a **neutral default theme** (`theme-defaults.ts`) that backs every color var the token contract references, in both light and dark. It's applied to `:root` beneath the host's variables — so the host's brand values still win for the keys it provides, and any var the host omits resolves to a theme-correct neutral default. The three previously-unbacked tokens now render correctly in dark mode against any host, a standalone `connect()` widget, or a third-party host. Values stay neutral (brand arrives only by host injection), preserving the library's host-agnostic design. +- The preview harnesses (`vite` plugin host + `preview` server) now inject the three vars in both themes, so `synapse dev`/`preview` matches a complete host, and a duplicate `--color-text-primary` key in those theme maps was removed. + +### Changed + +- Theme variables now reach the DOM through a single shared path, `applyThemeVariables(mode, hostVars)`, replacing three near-duplicate inline injectors in `core.ts`, `connect.ts`, and the React ``. No public API change. + ## [0.10.1] - 2026-06-19 Fixes a master/detail overlap. A too-wide child in `ListDetailLayout.List` — e.g. an auto-layout `` that won't shrink below its content — spilled out of the fixed-width list rail and painted over the detail pane. Surfaced dogfooding the People CRM, whose list rendered a 3-column table inside the 320px rail. diff --git a/package.json b/package.json index 8ae7372..0f5902e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nimblebrain/synapse", - "version": "0.10.1", + "version": "0.10.2", "description": "Agent-aware app SDK for the MCP ext-apps protocol", "type": "module", "exports": { diff --git a/src/__tests__/ui/tokens.test.ts b/src/__tests__/ui/tokens.test.ts index 827d1d4..912f39d 100644 --- a/src/__tests__/ui/tokens.test.ts +++ b/src/__tests__/ui/tokens.test.ts @@ -1,6 +1,13 @@ import { describe, expect, it } from "vitest"; +import { DEFAULT_THEME_VARS } from "../../theme-defaults.js"; import { headingStyle, textStyle, tokens } from "../../ui/tokens.js"; +/** Pull the `--var-name` out of a `var(--name, fallback)` reference. */ +function referencedVar(value: string): string | null { + const m = value.match(/^var\(\s*(--[a-zA-Z0-9-]+)\s*,/); + return m ? m[1] : null; +} + describe("token contract", () => { it("exposes tokens as host CSS var() references, not raw brand hex", () => { // Every color/font token must be a var() so the host's injected value wins @@ -32,3 +39,52 @@ describe("token contract", () => { }); }); }); + +describe("default theme backs the token contract", () => { + const referenced = Object.values(tokens) + .map(referencedVar) + .filter((v): v is string => v !== null); + + // A var is theme-sensitive iff it names a color. Theme-invariant vars (radii, + // type scale, shadows, fonts, weights, widths) look identical in both themes, + // so their static var() fallback is already correct — they are intentionally + // NOT backed by DEFAULT_THEME_VARS. + const colorVars = referenced.filter((v) => v.includes("color")); + + it("defines every color var a token references in BOTH light and dark", () => { + // The regression guard. A `var()` fallback is a static literal that fires + // exactly when the var is unset — the moment there's no theme signal to + // branch on — so an unbacked color token resolves to its light fallback in + // dark mode (white-on-white). This fails the instant someone adds a color + // token whose var nothing backs, instead of shipping it inert. + expect(colorVars.length).toBeGreaterThan(0); + for (const v of colorVars) { + expect(Object.keys(DEFAULT_THEME_VARS.light), `light missing ${v}`).toContain(v); + expect(Object.keys(DEFAULT_THEME_VARS.dark), `dark missing ${v}`).toContain(v); + } + }); + + it("light and dark define the same set of vars", () => { + expect(Object.keys(DEFAULT_THEME_VARS.light).sort()).toEqual( + Object.keys(DEFAULT_THEME_VARS.dark).sort(), + ); + }); + + it("ships no dead defaults — every default var is referenced by a token", () => { + const referencedSet = new Set(referenced); + for (const v of Object.keys(DEFAULT_THEME_VARS.light)) { + expect(referencedSet.has(v), `unreferenced default ${v}`).toBe(true); + } + }); + + it("light defaults equal the token fallbacks (no light-mode regression)", () => { + // The light layer must reproduce each color token's baked fallback, so + // introducing the default layer changes nothing observable in light mode. + for (const value of Object.values(tokens)) { + const v = referencedVar(value); + if (!v || !v.includes("color")) continue; + const fallback = value.slice(value.indexOf(",") + 1, -1).trim(); + expect(DEFAULT_THEME_VARS.light[v]).toBe(fallback); + } + }); +}); diff --git a/src/connect.ts b/src/connect.ts index 54233b4..04caf3f 100644 --- a/src/connect.ts +++ b/src/connect.ts @@ -27,6 +27,7 @@ import { parseToolResultParams } from "./content-parser.js"; import { resolveEventMethod } from "./event-map.js"; import { createResizer } from "./resize.js"; import { parseToolResult } from "./result-parser.js"; +import { applyThemeVariables } from "./theme-defaults.js"; import { SynapseTransport } from "./transport.js"; import type { App, ConnectOptions, Dimensions, Theme, ToolCallResult } from "./types.js"; @@ -106,8 +107,9 @@ export async function connect(options: ConnectOptions): Promise { containerDimensions = ctx.containerDimensions as Dimensions; } - // Inject host CSS variables into the DOM - injectCssVariables(ctx.styles?.variables as Record | undefined); + // Inject the theme into the DOM: neutral defaults for the mode back any + // var the host omits, then host values win. + applyThemeVariables(currentTheme.mode, currentTheme.tokens); } } @@ -124,7 +126,7 @@ export async function connect(options: ConnectOptions): Promise { ? (variables as Record) : currentTheme.tokens; currentTheme = { mode, tokens }; - injectCssVariables(tokens); + applyThemeVariables(mode, tokens); const set = handlers.get(HOST_CONTEXT_CHANGED_METHOD); if (set) { for (const handler of set) handler(currentTheme); @@ -269,15 +271,3 @@ export async function connect(options: ConnectOptions): Promise { return app; } - -// --- Helpers --- - -/** Inject CSS custom properties onto :root so widgets inherit host theming. */ -function injectCssVariables(vars: Record | undefined | null): void { - if (!vars || typeof vars !== "object") return; - for (const [k, v] of Object.entries(vars)) { - if (typeof k === "string" && typeof v === "string") { - document.documentElement.style.setProperty(k, v); - } - } -} diff --git a/src/core.ts b/src/core.ts index 6a5a04c..e26744c 100644 --- a/src/core.ts +++ b/src/core.ts @@ -24,6 +24,7 @@ import { detectHost, extractTheme } from "./detection.js"; import { KeyboardForwarder } from "./keyboard.js"; import { parseToolResult } from "./result-parser.js"; import { callToolAsTask as callToolAsTaskImpl, createTaskStatusRouter } from "./task-handle.js"; +import { applyThemeVariables } from "./theme-defaults.js"; import { SynapseTransport } from "./transport.js"; import type { AgentAction, @@ -124,9 +125,13 @@ export function createSynapse(options: SynapseOptions): Synapse { ? (rawTasks as TasksCapability) : undefined; - // Inject host CSS variables into :root so plain-CSS styles can consume - // them via `var(--…)` without needing to read theme.tokens imperatively. - injectCssVariables(extractTheme(currentHostContext).tokens); + // Inject the theme into :root so plain-CSS styles consume it via + // `var(--…)` without reading theme.tokens imperatively. Neutral defaults + // back any var the host omits (theme-correct), then host values win. + { + const theme = extractTheme(currentHostContext); + applyThemeVariables(theme.mode, theme.tokens); + } // Notify subscribers so React hooks (useTheme, useHostContext) and // custom listeners reflect the handshake-provided context (not just @@ -142,7 +147,8 @@ export function createSynapse(options: SynapseOptions): Synapse { // full snapshot of the host context, so we replace — never merge. const unsubHostContext = transport.onMessage(HOST_CONTEXT_CHANGED_METHOD, (params) => { currentHostContext = (params ?? {}) as McpUiHostContext; - injectCssVariables(extractTheme(currentHostContext).tokens); + const theme = extractTheme(currentHostContext); + applyThemeVariables(theme.mode, theme.tokens); for (const cb of hostContextCallbacks) cb(currentHostContext); }); @@ -462,14 +468,3 @@ function themesEqual(a: SynapseTheme, b: SynapseTheme): boolean { } return true; } - -/** Inject CSS custom properties onto :root so widgets inherit host theming. */ -function injectCssVariables(vars: Record | undefined | null): void { - if (!vars || typeof vars !== "object") return; - if (typeof document === "undefined") return; - for (const [k, v] of Object.entries(vars)) { - if (typeof k === "string" && typeof v === "string") { - document.documentElement.style.setProperty(k, v); - } - } -} diff --git a/src/preview/server.ts b/src/preview/server.ts index 10eb819..81b64a5 100644 --- a/src/preview/server.ts +++ b/src/preview/server.ts @@ -58,17 +58,19 @@ const HOST_HTML = (uiPort: number, serverPort: number) => ` // Minimal NimbleBrain bridge host — just enough to make Synapse work var tokens = darkMode ? { - "--color-background-primary": "#0f172a", "--color-text-primary": "#e2e8f0", - "--color-background-secondary": "#1e293b", "--color-text-primary": "#e2e8f0", + "--color-background-primary": "#0f172a", "--color-background-secondary": "#1e293b", + "--color-background-tertiary": "#2a374a", "--color-text-primary": "#e2e8f0", + "--color-text-secondary": "#94a3b8", "--color-text-tertiary": "#64748b", "--color-text-accent": "#6366f1", "--nb-color-accent-foreground": "#ffffff", - "--color-text-secondary": "#94a3b8", "--color-border-primary": "#334155", + "--color-border-primary": "#334155", "--color-border-secondary": "#475569", "--color-ring-primary": "#6366f1", "--nb-color-danger": "#ef4444", "--border-radius-sm": "0.5rem", } : { - "--color-background-primary": "#ffffff", "--color-text-primary": "#1a1a1a", - "--color-background-secondary": "#f9fafb", "--color-text-primary": "#1a1a1a", + "--color-background-primary": "#ffffff", "--color-background-secondary": "#f9fafb", + "--color-background-tertiary": "#f1f5f9", "--color-text-primary": "#1a1a1a", + "--color-text-secondary": "#6b7280", "--color-text-tertiary": "#94a3b8", "--color-text-accent": "#6366f1", "--nb-color-accent-foreground": "#ffffff", - "--color-text-secondary": "#6b7280", "--color-border-primary": "#e5e7eb", + "--color-border-primary": "#e5e7eb", "--color-border-secondary": "#cbd5e1", "--color-ring-primary": "#6366f1", "--nb-color-danger": "#ef4444", "--border-radius-sm": "0.5rem", }; @@ -147,17 +149,19 @@ const HOST_HTML = (uiPort: number, serverPort: number) => ` darkMode = !darkMode; document.body.style.background = darkMode ? "#0f172a" : "#f1f5f9"; tokens = darkMode ? { - "--color-background-primary": "#0f172a", "--color-text-primary": "#e2e8f0", - "--color-background-secondary": "#1e293b", "--color-text-primary": "#e2e8f0", + "--color-background-primary": "#0f172a", "--color-background-secondary": "#1e293b", + "--color-background-tertiary": "#2a374a", "--color-text-primary": "#e2e8f0", + "--color-text-secondary": "#94a3b8", "--color-text-tertiary": "#64748b", "--color-text-accent": "#6366f1", "--nb-color-accent-foreground": "#ffffff", - "--color-text-secondary": "#94a3b8", "--color-border-primary": "#334155", + "--color-border-primary": "#334155", "--color-border-secondary": "#475569", "--color-ring-primary": "#6366f1", "--nb-color-danger": "#ef4444", "--border-radius-sm": "0.5rem", } : { - "--color-background-primary": "#ffffff", "--color-text-primary": "#1a1a1a", - "--color-background-secondary": "#f9fafb", "--color-text-primary": "#1a1a1a", + "--color-background-primary": "#ffffff", "--color-background-secondary": "#f9fafb", + "--color-background-tertiary": "#f1f5f9", "--color-text-primary": "#1a1a1a", + "--color-text-secondary": "#6b7280", "--color-text-tertiary": "#94a3b8", "--color-text-accent": "#6366f1", "--nb-color-accent-foreground": "#ffffff", - "--color-text-secondary": "#6b7280", "--color-border-primary": "#e5e7eb", + "--color-border-primary": "#e5e7eb", "--color-border-secondary": "#cbd5e1", "--color-ring-primary": "#6366f1", "--nb-color-danger": "#ef4444", "--border-radius-sm": "0.5rem", }; diff --git a/src/react/provider.tsx b/src/react/provider.tsx index 62ddd35..80827e8 100644 --- a/src/react/provider.tsx +++ b/src/react/provider.tsx @@ -1,5 +1,6 @@ import { createContext, type ReactNode, useContext, useEffect, useRef, useState } from "react"; import { createSynapse } from "../core.js"; +import { applyThemeVariables } from "../theme-defaults.js"; import type { Synapse, SynapseOptions, SynapseTheme } from "../types.js"; const SynapseContext = createContext(null); @@ -37,12 +38,9 @@ function ThemeInjector({ synapse }: { synapse: Synapse }) { }, [synapse]); useEffect(() => { - if (theme.tokens) { - const style = document.documentElement.style; - for (const [k, v] of Object.entries(theme.tokens)) { - style.setProperty(k, v); - } - } + // Neutral defaults for the mode back any var the host omits, then host + // values win — the same single path used by the handshake in core.ts. + applyThemeVariables(theme.mode, theme.tokens); }, [theme]); return null; diff --git a/src/theme-defaults.ts b/src/theme-defaults.ts new file mode 100644 index 0000000..da76987 --- /dev/null +++ b/src/theme-defaults.ts @@ -0,0 +1,133 @@ +/** + * Theme-aware default backing for the `@nimblebrain/synapse/ui` token contract. + * + * `tokens` (see `ui/tokens.ts`) are `var(--token, fallback)` references. A CSS + * `var()` fallback is a *static literal* — it cannot branch on light vs. dark. + * So any var the host does NOT inject resolves to its single hardcoded (light) + * fallback in BOTH themes. Pair such a token with a theme-aware one and you get + * white-on-white in dark mode: it looks correct in light, passes `tsc`/build, + * and only breaks when the theme is toggled. + * + * This module closes that gap with a default layer the SDK controls and that + * CAN branch on theme. {@link applyThemeVariables} writes the neutral defaults + * for the active `mode` first, then the host's variables on top — so: + * - the host's (brand) values always win for the keys it provides, and + * - any var the host omits still resolves to a theme-correct neutral value. + * + * It's the `var()` fallback, but able to branch on theme. The values stay + * deliberately NEUTRAL (grays + a generic blue), never NimbleBrain brand — + * brand arrives only by host injection, mirroring `ui/tokens.ts`. This keeps + * the library host-agnostic while guaranteeing every token resolves correctly + * in both themes even against an incomplete host, a standalone `connect()` + * widget, or a third-party host. + * + * Only COLOR vars are theme-sensitive and listed here. Theme-invariant vars + * (radii, type scale, shadows, font stacks, weights, border widths) look the + * same in both themes, so their static `var()` fallback is already correct — + * they are intentionally absent. The `tokens` regression test enforces exactly + * this: every color var a token references must be defined in both maps below. + */ + +/** + * Light-theme neutral defaults. Values match the (light) fallbacks baked into + * `tokens` — applying them changes nothing observable in light mode; they exist + * so the light path flows through the same code as dark and so a standalone + * render with an explicit `light` theme is backed identically to its fallbacks. + */ +const LIGHT: Record = { + // ── Surfaces ── + "--color-background-primary": "#ffffff", + "--color-background-secondary": "#fafafa", + "--color-background-tertiary": "#f3f4f6", + // ── Text ── + "--color-text-primary": "#111827", + "--color-text-secondary": "#6b7280", + "--color-text-tertiary": "#9ca3af", + "--color-text-accent": "#2563eb", + "--nb-color-accent-foreground": "#ffffff", + // ── Border / ring ── + "--color-border-primary": "#e5e7eb", + "--color-border-secondary": "#d1d5db", + "--color-ring-primary": "#2563eb", + // ── Status / brand semantics ── + "--nb-color-danger": "#dc2626", + "--nb-color-success": "#059669", + "--nb-color-warning": "#f59e0b", + "--nb-color-warm": "#d4620a", + "--nb-color-warm-light": "#fef5ee", + "--nb-color-processing": "#7c3aed", + "--nb-color-processing-light": "#f3eeff", + "--nb-color-info-light": "#eef4ff", +}; + +/** + * Dark-theme neutral defaults. A monotonic neutral ladder — the three surface + * tiers are the darkest, borders sit a step lighter so they stay visible + * against every surface, and text inverts to light. "Subtle"/"strong" keep + * their light-mode semantics: `bgSubtle` reads as a lifted hover/inset tint and + * `borderStrong` is more prominent than `border`. + */ +const DARK: Record = { + // ── Surfaces (base → lifted) ── + "--color-background-primary": "#18181b", + "--color-background-secondary": "#27272a", + "--color-background-tertiary": "#2f2f34", + // ── Text ── + "--color-text-primary": "#fafafa", + "--color-text-secondary": "#a1a1aa", + "--color-text-tertiary": "#71717a", + "--color-text-accent": "#818cf8", + "--nb-color-accent-foreground": "#ffffff", + // ── Border / ring (lighter than surfaces so they remain visible) ── + "--color-border-primary": "#3f3f46", + "--color-border-secondary": "#52525b", + "--color-ring-primary": "#818cf8", + // ── Status / brand semantics (brightened for contrast on dark) ── + "--nb-color-danger": "#f87171", + "--nb-color-success": "#34d399", + "--nb-color-warning": "#fbbf24", + "--nb-color-warm": "#fb923c", + "--nb-color-warm-light": "#3a2a1e", + "--nb-color-processing": "#a78bfa", + "--nb-color-processing-light": "#2a2440", + "--nb-color-info-light": "#1e2a44", +}; + +/** + * The neutral default theme, keyed by mode. Exported for the regression test + * that asserts every color var referenced by `tokens` is backed in both modes. + */ +export const DEFAULT_THEME_VARS: Record<"light" | "dark", Record> = { + light: LIGHT, + dark: DARK, +}; + +/** + * Apply theme CSS custom properties onto `document.documentElement`. + * + * Writes the neutral defaults for `mode` FIRST, then the host's variables on + * top — so the host's values win for the keys it provides, and any var it omits + * still resolves to a theme-correct neutral default. This is the single path by + * which theming reaches the DOM (the handshake, `host-context-changed`, and the + * React `` all funnel through here). + * + * SSR-safe (no-ops when `document` is unavailable). Idempotent — `setProperty` + * overwrites, so re-applying on every theme change is correct and cheap. + */ +export function applyThemeVariables( + mode: "light" | "dark", + hostVars: Record | undefined | null, +): void { + if (typeof document === "undefined") return; + const root = document.documentElement.style; + for (const [k, v] of Object.entries(DEFAULT_THEME_VARS[mode])) { + root.setProperty(k, v); + } + if (hostVars && typeof hostVars === "object") { + for (const [k, v] of Object.entries(hostVars)) { + if (typeof k === "string" && typeof v === "string") { + root.setProperty(k, v); + } + } + } +} diff --git a/src/vite/plugin.ts b/src/vite/plugin.ts index 53a988f..b247630 100644 --- a/src/vite/plugin.ts +++ b/src/vite/plugin.ts @@ -284,17 +284,19 @@ function previewHostHtml(appName: string): string { function getTokens(d) { return d ? { - "--color-background-primary":"#0f172a","--color-text-primary":"#e2e8f0", - "--color-background-secondary":"#1e293b","--color-text-primary":"#e2e8f0", + "--color-background-primary":"#0f172a","--color-background-secondary":"#1e293b", + "--color-background-tertiary":"#2a374a","--color-text-primary":"#e2e8f0", + "--color-text-secondary":"#94a3b8","--color-text-tertiary":"#64748b", "--color-text-accent":"#6366f1","--nb-color-accent-foreground":"#fff", - "--color-text-secondary":"#94a3b8","--color-border-primary":"#334155", + "--color-border-primary":"#334155","--color-border-secondary":"#475569", "--color-ring-primary":"#6366f1","--nb-color-danger":"#ef4444", "--border-radius-sm":"0.5rem","--font-sans":"-apple-system,BlinkMacSystemFont,sans-serif" } : { - "--color-background-primary":"#ffffff","--color-text-primary":"#0f172a", - "--color-background-secondary":"#f8fafc","--color-text-primary":"#0f172a", + "--color-background-primary":"#ffffff","--color-background-secondary":"#f8fafc", + "--color-background-tertiary":"#f1f5f9","--color-text-primary":"#0f172a", + "--color-text-secondary":"#64748b","--color-text-tertiary":"#94a3b8", "--color-text-accent":"#6366f1","--nb-color-accent-foreground":"#fff", - "--color-text-secondary":"#64748b","--color-border-primary":"#e2e8f0", + "--color-border-primary":"#e2e8f0","--color-border-secondary":"#cbd5e1", "--color-ring-primary":"#6366f1","--nb-color-danger":"#ef4444", "--border-radius-sm":"0.5rem","--font-sans":"-apple-system,BlinkMacSystemFont,sans-serif" }; From a82586a29389a5f44ea69113e4b4932440c8872c Mon Sep 17 00:00:00 2001 From: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:41:39 -1000 Subject: [PATCH 2/2] Harden regression guard: total partition over referenced vars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address QA feedback on the token-backing test. The guard split theme-sensitive vs theme-invariant vars by the substring "color" in the var name, so a future theme-sensitive var whose name lacks "color" would silently slip the "must be backed in both modes" check. Replace the name heuristic with an explicit THEME_INVARIANT_VARS allowlist (the radii/type-scale/shadow/font vars) and make the partition total: every referenced var must be either declared invariant or backed in both light and dark — anything else fails. Add a test that the invariant allowlist carries no stale (unreferenced) entries. Also clears the one new biome useOptionalChain warning by dropping the name-substring branch. No production change; test-only hardening. --- src/__tests__/ui/tokens.test.ts | 73 +++++++++++++++++++++++++++------ src/theme-defaults.ts | 8 ++-- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/__tests__/ui/tokens.test.ts b/src/__tests__/ui/tokens.test.ts index 912f39d..5c16daf 100644 --- a/src/__tests__/ui/tokens.test.ts +++ b/src/__tests__/ui/tokens.test.ts @@ -45,25 +45,72 @@ describe("default theme backs the token contract", () => { .map(referencedVar) .filter((v): v is string => v !== null); - // A var is theme-sensitive iff it names a color. Theme-invariant vars (radii, - // type scale, shadows, fonts, weights, widths) look identical in both themes, - // so their static var() fallback is already correct — they are intentionally - // NOT backed by DEFAULT_THEME_VARS. - const colorVars = referenced.filter((v) => v.includes("color")); + // Vars that look identical in light and dark — their static var() fallback is + // already correct, so they are intentionally NOT in DEFAULT_THEME_VARS. Listed + // explicitly (not inferred from the name) so the partition below is TOTAL: any + // referenced var that is neither declared invariant here nor backed in both + // modes fails the guard, forcing a conscious light/dark decision — even for a + // future theme-sensitive var whose name doesn't happen to contain "color". + const THEME_INVARIANT_VARS = new Set([ + "--font-sans", + "--font-mono", + "--nb-font-heading", + "--font-weight-normal", + "--font-weight-medium", + "--font-weight-semibold", + "--font-weight-bold", + "--font-text-xs-size", + "--font-text-xs-line-height", + "--font-text-sm-size", + "--font-text-sm-line-height", + "--font-text-base-size", + "--font-text-base-line-height", + "--font-text-lg-size", + "--font-text-lg-line-height", + "--font-heading-sm-size", + "--font-heading-sm-line-height", + "--font-heading-md-size", + "--font-heading-md-line-height", + "--font-heading-lg-size", + "--font-heading-lg-line-height", + "--border-radius-xs", + "--border-radius-sm", + "--border-radius-md", + "--border-radius-lg", + "--border-radius-xl", + "--border-width-regular", + "--shadow-hairline", + "--shadow-sm", + "--shadow-md", + "--shadow-lg", + ]); - it("defines every color var a token references in BOTH light and dark", () => { + // Theme-sensitive = referenced but not declared invariant. These MUST be + // backed in both modes. + const themeSensitive = referenced.filter((v) => !THEME_INVARIANT_VARS.has(v)); + + it("backs every theme-sensitive var the token contract references in BOTH light and dark", () => { // The regression guard. A `var()` fallback is a static literal that fires // exactly when the var is unset — the moment there's no theme signal to - // branch on — so an unbacked color token resolves to its light fallback in - // dark mode (white-on-white). This fails the instant someone adds a color - // token whose var nothing backs, instead of shipping it inert. - expect(colorVars.length).toBeGreaterThan(0); - for (const v of colorVars) { + // branch on — so an unbacked theme-sensitive token resolves to its light + // fallback in dark mode (white-on-white). This fails the instant someone + // adds such a token whose var nothing backs, instead of shipping it inert. + expect(themeSensitive.length).toBeGreaterThan(0); + for (const v of themeSensitive) { expect(Object.keys(DEFAULT_THEME_VARS.light), `light missing ${v}`).toContain(v); expect(Object.keys(DEFAULT_THEME_VARS.dark), `dark missing ${v}`).toContain(v); } }); + it("declares no stale invariant vars — every one is actually referenced", () => { + // Keeps THEME_INVARIANT_VARS honest: an entry no token references is dead + // and would silently shrink what the guard above covers. + const referencedSet = new Set(referenced); + for (const v of THEME_INVARIANT_VARS) { + expect(referencedSet.has(v), `stale invariant entry ${v}`).toBe(true); + } + }); + it("light and dark define the same set of vars", () => { expect(Object.keys(DEFAULT_THEME_VARS.light).sort()).toEqual( Object.keys(DEFAULT_THEME_VARS.dark).sort(), @@ -78,11 +125,11 @@ describe("default theme backs the token contract", () => { }); it("light defaults equal the token fallbacks (no light-mode regression)", () => { - // The light layer must reproduce each color token's baked fallback, so + // The light layer must reproduce each backed token's baked fallback, so // introducing the default layer changes nothing observable in light mode. for (const value of Object.values(tokens)) { const v = referencedVar(value); - if (!v || !v.includes("color")) continue; + if (!v || THEME_INVARIANT_VARS.has(v)) continue; const fallback = value.slice(value.indexOf(",") + 1, -1).trim(); expect(DEFAULT_THEME_VARS.light[v]).toBe(fallback); } diff --git a/src/theme-defaults.ts b/src/theme-defaults.ts index da76987..98a9220 100644 --- a/src/theme-defaults.ts +++ b/src/theme-defaults.ts @@ -21,11 +21,13 @@ * in both themes even against an incomplete host, a standalone `connect()` * widget, or a third-party host. * - * Only COLOR vars are theme-sensitive and listed here. Theme-invariant vars + * Only theme-sensitive (color) vars are listed here. Theme-invariant vars * (radii, type scale, shadows, font stacks, weights, border widths) look the * same in both themes, so their static `var()` fallback is already correct — - * they are intentionally absent. The `tokens` regression test enforces exactly - * this: every color var a token references must be defined in both maps below. + * they are intentionally absent. The `tokens` regression test enforces this + * with a TOTAL partition: every referenced var must be either declared + * theme-invariant (an explicit allowlist) or defined in both maps below, so a + * newly added theme-sensitive token can't slip through unbacked. */ /**