Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 6 additions & 0 deletions .changeset/fix-text-binding-state-prop-observer.md
Original file line number Diff line number Diff line change
@@ -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`.
5 changes: 5 additions & 0 deletions packages/vite-plugin-gea/src/analyze/binding-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, StateRefMeta> | undefined,
Expand Down
46 changes: 40 additions & 6 deletions packages/vite-plugin-gea/src/analyze/template-walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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 =
Expand Down
1 change: 1 addition & 0 deletions packages/vite-plugin-gea/src/ir/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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()
}
})
})
5 changes: 5 additions & 0 deletions tests/e2e/showcase.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading