From 69703abc7725d071cdbddf5d6d5933ecafa40633 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 13 Sep 2026 21:19:34 +0300 Subject: [PATCH] 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) + }) +}