Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Make vnode properties liberal in what they accept
Postel's Law: be conservative in what you do, be liberal in what
you accept from others.

Properties were previously being validated and would complain if you
included a property that was not allowed. Instead, we want to validate
properties, while allowing additional properties, but ignoring those
additional properties when rendering. This will make us more robust to
LLM hallucinations.
  • Loading branch information
gordonbrander authored and bfollington committed Jun 17, 2024
commit b9d066a6c3cfac798b4cf5f116778f2d029a380f
1 change: 1 addition & 0 deletions typescript/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 4 additions & 7 deletions typescript/packages/common-ui/src/components/button.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { view } from "../hyperscript/render.js";

export const button = view("button", {
type: "object",
properties: {
id: { type: "string" },
"@click": {
type: "object",
properties: { "@type": { type: "string" }, name: { type: "string" } },
},
id: { type: "string" },
"@click": {
type: "object",
properties: { "@type": { type: "string" }, name: { type: "string" } },
},
});
7 changes: 2 additions & 5 deletions typescript/packages/common-ui/src/components/datatable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ import { repeat } from 'lit/directives/repeat.js';
import { view } from '../hyperscript/render.js';

export const datatable = view('common-datatable', {
type: 'object',
properties: {
cols: { type: 'array' },
rows: { type: 'array' },
}
cols: { type: 'array' },
rows: { type: 'array' },
});

@customElement('common-datatable')
Expand Down
5 changes: 1 addition & 4 deletions typescript/packages/common-ui/src/components/dict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ import { repeat } from 'lit/directives/repeat.js';
import { view } from '../hyperscript/render.js';

export const dict = view('common-dict', {
type: 'object',
properties: {
records: { type: 'object' },
}
records: { type: 'object' },
});

@customElement('common-dict')
Expand Down
5 changes: 1 addition & 4 deletions typescript/packages/common-ui/src/components/div.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { view } from '../hyperscript/render.js';

export const div = view('div', {
type: 'object',
properties: {
id: { type: 'string' },
}
id: { type: 'string' },
});
5 changes: 1 addition & 4 deletions typescript/packages/common-ui/src/components/h1.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { view } from "../hyperscript/render.js";

export const h1 = view("h1", {
type: "object",
properties: {
id: { type: "string" },
},
id: { type: "string" },
});
5 changes: 1 addition & 4 deletions typescript/packages/common-ui/src/components/p.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { view } from "../hyperscript/render.js";

export const p = view("p", {
type: "object",
properties: {
id: { type: "string" },
},
id: { type: "string" },
});
5 changes: 1 addition & 4 deletions typescript/packages/common-ui/src/components/span.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { view } from "../hyperscript/render.js";

export const span = view("span", {
type: "object",
properties: {
id: { type: "string" },
},
id: { type: "string" },
});
19 changes: 19 additions & 0 deletions typescript/packages/common-ui/src/deep-freeze.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/** Deep freeze an object */
export const deepFreeze = <T extends object>(obj: T): T => {
// Retrieve the property names defined on object
const propNames = Reflect.ownKeys(obj);

// Freeze properties before freezing self
for (const name of propNames) {
// @ts-ignore
const value = obj[name];

if ((value && typeof value === "object") || typeof value === "function") {
deepFreeze(value);
}
}

return Object.freeze(obj);
}

export default deepFreeze;
88 changes: 57 additions & 31 deletions typescript/packages/common-ui/src/hyperscript/render.ts
Original file line number Diff line number Diff line change
@@ -1,70 +1,96 @@
import { Cancel, combineCancels } from '@commontools/common-frp';
import { Signal, effect } from '@commontools/common-frp/signal';
import { Cancel, combineCancels } from "@commontools/common-frp";
import { Signal, effect } from "@commontools/common-frp/signal";
import {
isBinding,
VNode,
AnyJSONSchema,
JSONSchemaRecord,
View,
view as createView
} from './view.js';
view as createView,
} from "./view.js";

/** Registry for tags that are allowed to be rendered */
const registry = () => {
const viewByTag = new Map<string, View>();

const listViews = () => Array.from(viewByTag.values());

const getViewByTag = (tag: string) => viewByTag.get(tag);

const register = (view: View) => {
const registerView = (view: View) => {
viewByTag.set(view.tag, view);
}
};

return {getViewByTag, register};
}
return { getViewByTag, listViews, registerView };
};

export const {getViewByTag, register} = registry();
export const { getViewByTag, listViews, registerView } = registry();

/** Define and register a view factory function */
export const view = (
tagName: string,
propsSchema: AnyJSONSchema = {}
props: JSONSchemaRecord = {},
): View => {
const factory = createView(tagName, propsSchema);
register(factory);
const factory = createView(tagName, props);
registerView(factory);
return factory;
}
};

export type RenderContext = Record<string, Signal<any>>
export type RenderContext = Record<string, Signal<any>>;

export const __cancel__ = Symbol('cancel');
export const __cancel__ = Symbol("cancel");

