diff --git a/.changeset/fix-text-binding-state-prop-observer.md b/.changeset/fix-text-binding-state-prop-observer.md new file mode 100644 index 00000000..58a83d82 --- /dev/null +++ b/.changeset/fix-text-binding-state-prop-observer.md @@ -0,0 +1,6 @@ +--- +"@geajs/core": patch +"@geajs/vite-plugin": patch +--- + +Fix state observer for text nodes with mixed state+prop dependencies: When a text node expression referenced both reactive state (e.g. `this.valueAsString`) and props (e.g. `props.placeholder`), the compiler reused the prop-change handler's expression for the state observer. In that expression `value` represented the new prop value — but in the state observer `value` is the new state value, causing props to read as empty. The compiler now generates a separate stateOnly binding for the state observer, so `this.props.X` is read live instead of being replaced with `value`. diff --git a/packages/vite-plugin-gea/src/analyze/binding-resolver.ts b/packages/vite-plugin-gea/src/analyze/binding-resolver.ts index b5044841..d93078f9 100644 --- a/packages/vite-plugin-gea/src/analyze/binding-resolver.ts +++ b/packages/vite-plugin-gea/src/analyze/binding-resolver.ts @@ -91,6 +91,11 @@ export function collectExpressionDependencies( return Array.from(dependencies.values()) } +/** Returns true when a dependency is reactive state (not a prop dependency). */ +export function isStateDep(d: ObserveDependency): boolean { + return d.storeVar !== undefined || (d.pathParts.length > 0 && d.pathParts[0] !== 'props') +} + function collectExpressionDependenciesInto( expr: t.Expression, stateRefs: Map | undefined, diff --git a/packages/vite-plugin-gea/src/analyze/template-walker.ts b/packages/vite-plugin-gea/src/analyze/template-walker.ts index 42aa3d73..3950deb0 100644 --- a/packages/vite-plugin-gea/src/analyze/template-walker.ts +++ b/packages/vite-plugin-gea/src/analyze/template-walker.ts @@ -31,7 +31,7 @@ import { extractKeyExpression, ITEM_IS_KEY, } from './helpers.ts' -import { collectExpressionDependencies, collectTemplateSetupStatements } from './binding-resolver.ts' +import { collectExpressionDependencies, collectTemplateSetupStatements, isStateDep } from './binding-resolver.ts' import type { StateRefMeta } from '../parse/state-refs.ts' import { resolveHelperCallExpression, @@ -92,9 +92,7 @@ export function analyzeAttributes( if (templateSetupContext && !t.isJSXEmptyExpression(expr)) { const setupStatements = collectTemplateSetupStatements(expr, templateSetupContext) const dependencies = collectExpressionDependencies(expr, stateRefs, setupStatements) - const stateDeps = dependencies.filter( - (d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props'), - ) + const stateDeps = dependencies.filter(isStateDep) if (stateDeps.length > 0) { const selector = generateSelector(elementPath) propBindings.push({ @@ -153,7 +151,7 @@ export function analyzeAttributes( if (isNativeElement && templateSetupContext && !t.isJSXEmptyExpression(expr)) { const setupStatements = collectTemplateSetupStatements(expr, templateSetupContext) const dependencies = collectExpressionDependencies(expr, stateRefs, setupStatements) - const stateDeps = dependencies.filter((d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props')) + const stateDeps = dependencies.filter(isStateDep) if (stateDeps.length > 0) { const selector = generateSelector(elementPath) propBindings.push({ @@ -793,6 +791,22 @@ function handleTextBinding( ...(textNodeIndex !== undefined ? { textNodeIndex } : {}), })), ) + const templateStateDeps = collectExpressionDependencies(derivedTemplateExpr, stateRefs, setupStatements).filter( + isStateDep, + ) + if (templateStateDeps.length > 0) { + const stateOnlyBinding: PropBinding = { + propName: '__state__', + selector, + type: 'text', + elementPath: [...elementPath], + expression: t.cloneNode(derivedTemplateExpr, true) as t.Expression, + setupStatements: setupStatements.map((s) => t.cloneNode(s, true) as t.Statement), + stateOnly: true, + } + if (textNodeIndex !== undefined) stateOnlyBinding.textNodeIndex = textNodeIndex + propBindings.push(stateOnlyBinding) + } return } } @@ -811,6 +825,26 @@ function handleTextBinding( for (const d of derived) d.textNodeIndex = textNodeIndex } propBindings.push(...derived) + // When the expression also has state deps, add a stateOnly binding so the state + // observer reads this.props.X live rather than inheriting `value` from the prop + // binding's patch body (where `value` is the incoming prop value, not the new state). + const setupStatements = derived[0].setupStatements + const stateDeps = collectExpressionDependencies(expr as t.Expression, stateRefs, setupStatements ?? []).filter( + isStateDep, + ) + if (stateDeps.length > 0) { + const stateOnlyBinding: PropBinding = { + propName: '__state__', + selector: generateSelector(elementPath), + type: 'text', + elementPath: [...elementPath], + expression: t.cloneNode(expr, true) as t.Expression, + setupStatements, + stateOnly: true, + } + if (textNodeIndex !== undefined) stateOnlyBinding.textNodeIndex = textNodeIndex + propBindings.push(stateOnlyBinding) + } return } } @@ -875,7 +909,7 @@ function handleTextBinding( : expr const setupStatements = collectTemplateSetupStatements(exprToUse, templateSetupContext) const dependencies = collectExpressionDependencies(exprToUse, stateRefs, setupStatements) - const stateDeps = dependencies.filter((d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props')) + const stateDeps = dependencies.filter(isStateDep) if (stateDeps.length > 0) { const selector = generateSelector(elementPath) const isChildrenPropBinding = diff --git a/packages/vite-plugin-gea/src/ir/types.ts b/packages/vite-plugin-gea/src/ir/types.ts index 0be9debf..12f1a15c 100644 --- a/packages/vite-plugin-gea/src/ir/types.ts +++ b/packages/vite-plugin-gea/src/ir/types.ts @@ -148,6 +148,7 @@ export interface PropBinding { stateOnly?: boolean /** When true, the binding value contains HTML and must update via innerHTML (not textContent). */ isChildrenProp?: boolean + /** Index of the text node within its parent when the binding targets a specific text node */ textNodeIndex?: number } diff --git a/packages/vite-plugin-gea/tests/regressions/runtime-text-state-and-prop.test.ts b/packages/vite-plugin-gea/tests/regressions/runtime-text-state-and-prop.test.ts new file mode 100644 index 00000000..d3cff210 --- /dev/null +++ b/packages/vite-plugin-gea/tests/regressions/runtime-text-state-and-prop.test.ts @@ -0,0 +1,125 @@ +/** + * Regression: text node with mixed state+prop deps must generate a stateOnly binding + * so the state observer reads this.props.X live instead of inheriting `value` from + * the prop binding's patch body (where `value` is the incoming prop value). + * + * Reproduces the Select placeholder bug: `{this.valueAsString || props.placeholder || 'Select...'}` + * showed 'Select...' after the Zag machine started (setting valueAsString="") because the state + * observer for valueAsString received value="" and the compiled patch body substituted that for + * props.placeholder. Initial HTML rendered correctly; the bug fired on the post-render state flush. + */ + +import assert from 'node:assert/strict' +import { describe, it } from 'node:test' +import { installDom, flushMicrotasks } from '../../../../tests/helpers/jsdom-setup' +import { compileJsxComponent, loadRuntimeModules } from '../helpers/compile' + +const SOURCE = ` + import { Component } from '@geajs/core' + + export default class SelectLike extends Component { + declare valueAsString: string + + onAfterRender(): void { + this.valueAsString = '' + } + + template(props: any) { + return ( +
+ + {this.valueAsString || props.placeholder || 'Select...'} + +
+ ) + } + } +` + +describe('text mixed state+prop regression', () => { + it('prop fallback shown on initial render when state is empty', async () => { + const restoreDom = installDom() + try { + const seed = `text-state-prop-${Date.now()}` + const [{ default: Component }] = await loadRuntimeModules(seed) + const SelectLike = await compileJsxComponent(SOURCE, '/virtual/SelectLike.jsx', 'SelectLike', { Component }) + + const root = document.createElement('div') + document.body.appendChild(root) + + const comp = new SelectLike({ placeholder: 'Pick one...' }) + comp.render(root) + await flushMicrotasks() + + assert.equal( + comp.el.querySelector('.display')?.textContent?.trim(), + 'Pick one...', + 'placeholder must be shown when valueAsString is empty', + ) + + comp.valueAsString = 'Option A' + await flushMicrotasks() + assert.equal(comp.el.querySelector('.display')?.textContent?.trim(), 'Option A', 'selected value must appear') + + comp.valueAsString = '' + await flushMicrotasks() + assert.equal( + comp.el.querySelector('.display')?.textContent?.trim(), + 'Pick one...', + 'placeholder must return when state is cleared', + ) + + comp.dispose() + } finally { + restoreDom() + } + }) + + it('prop change updates text using current state value', async () => { + const restoreDom = installDom() + try { + const seed = `text-state-prop-propchange-${Date.now()}` + const [{ default: Component }, { Store }] = await loadRuntimeModules(seed) + const store = new Store({ placeholder: 'First...' }) + + const SelectLike = await compileJsxComponent(SOURCE, '/virtual/SelectLike.jsx', 'SelectLike', { Component }) + + const Parent = await compileJsxComponent( + ` + import { Component } from '@geajs/core' + import store from './store' + import SelectLike from './SelectLike' + export default class Parent extends Component { + template() { + return + } + } + `, + '/virtual/Parent.jsx', + 'Parent', + { Component, store, SelectLike }, + ) + + const root = document.createElement('div') + document.body.appendChild(root) + + const parent = new Parent() + parent.render(root) + await flushMicrotasks() + + assert.equal(parent.el.querySelector('.display')?.textContent?.trim(), 'First...') + + store.placeholder = 'Second...' + await flushMicrotasks() + assert.equal( + parent.el.querySelector('.display')?.textContent?.trim(), + 'Second...', + 'prop change must update text when state is empty', + ) + + parent.dispose() + } finally { + restoreDom() + } + }) +}) diff --git a/tests/e2e/showcase.spec.ts b/tests/e2e/showcase.spec.ts index ef346c32..9e8b2426 100644 --- a/tests/e2e/showcase.spec.ts +++ b/tests/e2e/showcase.spec.ts @@ -74,6 +74,11 @@ test.describe('showcase component gallery', () => { await expect(page.locator('[data-scope="select"]').first()).toBeVisible() }) + test('select component uses placeholder passed in props', async ({ page }) => { + const select = page.locator('[data-scope="select"]').first() + await expect(select.locator('[data-part="value-text"]')).toHaveText('Pick one...') + }) + test('switch toggles are visible', async ({ page }) => { const switches = page.locator('[data-scope="switch"]') const count = await switches.count()