From 0dec97a6a79d519cf1f5c4eb13e272025f48aaf5 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Mon, 17 Aug 2026 08:17:44 -0500 Subject: [PATCH] fix: constrain theme state optional fields --- build.zig | 2 +- packages/core/src/service_contract.ts | 10 ++++++ packages/core/src/typed_ast.ts | 9 ++++-- packages/core/src/types.ts | 34 +++++++++++++------- packages/core/test/checker.test.ts | 14 +++++++++ packages/core/test/services.test.ts | 45 +++++++++++++++++++++++++++ 6 files changed, 98 insertions(+), 16 deletions(-) diff --git a/build.zig b/build.zig index 4e5d50df..caa9a700 100644 --- a/build.zig +++ b/build.zig @@ -2648,7 +2648,7 @@ pub fn build(b: *std.Build) void { \\case "$ready_snapshot" in *'view @w1/components-canvas kind=gpu_surface'*'gpu_nonblank=true'*'canvas_frame_gpu_packet_representable=true'*) ;; *) echo "component gallery GPU surface was not ready" >&2; exit 1 ;; esac \\case "$ready_snapshot" in *'view @w1/main kind=webview'*) echo "component gallery created an implicit WebView" >&2; exit 1 ;; *) ;; esac \\"$cli" automate assert 'role=tree name="Components"' 'role=treeitem name="Components".*state=\[expanded\]' 'role=treeitem name="Accordion".*state=\[selected\]' 'role=group name="Details".*state=\[selected,expanded\]' 'name="Accordion details are visible. The model owns this expanded state."' - \\"$cli" automate assert 'role=group name="Theme"' 'role=button name="Default".*state=\[selected\]' 'role=button name="Geist"' + \\"$cli" automate assert 'role=group name="Theme pack"' 'role=group name="Color scheme"' 'role=group name="Theme accent"' 'role=button name="Default".*state=\[selected\]' 'role=button name="Geist"' 'role=button name="System".*state=\[selected\]' 'role=button name="Pink"' 'role=button name="Teal"' \\"$cli" automate screenshot components-canvas >/dev/null 2>&1 \\cp "$automation_dir/screenshot-components-canvas.png" "$automation_dir/screenshot-components-house.png" \\rm -f "$automation_dir/screenshot-components-canvas.png" diff --git a/packages/core/src/service_contract.ts b/packages/core/src/service_contract.ts index 97e33b51..98ae22a6 100644 --- a/packages/core/src/service_contract.ts +++ b/packages/core/src/service_contract.ts @@ -258,6 +258,16 @@ class ServiceShapeTable { if (this.listed.has(name)) return; const info = this.table.structs.get(name); if (!info) throw new ServiceShapeError(`The shared type table has no record named \`${name}\``, this.fallbackSite); + const optionalField = info.fields.find((field) => + (ts.isPropertySignature(field.decl) || ts.isPropertyDeclaration(field.decl)) && + field.decl.questionToken !== undefined + ); + if (optionalField) { + throw new ServiceShapeError( + `Service boundary record \`${name}\` has optional property \`${optionalField.tsName}?\`; spell absence explicitly as \`${optionalField.tsName}: T | null\` so both service codecs encode the same state`, + optionalField.decl, + ); + } this.listed.add(name); const record: ServiceRecordType = { name, diff --git a/packages/core/src/typed_ast.ts b/packages/core/src/typed_ast.ts index 6e18def8..b02a5edb 100644 --- a/packages/core/src/typed_ast.ts +++ b/packages/core/src/typed_ast.ts @@ -342,12 +342,15 @@ export class TypedAst { /// order — null unless every member is a plain record property (an /// identifier-named, annotated property signature), so /// a shape this walk cannot carry whole refuses as an unsupported - /// alias instead of registering a struct with silently missing - /// fields. - propsOfTypeLiteral(node: tsImpl.TypeLiteralNode): PropInfo[] | null { + /// alias instead of registering a struct with silently missing fields. + /// The one omission-carrying value record is the SDK-owned ThemeState; + /// its caller opts in explicitly so authored/service records keep the + /// fixed-shape rule. + propsOfTypeLiteral(node: tsImpl.TypeLiteralNode, allowOptional = false): PropInfo[] | null { const out: PropInfo[] = []; for (const member of node.members) { if (!tsImpl.isPropertySignature(member) || !member.name || !tsImpl.isIdentifier(member.name) || !member.type) return null; + if (member.questionToken && !allowOptional) return null; out.push({ name: member.name.text, optional: member.questionToken !== undefined, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 59fd06c4..a999b049 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -12,7 +12,8 @@ // T | null/undefined -> ?T (R7) // Uint8Array -> []const u8 (R3) -import { ts, TypedAst, hasExportModifier, exportListBindings, type PropInfo } from "./typed_ast.ts"; +import path from "node:path"; +import { ts, TypedAst, hasExportModifier, exportListBindings, sdkLibraryModules, type PropInfo } from "./typed_ast.ts"; import { mutatingMethodNames } from "./ownership.ts"; export type ZType = @@ -265,14 +266,17 @@ export class TypeTable { this.declOrder.push(name); continue; } - if (ts.isTypeLiteralNode(stmt.type) && this.tast.propsOfTypeLiteral(stmt.type) !== null) { + const projectOptional = this.isCanonicalThemeState(stmt); + if (ts.isTypeLiteralNode(stmt.type) && this.tast.propsOfTypeLiteral(stmt.type, projectOptional) !== null) { // A plain-record object-literal alias is a struct exactly like // an interface; the alias FORM is how a contract projection // spells a value-stored record (interfaces spell node // storage), and the storage itself still comes from the // promotion walk. Shapes the plain-record walk cannot carry // whole (quoted or optional properties) stay unclassified and - // refuse at emission instead of losing fields silently. + // refuse at emission instead of losing fields silently. The + // canonical SDK ThemeState is the sole optional-property record: + // omission is its manifest/system inheritance signal. this.structs.set(name, { name, decl: stmt, @@ -319,24 +323,30 @@ export class TypeTable { } const structInfo = this.structs.get(stmt.name.text); if (structInfo && structInfo.decl === stmt && ts.isTypeLiteralNode(stmt.type)) { - const props = this.tast.propsOfTypeLiteral(stmt.type); - if (props) structInfo.fields = props.map((p) => this.fieldOf(p)); + const projectOptional = this.isCanonicalThemeState(stmt); + const props = this.tast.propsOfTypeLiteral(stmt.type, projectOptional); + if (props) structInfo.fields = props.map((p) => this.fieldOf(p, projectOptional)); } } } } - private fieldOf(p: PropInfo): ZField { + private isCanonicalThemeState(decl: ts.TypeAliasDeclaration): boolean { + const events = sdkLibraryModules.get("@native-sdk/core/events"); + return decl.name.text === "ThemeState" && events !== undefined && + path.resolve(decl.getSourceFile().fileName) === path.resolve(events); + } + + private fieldOf(p: PropInfo, projectOptional = false): ZField { const resolved = p.typeNode ? this.resolveTypeNode(p.typeNode) : { k: "void" } as ZType; return { tsName: p.name, zigName: zigDeclName(p.name), - // An optional interface property is one JS `undefined` absence level. - // Project it onto the contract's ordinary optional slot so helper - // records such as ThemeState preserve omission across the ABI. Model - // and Msg trees still teach authors toward explicit `T | null` at - // their own shape checks; projection-safe helper records may use `?`. - type: p.optional && resolved.k !== "void" && resolved.k !== "optional" + // Only the canonical ThemeState projects JS `undefined` omission onto + // the contract's ordinary optional slot. Applying this globally would + // let service records acquire an absent state while their generated + // codecs still accept only explicit null. + type: projectOptional && p.optional && resolved.k !== "void" && resolved.k !== "optional" ? { k: "optional", inner: resolved } : resolved, decl: p.declaration, diff --git a/packages/core/test/checker.test.ts b/packages/core/test/checker.test.ts index 4e86aebf..637f1344 100644 --- a/packages/core/test/checker.test.ts +++ b/packages/core/test/checker.test.ts @@ -1156,6 +1156,20 @@ export function update(model: Model, msg: Msg): Model { return model; } assert.equal(clean.ok, true, clean.diagnostics.map((d) => d.message).join("\n")); assert.ok(!ruleIds(clean).includes("NS1033"), `got ${ruleIds(clean)}`); + const lookalike = checkOnly(` +export type ThemeState = { + readonly pack?: "house" | "geist"; + readonly colorScheme?: "light" | "dark" | "system"; + readonly accent?: string; +}; +export interface Model { readonly enabled: boolean; } +export type Msg = { readonly kind: "tick" }; +export function initialModel(): Model { return { enabled: false }; } +export function themeState(model: Model): ThemeState { return {}; } +export function update(model: Model, msg: Msg): Model { return model; } +`); + assert.ok(ruleIds(lookalike).includes("NS1033"), `got ${ruleIds(lookalike)}`); + const wrongRecord = checkOnly(` export interface WrongThemeState { readonly pack: "house" | "geist"; readonly accent: Uint8Array; } export interface Model { readonly enabled: boolean; } diff --git a/packages/core/test/services.test.ts b/packages/core/test/services.test.ts index 64a8f83e..a8ef5ea6 100644 --- a/packages/core/test/services.test.ts +++ b/packages/core/test/services.test.ts @@ -265,6 +265,51 @@ export function roundTrip(request: BoundaryRecord): BoundaryRecord { return requ assert.match(result.servicesClient!, /serviceUnionBytes/); }); +test("optional service record properties refuse before generating an undefined-unsafe codec", () => { + const files = { + "core.ts": ` +import { Cmd } from "@native-sdk/core"; +import { preferencesSave } from "@native-sdk/services"; +import type { Preferences } from "./shared.ts"; +export interface Model { readonly saved: boolean; } +export type Msg = + | { readonly kind: "save" } + | { readonly kind: "saved"; readonly bytes: Uint8Array } + | { readonly kind: "failed"; readonly error: Uint8Array }; +export function update(model: Model, msg: Msg): Model | [Model, Cmd] { + switch (msg.kind) { + case "save": { + const request: Preferences = {}; + return [model, preferencesSave(request, { ok: "saved", err: "failed" })]; + } + case "saved": return { saved: true }; + case "failed": return model; + } +}`, + "shared.ts": `export type Preferences = { readonly accent?: Uint8Array };`, + "services/preferences.ts": ` +import type { Preferences } from "../shared.ts"; +export function save(request: Preferences): Uint8Array { + return request.accent ?? new Uint8Array(0); +}`, + }; + for (const declaration of [ + `export type Preferences = { readonly accent?: Uint8Array };`, + `export interface Preferences { readonly accent?: Uint8Array }`, + ]) { + const result = checkFiles({ ...files, "shared.ts": declaration }, { + contractEntry: "src/core.ts", + servicesContract: true, + }); + assert.equal(result.ok, false); + assert.ok(result.diagnostics.some((diagnostic) => + diagnostic.id === "NS1067" && /(resolves to void|has optional property)/.test(diagnostic.message) + ), JSON.stringify(result.diagnostics)); + assert.equal(result.servicesContract, null); + assert.equal(result.servicesClient, null); + } +}); + test("generated clients parenthesize composite slice element types", () => { const result = checkFiles({ "core.ts": serviceCore,