fix(vite-plugin): fix state observer for text nodes with mixed state+prop dependencies

Author: kraus-milan

.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`.

package-lock.json

@@ -40,7 +40,7 @@ import {
   detectContainerSelector,
   hasExplicitItemKey,
 } from './analyze-helpers.ts'
-import { collectExpressionDependencies, collectTemplateSetupStatements } from './transform-attributes.ts'
+import { collectExpressionDependencies, collectTemplateSetupStatements, isStateDep } from './transform-attributes.ts'
 import type { StateRefMeta } from './parse.ts'
 import { createRequire } from 'module'
 
@@ -684,9 +684,7 @@ 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({
@@ -748,7 +746,7 @@ 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({
@@ -1150,7 +1148,7 @@ function handleArrayMap(
       const locStr = loc ? ` (line ${loc.line}, col ${loc.column})` : ''
       const err = new Error(
         `[gea] Array .map() items must have a \`key\` prop on the root element${locStr}. ` +
-          `Add key={item.id} (or another unique identifier) to the outermost JSX element returned by the .map() callback.`,
+        `Add key={item.id} (or another unique identifier) to the outermost JSX element returned by the .map() callback.`,
       )
       ;(err as any).__geaCompileError = true
       throw err
@@ -1462,6 +1460,21 @@ 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
       }
     }
@@ -1480,6 +1493,25 @@ 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
     }
   }
@@ -1546,7 +1578,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)
         propBindings.push({
@@ -1839,13 +1871,13 @@ function collectAllStateAccesses(
   const setupStatements =
     rootExpr && templateSetupContext
       ? (() => {
-          const collected = collectTemplateSetupStatements(rootExpr, templateSetupContext)
-          if (collected.length > 0) return collected.map((s) => t.cloneNode(s, true) as t.Statement)
-          if (templateSetupContext.earlyReturnBarrierIndex === undefined) return []
-          return templateSetupContext.statements
-            .slice(0, templateSetupContext.earlyReturnBarrierIndex + 1)
-            .map((s) => t.cloneNode(s, true) as t.Statement)
-        })()
+        const collected = collectTemplateSetupStatements(rootExpr, templateSetupContext)
+        if (collected.length > 0) return collected.map((s) => t.cloneNode(s, true) as t.Statement)
+        if (templateSetupContext.earlyReturnBarrierIndex === undefined) return []
+        return templateSetupContext.statements
+          .slice(0, templateSetupContext.earlyReturnBarrierIndex + 1)
+          .map((s) => t.cloneNode(s, true) as t.Statement)
+      })()
       : []
   const prog = t.program([...setupStatements, t.expressionStatement(expr)])
 

packages/vite-plugin-gea/src/analyze.ts

@@ -137,6 +137,8 @@ export interface PropBinding {
   userIdExpr?: t.Expression
   /** When true, the binding depends solely on local/imported state, not on props */
   stateOnly?: boolean
+  /** Index of the text node within its parent when the binding targets a specific text node */
+  textNodeIndex?: number
 }
 
 export interface ConditionalSlot {

packages/vite-plugin-gea/src/ir.ts

@@ -89,6 +89,10 @@ export function collectExpressionDependencies(
   return Array.from(dependencies.values())
 }
 
+/** Returns true when a dependency is reactive state (not a prop dependency). */
+export const isStateDep = (d: ObserveDependency): boolean =>
+  d.storeVar !== undefined || (d.pathParts.length > 0 && d.pathParts[0] !== 'props')
+
 function collectExpressionDependenciesInto(
   expr: t.Expression,
   stateRefs: Map<string, StateRefMeta> | undefined,

packages/vite-plugin-gea/src/transform-attributes.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 (
+        <div>
+          <span class="display">
+            {this.valueAsString || props.placeholder || 'Select...'}
+          </span>
+        </div>
+      )
+    }
+  }
+`
+
+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 <SelectLike placeholder={store.placeholder} />
+          }
+        }
+      `,
+        '/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()
+    }
+  })
+})

packages/vite-plugin-gea/tests/regressions/runtime-text-state-and-prop.test.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()