From becc4f743222bbae0300fd2123dca73c09691f8b Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 13 Sep 2026 19:27:06 +0300 Subject: [PATCH 1/6] feat: author scoped component properties in Design JSX --- CHANGELOG.md | 1 + .../src/design-jsx/component-properties.ts | 69 +++++++ packages/core/src/design-jsx/components.ts | 18 +- packages/core/src/design-jsx/index.ts | 2 + .../src/design-jsx/reference/authoring.md | 5 +- packages/core/src/design-jsx/renderer.ts | 46 ++++- packages/core/src/design-jsx/schema.ts | 2 + packages/core/src/design-jsx/tree.ts | 17 +- packages/core/src/index.ts | 2 + packages/docs/reference/design-authoring.md | 7 +- .../references/design-authoring.md | 7 +- .../render/jsx/component-properties.test.ts | 177 ++++++++++++++++++ 12 files changed, 331 insertions(+), 22 deletions(-) create mode 100644 packages/core/src/design-jsx/component-properties.ts create mode 100644 tests/engine/render/jsx/component-properties.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 726d52611..5570fd372 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Added - Bind Design JSX spacing, sizing, corners, and typography directly to numeric document variables. +- Define component properties and assign instance values in Design JSX using stable property IDs. - Save AI conversations and attachment previews locally, switch between chats, rename or delete them, and browse saved transcripts across documents. Choose whether reasoning stays collapsed, expands while thinking, or stays expanded, with animated disclosure controls that respect reduced motion. - Add a searchable command palette for editor and application actions. diff --git a/packages/core/src/design-jsx/component-properties.ts b/packages/core/src/design-jsx/component-properties.ts new file mode 100644 index 000000000..0a0d01c4b --- /dev/null +++ b/packages/core/src/design-jsx/component-properties.ts @@ -0,0 +1,69 @@ +import * as v from 'valibot' + +import { applyComponentPropertyValue, componentPropertyDefinitions } from '@open-pencil/scene-graph' +import type { + ComponentPropertyDefinition, + ComponentPropertyReference, + SceneGraph, + SceneNode +} from '@open-pencil/scene-graph' + +const definitionSchema = v.object({ + id: v.string(), + name: v.string(), + type: v.picklist(['VARIANT', 'TEXT', 'BOOLEAN', 'INSTANCE_SWAP']), + defaultValue: v.string(), + variantOptions: v.optional(v.array(v.string())), + preferredValues: v.optional(v.array(v.string())) +}) satisfies v.GenericSchema +const referenceSchema = v.object({ + propertyId: v.string(), + field: v.picklist(['VISIBLE', 'TEXT', 'INSTANCE_SWAP']) +}) satisfies v.GenericSchema + +/** Accept the native graph contracts, without introducing a second property model. */ +export function componentMetadata( + props: Record, + type: SceneNode['type'] +): Partial { + const result: Partial = {} + if (props.properties !== undefined && type !== 'INSTANCE') { + if (type !== 'COMPONENT' && type !== 'COMPONENT_SET') + throw new Error('Only components, component sets, and instances accept properties') + const definitions = v.parse(v.array(definitionSchema), props.properties) + if (new Set(definitions.map((definition) => definition.id)).size !== definitions.length) + throw new Error('Duplicate component property IDs') + result.componentPropertyDefinitions = definitions + } + if (props.propertyRefs !== undefined) { + const references = v.parse(v.array(referenceSchema), props.propertyRefs) + for (const reference of references) { + if (reference.field === 'TEXT' && type !== 'TEXT') + throw new Error('TEXT properties require a text node') + if (reference.field === 'INSTANCE_SWAP' && type !== 'INSTANCE') + throw new Error('INSTANCE_SWAP properties require an instance') + } + result.componentPropertyReferences = references + } + return result +} + +export function assignComponentProperties( + graph: SceneGraph, + instance: SceneNode, + input: unknown +): void { + if (input === undefined) return + const assignments = v.parse(v.record(v.string(), v.string()), input) + const definitions = componentPropertyDefinitions(graph, instance) + for (const [id, value] of Object.entries(assignments)) { + const definition = definitions.find((item) => item.id === id) + if (!definition) throw new Error(`Unknown component property: ${id}`) + if (definition.type === 'VARIANT') + throw new Error('Select a component-set variant when creating the instance') + if (definition.type === 'BOOLEAN' && value !== 'true' && value !== 'false') + throw new Error(`Expected true or false for component property: ${id}`) + if (!applyComponentPropertyValue(graph, instance.id, definition, value)) + throw new Error(`Cannot assign component property: ${id}`) + } +} diff --git a/packages/core/src/design-jsx/components.ts b/packages/core/src/design-jsx/components.ts index 0b9d928ea..c9d245297 100644 --- a/packages/core/src/design-jsx/components.ts +++ b/packages/core/src/design-jsx/components.ts @@ -1,4 +1,11 @@ -import { node, type BaseProps, type TextProps, type TreeNode } from './tree' +import { + node, + type BaseProps, + type ComponentProps, + type InstanceProps, + type TextProps, + type TreeNode +} from './tree' type Child = TreeNode | string @@ -55,18 +62,15 @@ export function Section(props: BaseProps, ...children: Child[]): TreeNode { return withChildren('section', props, children) } -export function Component(props: BaseProps, ...children: Child[]): TreeNode { +export function Component(props: ComponentProps, ...children: Child[]): TreeNode { return withChildren('component', props, children) } -export function ComponentSet(props: BaseProps, ...children: Child[]): TreeNode { +export function ComponentSet(props: ComponentProps, ...children: Child[]): TreeNode { return withChildren('component-set', props, children) } -export function Instance( - props: BaseProps & { component?: string; componentId?: string; of?: string }, - ...children: Child[] -): TreeNode { +export function Instance(props: InstanceProps, ...children: Child[]): TreeNode { return withChildren('instance', props, children) } diff --git a/packages/core/src/design-jsx/index.ts b/packages/core/src/design-jsx/index.ts index 67b24e20e..70cdebdf9 100644 --- a/packages/core/src/design-jsx/index.ts +++ b/packages/core/src/design-jsx/index.ts @@ -21,6 +21,8 @@ export { export { type TreeNode, type BaseProps, + type ComponentProps, + type InstanceProps, type TextProps, type StyleProps, type PaintProp, diff --git a/packages/core/src/design-jsx/reference/authoring.md b/packages/core/src/design-jsx/reference/authoring.md index 9cd9cd518..ba139ff0c 100644 --- a/packages/core/src/design-jsx/reference/authoring.md +++ b/packages/core/src/design-jsx/reference/authoring.md @@ -31,7 +31,10 @@ This reference describes scene creation, not React DOM output. Use the `render` - `bind` maps supported scene-field paths to variable IDs or references when no shorthand exists. Use semantic tokens consistently rather than declaring unused collections. - A reusable JavaScript function shares source code, not component identity. Use `Component`, `ComponentSet`, and `Instance` for editable main components and linked instances. - `Instance` resolves an existing component through `of`, `component`, or `componentId`. Component-set children named `variant=Primary`, for example, define variants that can be selected when instantiating the set. -- Reuse existing local or library components before recreating them. Expose meaningful text, visibility, and swap properties through the existing component-property APIs; do not assume every native property API is already exposed as a JSX prop. +- `Component` and `ComponentSet` accept `properties`, an array of native property definitions (`id`, `name`, `type`, `defaultValue`). `Instance` accepts `properties`, an ID-to-value assignment object. Ordinary nodes do not accept `properties`. +- Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. +- Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. +- Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. ## Verification diff --git a/packages/core/src/design-jsx/renderer.ts b/packages/core/src/design-jsx/renderer.ts index 1059a28c7..7c6476f7a 100644 --- a/packages/core/src/design-jsx/renderer.ts +++ b/packages/core/src/design-jsx/renderer.ts @@ -16,6 +16,7 @@ import type { IconData } from '#core/icons/types' import { computeAllLayouts } from '#core/layout' import { randomHex } from '#core/random' +import { assignComponentProperties, componentMetadata } from './component-properties' import { applySizeOverrides, propsToOverrides } from './props-overrides' import { prepareScalarBindings } from './scalar-bindings' import { isTreeNode } from './tree' @@ -367,7 +368,11 @@ function findVariantInSet( ) { const requested = Object.fromEntries( Object.entries(props) - .filter(([key]) => !['component', 'componentId', 'of', 'name', 'children'].includes(key)) + .filter(([key]) => + componentSet.componentPropertyDefinitions.some( + (definition) => definition.type === 'VARIANT' && definition.name === key + ) + ) .map(([key, value]) => [key, String(value)]) ) const variants = graph.getChildren(componentSet.id).filter((node) => node.type === 'COMPONENT') @@ -415,12 +420,21 @@ async function renderInstanceNode( const label = typeof ref === 'string' || typeof ref === 'number' ? String(ref) : '' throw new Error(` component not found: ${label}`) } - const overrides = propsToOverrides(props, false, parentLayout) + const overrides = { + ...propsToOverrides(props, false, parentLayout), + ...componentMetadata(props, 'INSTANCE') + } const instance = graph.createInstance(component.id, parentId, overrides) ?? graph.createNode('FRAME', parentId) - applyBindings(graph, instance.id, bindings) - applyInstanceOverrides(graph, instance, tree.props.overrides) - return instance + try { + applyBindings(graph, instance.id, bindings) + applyInstanceOverrides(graph, instance, tree.props.overrides) + assignComponentProperties(graph, instance, props.properties) + return instance + } catch (error) { + graph.deleteNode(instance.id) + throw error + } } /** @@ -465,9 +479,22 @@ function applyInstanceOverrides( } } +async function renderArtworkNode( + graph: SceneGraph, + tree: TreeNode, + parentId: string +): Promise { + const metadata = componentMetadata(tree.props, 'VECTOR') + const node = + tree.type === 'icon' + ? await renderIconNode(graph, tree, parentId) + : await renderSVGNode(graph, tree, parentId) + if (Object.keys(metadata).length > 0) graph.updateNode(node.id, metadata) + return node +} + async function renderNode(graph: SceneGraph, tree: TreeNode, parentId: string): Promise { - if (tree.type === 'icon') return renderIconNode(graph, tree, parentId) - if (tree.type === 'svg') return renderSVGNode(graph, tree, parentId) + if (tree.type === 'icon' || tree.type === 'svg') return renderArtworkNode(graph, tree, parentId) if (tree.type === 'instance') return renderInstanceNode(graph, tree, parentId) const nodeType = TYPE_MAP[tree.type] @@ -478,7 +505,10 @@ async function renderNode(graph: SceneGraph, tree: TreeNode, parentId: string): const isText = nodeType === 'TEXT' const { props, bindings } = preparePropsForRender(graph, tree.props, isText, parentId) - const overrides = propsToOverrides(props, isText, parentLayout) + const overrides = { + ...propsToOverrides(props, isText, parentLayout), + ...componentMetadata(props, nodeType) + } if (isText) { const childText = tree.children.filter((c): c is string => typeof c === 'string').join('') diff --git a/packages/core/src/design-jsx/schema.ts b/packages/core/src/design-jsx/schema.ts index f398fba03..0f4e2018a 100644 --- a/packages/core/src/design-jsx/schema.ts +++ b/packages/core/src/design-jsx/schema.ts @@ -137,6 +137,8 @@ export const DESIGN_JSX_SUPPORTED_PROPERTY_NAMES = [ 'bind', 'component', 'componentId', + 'properties', + 'propertyRefs', 'of' ] as const diff --git a/packages/core/src/design-jsx/tree.ts b/packages/core/src/design-jsx/tree.ts index cadc5e708..18d56702f 100644 --- a/packages/core/src/design-jsx/tree.ts +++ b/packages/core/src/design-jsx/tree.ts @@ -1,4 +1,4 @@ -import type { Effect, Fill } from '@open-pencil/scene-graph' +import type { Effect, Fill, SceneNode } from '@open-pencil/scene-graph' import type { Color } from '@open-pencil/scene-graph/primitives' import type { DesignVariable } from './vars' @@ -160,12 +160,25 @@ export type StyleProps = { textAutoResize?: 'none' | 'width' | 'height' } -export type BaseProps = StyleProps & { +type NodeProps = StyleProps & { name?: string key?: string | number children?: unknown bind?: Record + propertyRefs?: SceneNode['componentPropertyReferences'] [key: string]: unknown } +export type BaseProps = NodeProps & { properties?: never } export type TextProps = BaseProps + +export type ComponentProps = NodeProps & { + properties?: SceneNode['componentPropertyDefinitions'] +} + +export type InstanceProps = NodeProps & { + component?: string + componentId?: string + of?: string + properties?: SceneNode['componentPropertyAssignments'] +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1ab00050f..1ffa9ec8d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -351,6 +351,8 @@ export { node, type TreeNode, type BaseProps, + type ComponentProps, + type InstanceProps, type TextProps, type StyleProps, type PaintProp, diff --git a/packages/docs/reference/design-authoring.md b/packages/docs/reference/design-authoring.md index fe1516998..b6af7e3ce 100644 --- a/packages/docs/reference/design-authoring.md +++ b/packages/docs/reference/design-authoring.md @@ -33,7 +33,10 @@ This reference describes scene creation, not React DOM output. Use the `render` - `bind` maps supported scene-field paths to variable IDs or references when no shorthand exists. Use semantic tokens consistently rather than declaring unused collections. - A reusable JavaScript function shares source code, not component identity. Use `Component`, `ComponentSet`, and `Instance` for editable main components and linked instances. - `Instance` resolves an existing component through `of`, `component`, or `componentId`. Component-set children named `variant=Primary`, for example, define variants that can be selected when instantiating the set. -- Reuse existing local or library components before recreating them. Expose meaningful text, visibility, and swap properties through the existing component-property APIs; do not assume every native property API is already exposed as a JSX prop. +- `Component` and `ComponentSet` accept `properties`, an array of native property definitions (`id`, `name`, `type`, `defaultValue`). `Instance` accepts `properties`, an ID-to-value assignment object. Ordinary nodes do not accept `properties`. +- Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. +- Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. +- Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. ## Verification @@ -66,4 +69,4 @@ Generated from the renderer metadata. This inventory lists accepted names, not a **Helpers:** `solid`, `gradient`, `linearGradient`, `radialGradient`, `angularGradient`, `diamondGradient`, `dropShadow`, `innerShadow`, `layerBlur`, `backgroundBlur`, `foregroundBlur`, `designVar`, `defineVars`. -**Properties:** `name`, `key`, `flex`, `flow`, `dir`, `gap`, `wrap`, `rowGap`, `columnGap`, `justify`, `justifyContent`, `items`, `align`, `alignItems`, `grow`, `w`, `h`, `width`, `height`, `minW`, `maxW`, `minH`, `maxH`, `x`, `y`, `top`, `left`, `position`, `p`, `padding`, `px`, `py`, `pt`, `pr`, `pb`, `pl`, `bg`, `fill`, `fills`, `background`, `backgroundColor`, `stroke`, `border`, `borderColor`, `strokeWidth`, `borderWidth`, `strokeAlign`, `strokeDash`, `rounded`, `borderRadius`, `roundedTL`, `roundedTR`, `roundedBL`, `roundedBR`, `cornerRadius`, `cornerSmoothing`, `opacity`, `blendMode`, `rotate`, `rotation`, `overflow`, `shadow`, `blur`, `effects`, `size`, `fontSize`, `font`, `fontFamily`, `weight`, `fontWeight`, `color`, `text`, `characters`, `content`, `value`, `title`, `textAlign`, `textAlignHorizontal`, `textHorizontalAlignment`, `textAlignVertical`, `textVerticalAlignment`, `textAutoResize`, `lineHeight`, `letterSpacing`, `textDecoration`, `textCase`, `maxLines`, `truncate`, `grid`, `columns`, `rows`, `colStart`, `rowStart`, `col`, `row`, `colSpan`, `rowSpan`, `points`, `pointCount`, `innerRadius`, `label`, `style`, `bind`, `component`, `componentId`, `of`. \ No newline at end of file +**Properties:** `name`, `key`, `flex`, `flow`, `dir`, `gap`, `wrap`, `rowGap`, `columnGap`, `justify`, `justifyContent`, `items`, `align`, `alignItems`, `grow`, `w`, `h`, `width`, `height`, `minW`, `maxW`, `minH`, `maxH`, `x`, `y`, `top`, `left`, `position`, `p`, `padding`, `px`, `py`, `pt`, `pr`, `pb`, `pl`, `bg`, `fill`, `fills`, `background`, `backgroundColor`, `stroke`, `border`, `borderColor`, `strokeWidth`, `borderWidth`, `strokeAlign`, `strokeDash`, `rounded`, `borderRadius`, `roundedTL`, `roundedTR`, `roundedBL`, `roundedBR`, `cornerRadius`, `cornerSmoothing`, `opacity`, `blendMode`, `rotate`, `rotation`, `overflow`, `shadow`, `blur`, `effects`, `size`, `fontSize`, `font`, `fontFamily`, `weight`, `fontWeight`, `color`, `text`, `characters`, `content`, `value`, `title`, `textAlign`, `textAlignHorizontal`, `textHorizontalAlignment`, `textAlignVertical`, `textVerticalAlignment`, `textAutoResize`, `lineHeight`, `letterSpacing`, `textDecoration`, `textCase`, `maxLines`, `truncate`, `grid`, `columns`, `rows`, `colStart`, `rowStart`, `col`, `row`, `colSpan`, `rowSpan`, `points`, `pointCount`, `innerRadius`, `label`, `style`, `bind`, `component`, `componentId`, `properties`, `propertyRefs`, `of`. \ No newline at end of file diff --git a/skills/open-pencil/references/design-authoring.md b/skills/open-pencil/references/design-authoring.md index fe1516998..b6af7e3ce 100644 --- a/skills/open-pencil/references/design-authoring.md +++ b/skills/open-pencil/references/design-authoring.md @@ -33,7 +33,10 @@ This reference describes scene creation, not React DOM output. Use the `render` - `bind` maps supported scene-field paths to variable IDs or references when no shorthand exists. Use semantic tokens consistently rather than declaring unused collections. - A reusable JavaScript function shares source code, not component identity. Use `Component`, `ComponentSet`, and `Instance` for editable main components and linked instances. - `Instance` resolves an existing component through `of`, `component`, or `componentId`. Component-set children named `variant=Primary`, for example, define variants that can be selected when instantiating the set. -- Reuse existing local or library components before recreating them. Expose meaningful text, visibility, and swap properties through the existing component-property APIs; do not assume every native property API is already exposed as a JSX prop. +- `Component` and `ComponentSet` accept `properties`, an array of native property definitions (`id`, `name`, `type`, `defaultValue`). `Instance` accepts `properties`, an ID-to-value assignment object. Ordinary nodes do not accept `properties`. +- Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. +- Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. +- Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. ## Verification @@ -66,4 +69,4 @@ Generated from the renderer metadata. This inventory lists accepted names, not a **Helpers:** `solid`, `gradient`, `linearGradient`, `radialGradient`, `angularGradient`, `diamondGradient`, `dropShadow`, `innerShadow`, `layerBlur`, `backgroundBlur`, `foregroundBlur`, `designVar`, `defineVars`. -**Properties:** `name`, `key`, `flex`, `flow`, `dir`, `gap`, `wrap`, `rowGap`, `columnGap`, `justify`, `justifyContent`, `items`, `align`, `alignItems`, `grow`, `w`, `h`, `width`, `height`, `minW`, `maxW`, `minH`, `maxH`, `x`, `y`, `top`, `left`, `position`, `p`, `padding`, `px`, `py`, `pt`, `pr`, `pb`, `pl`, `bg`, `fill`, `fills`, `background`, `backgroundColor`, `stroke`, `border`, `borderColor`, `strokeWidth`, `borderWidth`, `strokeAlign`, `strokeDash`, `rounded`, `borderRadius`, `roundedTL`, `roundedTR`, `roundedBL`, `roundedBR`, `cornerRadius`, `cornerSmoothing`, `opacity`, `blendMode`, `rotate`, `rotation`, `overflow`, `shadow`, `blur`, `effects`, `size`, `fontSize`, `font`, `fontFamily`, `weight`, `fontWeight`, `color`, `text`, `characters`, `content`, `value`, `title`, `textAlign`, `textAlignHorizontal`, `textHorizontalAlignment`, `textAlignVertical`, `textVerticalAlignment`, `textAutoResize`, `lineHeight`, `letterSpacing`, `textDecoration`, `textCase`, `maxLines`, `truncate`, `grid`, `columns`, `rows`, `colStart`, `rowStart`, `col`, `row`, `colSpan`, `rowSpan`, `points`, `pointCount`, `innerRadius`, `label`, `style`, `bind`, `component`, `componentId`, `of`. \ No newline at end of file +**Properties:** `name`, `key`, `flex`, `flow`, `dir`, `gap`, `wrap`, `rowGap`, `columnGap`, `justify`, `justifyContent`, `items`, `align`, `alignItems`, `grow`, `w`, `h`, `width`, `height`, `minW`, `maxW`, `minH`, `maxH`, `x`, `y`, `top`, `left`, `position`, `p`, `padding`, `px`, `py`, `pt`, `pr`, `pb`, `pl`, `bg`, `fill`, `fills`, `background`, `backgroundColor`, `stroke`, `border`, `borderColor`, `strokeWidth`, `borderWidth`, `strokeAlign`, `strokeDash`, `rounded`, `borderRadius`, `roundedTL`, `roundedTR`, `roundedBL`, `roundedBR`, `cornerRadius`, `cornerSmoothing`, `opacity`, `blendMode`, `rotate`, `rotation`, `overflow`, `shadow`, `blur`, `effects`, `size`, `fontSize`, `font`, `fontFamily`, `weight`, `fontWeight`, `color`, `text`, `characters`, `content`, `value`, `title`, `textAlign`, `textAlignHorizontal`, `textHorizontalAlignment`, `textAlignVertical`, `textVerticalAlignment`, `textAutoResize`, `lineHeight`, `letterSpacing`, `textDecoration`, `textCase`, `maxLines`, `truncate`, `grid`, `columns`, `rows`, `colStart`, `rowStart`, `col`, `row`, `colSpan`, `rowSpan`, `points`, `pointCount`, `innerRadius`, `label`, `style`, `bind`, `component`, `componentId`, `properties`, `propertyRefs`, `of`. \ No newline at end of file diff --git a/tests/engine/render/jsx/component-properties.test.ts b/tests/engine/render/jsx/component-properties.test.ts new file mode 100644 index 000000000..172e2f2a4 --- /dev/null +++ b/tests/engine/render/jsx/component-properties.test.ts @@ -0,0 +1,177 @@ +import { expect, expectTypeOf, test } from 'bun:test' + +import { + Component, + ComponentSet, + type Frame, + Instance, + Text, + node, + renderTree +} from '@open-pencil/core/design-jsx' +import type { ComponentPropertyDefinition, SceneNode } from '@open-pencil/scene-graph' + +import { getNodeOrThrow } from '#tests/helpers/assert' +import { makeSceneGraph } from '#tests/helpers/scene' + +test('property props are scoped to their authoring entities', () => { + expectTypeOf[0]['properties']>().toEqualTypeOf() + expectTypeOf[0]['properties']>().toEqualTypeOf() + expectTypeOf[0]['properties']>().toEqualTypeOf< + SceneNode['componentPropertyDefinitions'] | undefined + >() + expectTypeOf[0]['properties']>().toEqualTypeOf< + SceneNode['componentPropertyAssignments'] | undefined + >() +}) + +const MESSAGE: ComponentPropertyDefinition = { + id: 'message', + name: 'Message', + type: 'TEXT', + defaultValue: 'Default' +} +const VISIBLE: ComponentPropertyDefinition = { + id: 'visible', + name: 'Visible', + type: 'BOOLEAN', + defaultValue: 'true' +} + +async function setup() { + const graph = makeSceneGraph() + const component = await renderTree( + graph, + Component({ + name: 'Note', + flex: 'col', + w: 280, + h: 'hug', + properties: [MESSAGE, VISIBLE], + children: [ + Text({ + name: 'Duplicate name', + children: 'Default', + w: 'fill', + color: '#111111', + propertyRefs: [ + { propertyId: MESSAGE.id, field: 'TEXT' }, + { propertyId: VISIBLE.id, field: 'VISIBLE' } + ] + }), + Text({ name: 'Duplicate name', children: 'Unrelated', color: '#111111' }) + ] + }) + ) + return { graph, component } +} + +test('assigns real text and visibility properties without resolving child names', async () => { + const { graph, component } = await setup() + const result = await renderTree( + graph, + Instance({ of: component.id, properties: { message: 'June’s review', visible: 'false' } }) + ) + const instance = getNodeOrThrow(graph, result.id) + const children = graph.getChildren(instance.id) + expect(children.map((child) => child.text)).toEqual(['June’s review', 'Unrelated']) + expect(children[0].visible).toBe(false) + expect(instance.componentPropertyAssignments).toEqual({ + message: 'June’s review', + visible: 'false' + }) + graph.syncInstances(component.id) + expect(graph.getChildren(instance.id)[0].text).toBe('June’s review') + expect(graph.getChildren(instance.id)[0].visible).toBe(false) + expect(graph.getChildren(component.id)[0].text).toBe('Default') +}) + +test('variant selection ignores non-variant instance props', async () => { + const graph = makeSceneGraph() + const set = await renderTree( + graph, + ComponentSet({ + name: 'Button', + children: [ + Component({ name: 'variant=Primary', w: 100, h: 40 }), + Component({ + name: 'variant=Secondary', + w: 100, + h: 40, + properties: [MESSAGE], + children: Text({ + children: 'Default', + propertyRefs: [{ propertyId: MESSAGE.id, field: 'TEXT' }] + }) + }) + ] + }) + ) + const result = await renderTree( + graph, + Instance({ of: set.id, variant: 'Secondary', w: 120, properties: { message: 'Changed' } }) + ) + const instance = getNodeOrThrow(graph, result.id) + expect(instance.componentId).toBe(graph.getChildren(set.id)[1].id) + expect(graph.getChildren(instance.id)[0].text).toBe('Changed') +}) + +test('invalid assignments remove the new instance and do not edit its source', async () => { + const { graph, component } = await setup() + const count = graph.nodes.size + for (const assignments of [ + { missing: 'value' }, + { visible: 'yes' }, + { message: 'Changed', missing: 'value' } + ]) { + await expect( + renderTree(graph, Instance({ of: component.id, properties: assignments })) + ).rejects.toThrow() + expect(graph.nodes.size).toBe(count) + expect(graph.getChildren(component.id)[0].text).toBe('Default') + } +}) + +test('assigns nested instance swaps through stable property references', async () => { + const graph = makeSceneGraph() + const first = await renderTree(graph, Component({ name: 'First', w: 20, h: 20 })) + const second = await renderTree(graph, Component({ name: 'Second', w: 30, h: 30 })) + const component = await renderTree( + graph, + Component({ + name: 'With slot', + properties: [{ id: 'slot', name: 'Slot', type: 'INSTANCE_SWAP', defaultValue: first.id }], + children: Instance({ + of: first.id, + propertyRefs: [{ propertyId: 'slot', field: 'INSTANCE_SWAP' }] + }) + }) + ) + const result = await renderTree( + graph, + Instance({ of: component.id, properties: { slot: second.id } }) + ) + expect(graph.getChildren(result.id)[0].componentId).toBe(second.id) + graph.syncInstances(component.id) + expect(graph.getChildren(result.id)[0].componentId).toBe(second.id) + expect(graph.getChildren(component.id)[0].componentId).toBe(first.id) +}) + +test('ordinary elements reject properties before rendering', async () => { + const graph = makeSceneGraph() + for (const type of ['frame', 'text', 'rectangle', 'icon', 'svg']) { + await expect(renderTree(graph, node(type, { properties: [MESSAGE] }))).rejects.toThrow( + 'Only components' + ) + } +}) + +test('validates metadata rather than accepting silent malformed property definitions', async () => { + const graph = makeSceneGraph() + await expect(renderTree(graph, node('text', { properties: [MESSAGE] }))).rejects.toThrow( + 'Only components' + ) + await expect(renderTree(graph, Component({ properties: [MESSAGE, MESSAGE] }))).rejects.toThrow( + 'Duplicate' + ) +}) From 69703abc7725d071cdbddf5d6d5933ecafa40633 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 13 Sep 2026 21:19:34 +0300 Subject: [PATCH 2/6] fix: preserve variant selection with declared set properties --- packages/core/src/design-jsx/renderer.ts | 20 ++++--- .../jsx/component-set-properties.test.ts | 53 +++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 tests/engine/render/jsx/component-set-properties.test.ts diff --git a/packages/core/src/design-jsx/renderer.ts b/packages/core/src/design-jsx/renderer.ts index 7c6476f7a..33f1c983b 100644 --- a/packages/core/src/design-jsx/renderer.ts +++ b/packages/core/src/design-jsx/renderer.ts @@ -315,7 +315,7 @@ function parseVariantValues(name: string): Record { function inferComponentSetProperties(graph: SceneGraph, componentSetId: string): void { const componentSet = graph.getNode(componentSetId) if (componentSet?.type !== 'COMPONENT_SET') return - if (componentSet.componentPropertyDefinitions.length > 0) return + const existingDefinitions = componentSet.componentPropertyDefinitions const variants = graph.getChildren(componentSetId).filter((node) => node.type === 'COMPONENT') const options = new Map>() @@ -334,8 +334,14 @@ function inferComponentSetProperties(graph: SceneGraph, componentSetId: string): } } - const definitions: ComponentPropertyDefinition[] = [...options.entries()].map( - ([name, values]) => { + const definitions: ComponentPropertyDefinition[] = [...options.entries()] + .filter( + ([name]) => + !existingDefinitions.some( + (definition) => definition.type === 'VARIANT' && definition.name === name + ) + ) + .map(([name, values]) => { const variantOptions = [...values] return { id: `prop:${randomHex(8)}`, @@ -344,14 +350,14 @@ function inferComponentSetProperties(graph: SceneGraph, componentSetId: string): defaultValue: variantOptions[0] ?? '', variantOptions } - } - ) - if (definitions.length === 0) return + }) for (const [id, values] of valuesById) { graph.updateNode(id, { componentPropertyValues: values }) } - graph.updateNode(componentSetId, { componentPropertyDefinitions: definitions }) + graph.updateNode(componentSetId, { + componentPropertyDefinitions: [...existingDefinitions, ...definitions] + }) } function findComponentByName(graph: SceneGraph, name: string): SceneNode | undefined { diff --git a/tests/engine/render/jsx/component-set-properties.test.ts b/tests/engine/render/jsx/component-set-properties.test.ts new file mode 100644 index 000000000..51fe6dacb --- /dev/null +++ b/tests/engine/render/jsx/component-set-properties.test.ts @@ -0,0 +1,53 @@ +import { expect, test } from 'bun:test' + +import { Component, ComponentSet, Instance, Text, renderTree } from '@open-pencil/core/design-jsx' +import type { ComponentPropertyDefinition } from '@open-pencil/scene-graph' + +import { getNodeOrThrow } from '#tests/helpers/assert' +import { makeSceneGraph } from '#tests/helpers/scene' + +const MESSAGE: ComponentPropertyDefinition = { + id: 'message', + name: 'Message', + type: 'TEXT', + defaultValue: 'Default' +} +const VARIANT: ComponentPropertyDefinition = { + id: 'variant', + name: 'variant', + type: 'VARIANT', + defaultValue: 'Primary', + variantOptions: ['Primary', 'Secondary'] +} + +for (const explicitVariant of [false, true]) { + test(`component-set properties preserve ${explicitVariant ? 'explicit' : 'inferred'} variant selection`, async () => { + const graph = makeSceneGraph() + const set = await renderTree( + graph, + ComponentSet({ + properties: explicitVariant ? [MESSAGE, VARIANT] : [MESSAGE], + children: ['Primary', 'Secondary'].map((variant) => + Component({ + name: `variant=${variant}`, + children: Text({ + children: 'Default', + propertyRefs: [{ propertyId: MESSAGE.id, field: 'TEXT' }] + }) + }) + ) + }) + ) + const result = await renderTree( + graph, + Instance({ of: set.id, variant: 'Secondary', properties: { message: 'Changed' } }) + ) + const instance = getNodeOrThrow(graph, result.id) + expect(instance.componentId).toBe(graph.getChildren(set.id)[1].id) + expect(graph.getChildren(instance.id)[0].text).toBe('Changed') + const definitions = getNodeOrThrow(graph, set.id).componentPropertyDefinitions + expect(definitions).toContainEqual(MESSAGE) + expect(definitions.filter((definition) => definition.type === 'VARIANT')).toHaveLength(1) + if (explicitVariant) expect(definitions).toContainEqual(VARIANT) + }) +} From 243eaf9626d02912a462bb18817fec1337c6e5f0 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 13 Sep 2026 23:43:07 +0300 Subject: [PATCH 3/6] fix: declare the Core component validation dependency Declare Valibot in Core so installed artifacts resolve it by package name instead of retaining a workspace-only Bun cache path. --- bun.lock | 1 + packages/core/package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/bun.lock b/bun.lock index 1a6a7ee8b..819236e69 100644 --- a/bun.lock +++ b/bun.lock @@ -203,6 +203,7 @@ "svgpath": "^2.6.0", "twirlwind": "^0.3.0", "unifont": "0.7.4", + "valibot": "^1.4.2", "yoga-layout": "npm:@open-pencil/yoga-layout@3.3.0-grid.3", }, "devDependencies": { diff --git a/packages/core/package.json b/packages/core/package.json index 268cba1a0..2fba31403 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -205,6 +205,7 @@ "svgpath": "^2.6.0", "twirlwind": "^0.3.0", "unifont": "0.7.4", + "valibot": "^1.4.2", "yoga-layout": "npm:@open-pencil/yoga-layout@3.3.0-grid.3" }, "devDependencies": { From 0a6ce6e72d0bd08985ba3380e54b5a8ac7d211f6 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 13 Sep 2026 23:57:24 +0300 Subject: [PATCH 4/6] fix: validate component property reference scopes Reject unknown or incompatible references while the owning component definitions are available, including inherited component-set definitions. Keep nested component scopes separate. --- .../src/design-jsx/component-properties.ts | 31 ++++++++- packages/core/src/design-jsx/renderer.ts | 12 ++-- .../jsx/component-property-references.test.ts | 65 +++++++++++++++++++ 3 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 tests/engine/render/jsx/component-property-references.test.ts diff --git a/packages/core/src/design-jsx/component-properties.ts b/packages/core/src/design-jsx/component-properties.ts index 0a0d01c4b..905395a8f 100644 --- a/packages/core/src/design-jsx/component-properties.ts +++ b/packages/core/src/design-jsx/component-properties.ts @@ -24,7 +24,8 @@ const referenceSchema = v.object({ /** Accept the native graph contracts, without introducing a second property model. */ export function componentMetadata( props: Record, - type: SceneNode['type'] + type: SceneNode['type'], + definitions?: readonly ComponentPropertyDefinition[] ): Partial { const result: Partial = {} if (props.properties !== undefined && type !== 'INSTANCE') { @@ -42,12 +43,40 @@ export function componentMetadata( throw new Error('TEXT properties require a text node') if (reference.field === 'INSTANCE_SWAP' && type !== 'INSTANCE') throw new Error('INSTANCE_SWAP properties require an instance') + if (definitions) { + const definition = definitions.find((item) => item.id === reference.propertyId) + if (!definition) + throw new Error(`Unknown component property reference: ${reference.propertyId}`) + const expectedField = definition.type === 'BOOLEAN' ? 'VISIBLE' : definition.type + if (reference.field !== expectedField) + throw new Error(`Component property ${definition.id} cannot bind ${reference.field}`) + } } result.componentPropertyReferences = references } return result } +/** Match the nearest component scope, including definitions inherited from its set. */ +export function componentPropertyScope( + graph: SceneGraph, + parentId: string +): readonly ComponentPropertyDefinition[] | undefined { + let parent = graph.getNode(parentId) + while (parent) { + if (parent.type === 'INSTANCE') return componentPropertyDefinitions(graph, parent) + if (parent.type === 'COMPONENT_SET') return parent.componentPropertyDefinitions + if (parent.type === 'COMPONENT') { + const set = parent.parentId ? graph.getNode(parent.parentId) : undefined + return set?.type === 'COMPONENT_SET' + ? [...set.componentPropertyDefinitions, ...parent.componentPropertyDefinitions] + : parent.componentPropertyDefinitions + } + parent = parent.parentId ? graph.getNode(parent.parentId) : undefined + } + return undefined +} + export function assignComponentProperties( graph: SceneGraph, instance: SceneNode, diff --git a/packages/core/src/design-jsx/renderer.ts b/packages/core/src/design-jsx/renderer.ts index 33f1c983b..927e51365 100644 --- a/packages/core/src/design-jsx/renderer.ts +++ b/packages/core/src/design-jsx/renderer.ts @@ -16,7 +16,11 @@ import type { IconData } from '#core/icons/types' import { computeAllLayouts } from '#core/layout' import { randomHex } from '#core/random' -import { assignComponentProperties, componentMetadata } from './component-properties' +import { + assignComponentProperties, + componentMetadata, + componentPropertyScope +} from './component-properties' import { applySizeOverrides, propsToOverrides } from './props-overrides' import { prepareScalarBindings } from './scalar-bindings' import { isTreeNode } from './tree' @@ -428,7 +432,7 @@ async function renderInstanceNode( } const overrides = { ...propsToOverrides(props, false, parentLayout), - ...componentMetadata(props, 'INSTANCE') + ...componentMetadata(props, 'INSTANCE', componentPropertyScope(graph, parentId)) } const instance = graph.createInstance(component.id, parentId, overrides) ?? graph.createNode('FRAME', parentId) @@ -490,7 +494,7 @@ async function renderArtworkNode( tree: TreeNode, parentId: string ): Promise { - const metadata = componentMetadata(tree.props, 'VECTOR') + const metadata = componentMetadata(tree.props, 'VECTOR', componentPropertyScope(graph, parentId)) const node = tree.type === 'icon' ? await renderIconNode(graph, tree, parentId) @@ -513,7 +517,7 @@ async function renderNode(graph: SceneGraph, tree: TreeNode, parentId: string): const { props, bindings } = preparePropsForRender(graph, tree.props, isText, parentId) const overrides = { ...propsToOverrides(props, isText, parentLayout), - ...componentMetadata(props, nodeType) + ...componentMetadata(props, nodeType, componentPropertyScope(graph, parentId)) } if (isText) { diff --git a/tests/engine/render/jsx/component-property-references.test.ts b/tests/engine/render/jsx/component-property-references.test.ts new file mode 100644 index 000000000..4a1e6e25a --- /dev/null +++ b/tests/engine/render/jsx/component-property-references.test.ts @@ -0,0 +1,65 @@ +import { expect, test } from 'bun:test' + +import { Component, ComponentSet, Frame, Text, renderTree } from '@open-pencil/core/design-jsx' +import type { ComponentPropertyDefinition } from '@open-pencil/scene-graph' + +import { makeSceneGraph } from '#tests/helpers/scene' + +const definitions: ComponentPropertyDefinition[] = [ + { id: 'caption', name: 'Caption', type: 'TEXT', defaultValue: 'Hello' }, + { id: 'visible', name: 'Visible', type: 'BOOLEAN', defaultValue: 'true' } +] + +for (const reference of [ + { propertyId: 'missing', field: 'TEXT' }, + { propertyId: 'visible', field: 'TEXT' }, + { propertyId: 'caption', field: 'VISIBLE' } +] as const) { + test(`rejects invalid scoped reference ${reference.propertyId}/${reference.field}`, async () => { + await expect( + renderTree( + makeSceneGraph(), + Component({ + properties: definitions, + children: Frame({ children: Text({ children: 'Hello', propertyRefs: [reference] }) }) + }) + ) + ).rejects.toThrow(/component property/i) + }) +} + +test('accepts references inherited from the owning component set', async () => { + const result = await renderTree( + makeSceneGraph(), + ComponentSet({ + properties: definitions, + children: Component({ + name: 'Kind=Default', + children: Frame({ + children: Text({ + children: 'Hello', + propertyRefs: [{ propertyId: 'caption', field: 'TEXT' }] + }) + }) + }) + }) + ) + expect(result.type).toBe('COMPONENT_SET') +}) + +test('does not resolve a nested component reference against the outer component', async () => { + await expect( + renderTree( + makeSceneGraph(), + Component({ + properties: definitions, + children: Component({ + children: Text({ + children: 'Hello', + propertyRefs: [{ propertyId: 'caption', field: 'TEXT' }] + }) + }) + }) + ) + ).rejects.toThrow('Unknown component property reference: caption') +}) From 27e3bc302315ab33c7958256febd0b6d1840fae6 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Mon, 14 Sep 2026 00:01:41 +0300 Subject: [PATCH 5/6] test: group JSX component contracts by domain --- .../properties.test.ts} | 0 .../property-references.test.ts} | 0 .../set-properties.test.ts} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename tests/engine/render/jsx/{component-properties.test.ts => component/properties.test.ts} (100%) rename tests/engine/render/jsx/{component-property-references.test.ts => component/property-references.test.ts} (100%) rename tests/engine/render/jsx/{component-set-properties.test.ts => component/set-properties.test.ts} (100%) diff --git a/tests/engine/render/jsx/component-properties.test.ts b/tests/engine/render/jsx/component/properties.test.ts similarity index 100% rename from tests/engine/render/jsx/component-properties.test.ts rename to tests/engine/render/jsx/component/properties.test.ts diff --git a/tests/engine/render/jsx/component-property-references.test.ts b/tests/engine/render/jsx/component/property-references.test.ts similarity index 100% rename from tests/engine/render/jsx/component-property-references.test.ts rename to tests/engine/render/jsx/component/property-references.test.ts diff --git a/tests/engine/render/jsx/component-set-properties.test.ts b/tests/engine/render/jsx/component/set-properties.test.ts similarity index 100% rename from tests/engine/render/jsx/component-set-properties.test.ts rename to tests/engine/render/jsx/component/set-properties.test.ts From 7b16881e03633ef96a58b841241e2eccbe0957d5 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Mon, 14 Sep 2026 00:14:09 +0300 Subject: [PATCH 6/6] fix: reject ambiguous variant property names --- .../src/design-jsx/component-properties.ts | 5 ++++ .../render/jsx/component/properties.test.ts | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/core/src/design-jsx/component-properties.ts b/packages/core/src/design-jsx/component-properties.ts index 905395a8f..779f7a3db 100644 --- a/packages/core/src/design-jsx/component-properties.ts +++ b/packages/core/src/design-jsx/component-properties.ts @@ -34,6 +34,11 @@ export function componentMetadata( const definitions = v.parse(v.array(definitionSchema), props.properties) if (new Set(definitions.map((definition) => definition.id)).size !== definitions.length) throw new Error('Duplicate component property IDs') + const variantNames = definitions + .filter((item) => item.type === 'VARIANT') + .map((item) => item.name) + if (new Set(variantNames).size !== variantNames.length) + throw new Error('Duplicate variant property names') result.componentPropertyDefinitions = definitions } if (props.propertyRefs !== undefined) { diff --git a/tests/engine/render/jsx/component/properties.test.ts b/tests/engine/render/jsx/component/properties.test.ts index 172e2f2a4..ef5bf1d01 100644 --- a/tests/engine/render/jsx/component/properties.test.ts +++ b/tests/engine/render/jsx/component/properties.test.ts @@ -174,4 +174,28 @@ test('validates metadata rather than accepting silent malformed property definit await expect(renderTree(graph, Component({ properties: [MESSAGE, MESSAGE] }))).rejects.toThrow( 'Duplicate' ) + for (const element of [Component, ComponentSet]) { + await expect( + renderTree( + graph, + element({ + properties: [ + { id: 'kind-a', name: 'Kind', type: 'VARIANT', defaultValue: 'A' }, + { id: 'kind-b', name: 'Kind', type: 'VARIANT', defaultValue: 'B' } + ] + }) + ) + ).rejects.toThrow('Duplicate variant property names') + } +}) + +test('preserves duplicate display names for non-variant properties with distinct IDs', async () => { + const graph = makeSceneGraph() + const result = await renderTree( + graph, + Component({ properties: [MESSAGE, { ...MESSAGE, id: 'other-message' }] }) + ) + expect( + getNodeOrThrow(graph, result.id).componentPropertyDefinitions.map((item) => item.id) + ).toEqual(['message', 'other-message']) })