/** Render a VNode tree, binding reactive data sources. */
const renderVNode = (
vnode: VNode,
context: RenderContext
): Node => {
const renderVNode = (vnode: VNode, context: RenderContext): Node => {
// Make sure we have a view for this tag. If we don't it is not whitelisted.
const view = getViewByTag(vnode.tag);

if (typeof view !== 'function') {
if (typeof view !== "function") {
throw new TypeError(`Unknown tag: ${vnode.tag}`);
}

// Validate props against the view's schema.
if (!view.validateProps(vnode.props)) {
if (!view.props.validate(vnode.props)) {
throw new TypeError(`Invalid props for tag: ${vnode.tag}.
Props: ${JSON.stringify(vnode.props)}`);
Props: ${JSON.stringify(vnode.props)}
Schema: ${JSON.stringify(view.props.schema)}`);
}

// Create the element
const element = document.createElement(vnode.tag);

// Bind each prop to a reactive value (if any) and collect cancels
const cancels: Array<Cancel> = [];
const snapshot = { ...context };
for (const [key, value] of Object.entries(vnode.props)) {
// Don't bind properties that aren't whitelisted in the schema.
if (!Object.hasOwn(view.props.schema.properties, key)) {
continue;
}

if (key == "@click" || key == "onclick") {
if (isBinding(value)) {
console.log("onclick bind", snapshot);
const bound = snapshot[(value as any).name];
if (!bound) continue;

// IMPORTANT: we cannot close over a reference to a signal reference without lit-html dropping it
// so we need to extract the send function from the signal and use that directly.
const send = (bound as any).send;
console.log("onclick bind 2", bound);
element.addEventListener("click", (ev: MouseEvent) => {
const event = { type: "click", target: ev.target, button: ev.button };
console.log("onlick", value, send, event);
send(event);
});
}

continue;
}

if (isBinding(value)) {
const boundValue = context[value.name];
if (boundValue != null) {
const cancel = effect([boundValue], value => {
const cancel = effect([boundValue], (value) => {
setProp(element, key, value);
});
cancels.push(cancel);
Expand All @@ -80,33 +106,33 @@ const renderVNode = (
element[__cancel__] = cancel;

for (const child of vnode.children) {
if (typeof child === 'string') {
if (typeof child === "string") {
element.appendChild(document.createTextNode(child));
} else {
element.appendChild(render(child, context));
}
}

return element;
}
};

/** Render a view tree, binding reactive data sources. */
export const render = (
vnode: VNode | string | undefined | null,
context: RenderContext = {}
context: RenderContext = {},
): Node => {
if (vnode == null) {
return document.createTextNode('');
return document.createTextNode("");
}
if (typeof vnode === 'string') {
if (typeof vnode === "string") {
return document.createTextNode(vnode);
}
return renderVNode(vnode, context);
}
};

export default render;

const setProp = (element: HTMLElement, key: string, value: any) => {
// @ts-ignore
element[key] = value;
}
};
48 changes: 38 additions & 10 deletions typescript/packages/common-ui/src/hyperscript/view.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import * as schema from '../schema.js';
import * as Schema from '../schema.js';
import deepFreeze from '../deep-freeze.js';

export type AnyJSONSchema = object;

export type JSONSchemaRecord = Record<string, AnyJSONSchema>;

export type AnyJSONObjectSchema = {
type: "object";
properties: Record<string, AnyJSONSchema>;
additionalProperties?: boolean;
};

export type Binding = {
"@type": "binding";
name: string;
Expand Down Expand Up @@ -75,7 +84,7 @@ export const VNodeSchema = {
}

/** Is object a VNode? */
export const isVNode = schema.compile(VNodeSchema)
export const isVNode = Schema.compile(VNodeSchema)

/** Internal helper for creating VNodes */
const vh = (
Expand All @@ -88,6 +97,15 @@ const vh = (
children
});

/** Decorate the props JSON schema object with additional JSON schema flags */
const decoratePropsSchema = (
properties: JSONSchemaRecord
): AnyJSONObjectSchema => ({
type: "object",
properties,
additionalProperties: true,
});

export type Factory = {
(): VNode

Expand All @@ -97,38 +115,48 @@ export type Factory = {
): VNode
};

export type PropsDescription = {
schema: AnyJSONObjectSchema;
validate: (data: any) => boolean;
}

export type View = Factory & {
tag: Tag;
validateProps: (data: any) => boolean;
props: PropsDescription;
}

/**
* Create a tag factory that validates props against a schema.
* @param tagName - HTML tag name
* @param propsSchema - JSON schema for props
* @param props - the properties section of a JSON schema
*/
export const view = (
tagName: string,
propsSchema: AnyJSONSchema = {}
props: JSONSchemaRecord = {}
): View => {
// Normalize tag name
const tag = tagName.toLowerCase();

const schema = decoratePropsSchema(props);

// Compile props validator for fast validation at runtime.
const validateProps = schema.compile(propsSchema);
const validate = Schema.compile(schema);

/** Create an element from a view, validating props */
const create = (
props: Props = {},
...children: Array<VNode | string>
) => {
if (!validateProps(props)) {
throw new TypeError(`Invalid props for ${tag}`);
if (!validate(props)) {
throw new TypeError(`Invalid props for ${tag}.
Props: ${JSON.stringify(props)}
Schema: ${JSON.stringify(schema)}`);
}
return vh(tag, props, ...children);
}

create.tag = tag;
create.validateProps = validateProps;
create.props = {validate, schema};

return Object.freeze(create);
return deepFreeze(create);
};