Skip to content

Commit 0431a5e

Browse files
kraus-milandashersw
authored andcommitted
fix(vite-plugin): fix state observer for text nodes with mixed state+prop dependencies
1 parent 403eca7 commit 0431a5e

6 files changed

Lines changed: 182 additions & 6 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@geajs/core": patch
3+
"@geajs/vite-plugin": patch
4+
---
5+
6+
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`.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ export function collectExpressionDependencies(
9191
return Array.from(dependencies.values())
9292
}
9393

94+
/** Returns true when a dependency is reactive state (not a prop dependency). */
95+
export function isStateDep(d: ObserveDependency): boolean {
96+
return d.storeVar !== undefined || (d.pathParts.length > 0 && d.pathParts[0] !== 'props')
97+
}
98+
9499
function collectExpressionDependenciesInto(
95100
expr: t.Expression,
96101
stateRefs: Map<string, StateRefMeta> | undefined,

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

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
extractKeyExpression,
3232
ITEM_IS_KEY,
3333
} from './helpers.ts'
34-
import { collectExpressionDependencies, collectTemplateSetupStatements } from './binding-resolver.ts'
34+
import { collectExpressionDependencies, collectTemplateSetupStatements, isStateDep } from './binding-resolver.ts'
3535
import type { StateRefMeta } from '../parse/state-refs.ts'
3636
import {
3737
resolveHelperCallExpression,
@@ -92,9 +92,7 @@ export function analyzeAttributes(
9292
if (templateSetupContext && !t.isJSXEmptyExpression(expr)) {
9393
const setupStatements = collectTemplateSetupStatements(expr, templateSetupContext)
9494
const dependencies = collectExpressionDependencies(expr, stateRefs, setupStatements)
95-
const stateDeps = dependencies.filter(
96-
(d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props'),
97-
)
95+
const stateDeps = dependencies.filter(isStateDep)
9896
if (stateDeps.length > 0) {
9997
const selector = generateSelector(elementPath)
10098
propBindings.push({
@@ -153,7 +151,7 @@ export function analyzeAttributes(
153151
if (isNativeElement && templateSetupContext && !t.isJSXEmptyExpression(expr)) {
154152
const setupStatements = collectTemplateSetupStatements(expr, templateSetupContext)
155153
const dependencies = collectExpressionDependencies(expr, stateRefs, setupStatements)
156-
const stateDeps = dependencies.filter((d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props'))
154+
const stateDeps = dependencies.filter(isStateDep)
157155
if (stateDeps.length > 0) {
158156
const selector = generateSelector(elementPath)
159157
propBindings.push({
@@ -793,6 +791,22 @@ function handleTextBinding(
793791
...(textNodeIndex !== undefined ? { textNodeIndex } : {}),
794792
})),
795793
)
794+
const templateStateDeps = collectExpressionDependencies(derivedTemplateExpr, stateRefs, setupStatements).filter(
795+
isStateDep,
796+
)
797+
if (templateStateDeps.length > 0) {
798+
const stateOnlyBinding: PropBinding = {
799+
propName: '__state__',
800+
selector,
801+
type: 'text',
802+
elementPath: [...elementPath],
803+
expression: t.cloneNode(derivedTemplateExpr, true) as t.Expression,
804+
setupStatements: setupStatements.map((s) => t.cloneNode(s, true) as t.Statement),
805+
stateOnly: true,
806+
}
807+
if (textNodeIndex !== undefined) stateOnlyBinding.textNodeIndex = textNodeIndex
808+
propBindings.push(stateOnlyBinding)
809+
}
796810
return
797811
}
798812
}
@@ -811,6 +825,26 @@ function handleTextBinding(
811825
for (const d of derived) d.textNodeIndex = textNodeIndex
812826
}
813827
propBindings.push(...derived)
828+
// When the expression also has state deps, add a stateOnly binding so the state
829+
// observer reads this.props.X live rather than inheriting `value` from the prop
830+
// binding's patch body (where `value` is the incoming prop value, not the new state).
831+
const setupStatements = derived[0].setupStatements
832+
const stateDeps = collectExpressionDependencies(expr as t.Expression, stateRefs, setupStatements ?? []).filter(
833+
isStateDep,
834+
)
835+
if (stateDeps.length > 0) {
836+
const stateOnlyBinding: PropBinding = {
837+
propName: '__state__',
838+
selector: generateSelector(elementPath),
839+
type: 'text',
840+
elementPath: [...elementPath],
841+
expression: t.cloneNode(expr, true) as t.Expression,
842+
setupStatements,
843+
stateOnly: true,
844+
}
845+
if (textNodeIndex !== undefined) stateOnlyBinding.textNodeIndex = textNodeIndex
846+
propBindings.push(stateOnlyBinding)
847+
}
814848
return
815849
}
816850
}
@@ -875,7 +909,7 @@ function handleTextBinding(
875909
: expr
876910
const setupStatements = collectTemplateSetupStatements(exprToUse, templateSetupContext)
877911
const dependencies = collectExpressionDependencies(exprToUse, stateRefs, setupStatements)
878-
const stateDeps = dependencies.filter((d) => d.storeVar || (d.pathParts.length > 0 && d.pathParts[0] !== 'props'))
912+
const stateDeps = dependencies.filter(isStateDep)
879913
if (stateDeps.length > 0) {
880914
const selector = generateSelector(elementPath)
881915
const isChildrenPropBinding =

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ export interface PropBinding {
148148
stateOnly?: boolean
149149
/** When true, the binding value contains HTML and must update via innerHTML (not textContent). */
150150
isChildrenProp?: boolean
151+
/** Index of the text node within its parent when the binding targets a specific text node */
151152
textNodeIndex?: number
152153
}
153154

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* Regression: text node with mixed state+prop deps must generate a stateOnly binding
3+
* so the state observer reads this.props.X live instead of inheriting `value` from
4+
* the prop binding's patch body (where `value` is the incoming prop value).
5+
*
6+
* Reproduces the Select placeholder bug: `{this.valueAsString || props.placeholder || 'Select...'}`
7+
* showed 'Select...' after the Zag machine started (setting valueAsString="") because the state
8+
* observer for valueAsString received value="" and the compiled patch body substituted that for
9+
* props.placeholder. Initial HTML rendered correctly; the bug fired on the post-render state flush.
10+
*/
11+
12+
import assert from 'node:assert/strict'
13+
import { describe, it } from 'node:test'
14+
import { installDom, flushMicrotasks } from '../../../../tests/helpers/jsdom-setup'
15+
import { compileJsxComponent, loadRuntimeModules } from '../helpers/compile'
16+
17+
const SOURCE = `
18+
import { Component } from '@geajs/core'
19+
20+
export default class SelectLike extends Component {
21+
declare valueAsString: string
22+
23+
onAfterRender(): void {
24+
this.valueAsString = ''
25+
}
26+
27+
template(props: any) {
28+
return (
29+
<div>
30+
<span class="display">
31+
{this.valueAsString || props.placeholder || 'Select...'}
32+
</span>
33+
</div>
34+
)
35+
}
36+
}
37+
`
38+
39+
describe('text mixed state+prop regression', () => {
40+
it('prop fallback shown on initial render when state is empty', async () => {
41+
const restoreDom = installDom()
42+
try {
43+
const seed = `text-state-prop-${Date.now()}`
44+
const [{ default: Component }] = await loadRuntimeModules(seed)
45+
const SelectLike = await compileJsxComponent(SOURCE, '/virtual/SelectLike.jsx', 'SelectLike', { Component })
46+
47+
const root = document.createElement('div')
48+
document.body.appendChild(root)
49+
50+
const comp = new SelectLike({ placeholder: 'Pick one...' })
51+
comp.render(root)
52+
await flushMicrotasks()
53+
54+
assert.equal(
55+
comp.el.querySelector('.display')?.textContent?.trim(),
56+
'Pick one...',
57+
'placeholder must be shown when valueAsString is empty',
58+
)
59+
60+
comp.valueAsString = 'Option A'
61+
await flushMicrotasks()
62+
assert.equal(comp.el.querySelector('.display')?.textContent?.trim(), 'Option A', 'selected value must appear')
63+
64+
comp.valueAsString = ''
65+
await flushMicrotasks()
66+
assert.equal(
67+
comp.el.querySelector('.display')?.textContent?.trim(),
68+
'Pick one...',
69+
'placeholder must return when state is cleared',
70+
)
71+
72+
comp.dispose()
73+
} finally {
74+
restoreDom()
75+
}
76+
})
77+
78+
it('prop change updates text using current state value', async () => {
79+
const restoreDom = installDom()
80+
try {
81+
const seed = `text-state-prop-propchange-${Date.now()}`
82+
const [{ default: Component }, { Store }] = await loadRuntimeModules(seed)
83+
const store = new Store({ placeholder: 'First...' })
84+
85+
const SelectLike = await compileJsxComponent(SOURCE, '/virtual/SelectLike.jsx', 'SelectLike', { Component })
86+
87+
const Parent = await compileJsxComponent(
88+
`
89+
import { Component } from '@geajs/core'
90+
import store from './store'
91+
import SelectLike from './SelectLike'
92+
export default class Parent extends Component {
93+
template() {
94+
return <SelectLike placeholder={store.placeholder} />
95+
}
96+
}
97+
`,
98+
'/virtual/Parent.jsx',
99+
'Parent',
100+
{ Component, store, SelectLike },
101+
)
102+
103+
const root = document.createElement('div')
104+
document.body.appendChild(root)
105+
106+
const parent = new Parent()
107+
parent.render(root)
108+
await flushMicrotasks()
109+
110+
assert.equal(parent.el.querySelector('.display')?.textContent?.trim(), 'First...')
111+
112+
store.placeholder = 'Second...'
113+
await flushMicrotasks()
114+
assert.equal(
115+
parent.el.querySelector('.display')?.textContent?.trim(),
116+
'Second...',
117+
'prop change must update text when state is empty',
118+
)
119+
120+
parent.dispose()
121+
} finally {
122+
restoreDom()
123+
}
124+
})
125+
})

tests/e2e/showcase.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ test.describe('showcase component gallery', () => {
7474
await expect(page.locator('[data-scope="select"]').first()).toBeVisible()
7575
})
7676

77+
test('select component uses placeholder passed in props', async ({ page }) => {
78+
const select = page.locator('[data-scope="select"]').first()
79+
await expect(select.locator('[data-part="value-text"]')).toHaveText('Pick one...')
80+
})
81+
7782
test('switch toggles are visible', async ({ page }) => {
7883
const switches = page.locator('[data-scope="switch"]')
7984
const count = await switches.count()

0 commit comments

Comments
 (0)