fix: constrain theme state optional fields
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
+22
-12
@@ -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,
|
||||
|
||||
@@ -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; }
|
||||
|
||||
@@ -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<Msg>] {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user