Skip to content

Commit 80c4fa7

Browse files
committed
feat: connecting/connected indicator
1 parent d961b5c commit 80c4fa7

File tree

7 files changed

+530
-44
lines changed

7 files changed

+530
-44
lines changed

cli/knowledge.md

Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,224 @@ The implementation uses:
132132
- `markdownToInline()`: Converts markdown AST to array of inline JSX elements
133133
- `renderInlineContent()`: Renders inline styling (`<strong>`, `<em>`, `<span>`)
134134
- Returns a fragment `<>{inlineElements}</>` that can be safely placed inside parent `<text>`
135+
136+
## React Reconciliation Issues
137+
138+
### The "Child not found in children at remove" Error
139+
140+
OpenTUI's React reconciler has **critical limitations** with certain conditional rendering patterns that can cause the error:
141+
142+
```
143+
Error: Child not found in children
144+
at remove (/path/to/TextNode.ts:152:17)
145+
at removeChild (/path/to/host-config.ts:60:12)
146+
```
147+
148+
### Root Cause
149+
150+
OpenTUI's reconciler struggles when:
151+
1. **Conditionally rendering elements at the same level** using `{condition && <element>}`
152+
2. **The parent `<text>` element switches between different child structures**
153+
3. Components that dynamically create/remove `<span>` elements (like ShimmerText)
154+
4. **Conditionally rendering text nodes** (including spaces like `{showText ? ' ' : ''}`)
155+
156+
This happens because OpenTUI's reconciler doesn't handle React's reconciliation algorithm as smoothly as standard React DOM.
157+
158+
### The Text Node Problem
159+
160+
**CRITICAL INSIGHT**: The issue isn't just about conditionally rendering elements - it also affects **TEXT NODES**. Even something as simple as a conditional space can trigger the error:
161+
162+
```tsx
163+
// ❌ PROBLEMATIC: Conditionally adding/removing text nodes (including spaces)
164+
<span>■{showText ? ' ' : ''}</span>
165+
166+
// ✅ WORKING: Put the conditional text inside the span content itself
167+
<span>{showText ? '' : ''}</span>
168+
```
169+
170+
In React, spaces and other text are represented as text nodes in the virtual DOM. When you write `{showText ? ' ' : ''}`, you're conditionally adding/removing a text node child, which causes OpenTUI's reconciler to fail when trying to match up children.
171+
172+
**Key takeaway**: Always include text content (including spaces) as part of the string literal, not as separate conditional expressions.
173+
174+
### ❌ PROBLEMATIC PATTERNS
175+
176+
**Pattern 1: Shared parent with conditional children**
177+
```tsx
178+
// This causes reconciliation errors!
179+
<text wrap={false}>
180+
{isConnected ? (
181+
<>
182+
<span>■ </span>
183+
{showText && <span>connected</span>}
184+
</>
185+
) : (
186+
<ShimmerText text="connecting..." />
187+
)}
188+
</text>
189+
```
190+
191+
**Pattern 2: Conditionally rendering entire span elements**
192+
```tsx
193+
// Also problematic!
194+
<text wrap={false}>
195+
<span>■ </span>
196+
{showText && (
197+
<span>connected</span>
198+
)}
199+
</text>
200+
```
201+
202+
**Pattern 3: Conditionally rendering text nodes (spaces, strings, etc.)**
203+
```tsx
204+
// Triggers reconciliation errors!
205+
<span>■{showText ? ' ' : ''}</span>
206+
<span>{condition ? 'text' : ''}</span>
207+
```
208+
209+
### ✅ WORKING SOLUTION
210+
211+
**Keep each conditional state in its own stable `<text>` wrapper:**
212+
213+
```tsx
214+
// This works reliably!
215+
{isConnected ? (
216+
<text wrap={false}>
217+
<span>{showText ? '' : ''}</span>
218+
{showText && <span>connected</span>}
219+
</text>
220+
) : (
221+
<text wrap={false}>
222+
<ShimmerText text="connecting..." />
223+
</text>
224+
)}
225+
```
226+
227+
**Key principle:** Each major UI state (connected vs disconnected) should have its own `<text>` element. The `<text>` element itself should not change during state transitions within that UI state.
228+
229+
### Why This Works
230+
231+
- The `<text>` element for each state remains **stable**
232+
- Only the *children* inside each `<text>` change
233+
- React never tries to reconcile between the connected and disconnected `<text>` elements
234+
- The reconciler doesn't get confused trying to match up old and new children
235+
236+
### Best Practices
237+
238+
1. **Separate `<text>` elements for different UI states** - Don't try to share a single `<text>` element across major state changes
239+
2. **Keep element structure stable** - If you need conditional content, prefer changing text content over conditionally rendering elements
240+
3. **Avoid complex conditional rendering within OpenTUI components** - What works in React DOM may not work in OpenTUI
241+
4. **Test thoroughly** - Reconciliation errors often appear only during specific state transitions
242+
243+
### Alternative Approach: Stable Element Structure
244+
245+
If you must use a single `<text>` element, keep the child element structure completely stable:
246+
247+
```tsx
248+
// This also works - elements are always present
249+
<text wrap={false}>
250+
<span>{getIndicatorText()}</span>
251+
<span>{getStatusText()}</span>
252+
</text>
253+
```
254+
255+
But this approach is less flexible and harder to read than using separate `<text>` elements for each state.
256+
257+
### Best Practice: Direct Ternary Pattern
258+
259+
The cleanest solution is to use a direct ternary with separate `<text>` elements:
260+
261+
```tsx
262+
{isConnected ? (
263+
<text wrap={false}>
264+
<span>{showText ? '' : ''}</span>
265+
{showText && <span>connected</span>}
266+
</text>
267+
) : (
268+
<text wrap={false}>
269+
<ShimmerText text="connecting..." />
270+
</text>
271+
)}
272+
```
273+
274+
**Why this is the best approach:**
275+
- Clear and explicit about the two states
276+
- Minimal abstraction - easy to understand at a glance
277+
- Each state's `<text>` wrapper is clearly visible
278+
- No need for additional helper components
279+
280+
**Note:** Helper components like `ConditionalText` are not recommended as they add unnecessary abstraction without providing meaningful benefits. The direct ternary pattern is clearer and easier to maintain.
281+
282+
### The "Text Must Be Created Inside of a Text Node" Error
283+
284+
**Error message:**
285+
```
286+
Error: Text must be created inside of a text node
287+
at createTextInstance (/path/to/host-config.ts:108:17)
288+
```
289+
290+
**Root cause:** This error occurs when a component returns Fragment with `<span>` elements containing text, but the parent doesn't wrap it in a `<text>` element.
291+
292+
**What triggers it:**
293+
```tsx
294+
// Component returns Fragment with spans
295+
const ShimmerText = ({ text }) => {
296+
return (
297+
<>
298+
{text.split('').map(char => (
299+
<span>{char}</span> // Text nodes created here!
300+
))}
301+
</>
302+
)
303+
}
304+
305+
// ❌ INCORRECT: Using component without <text> wrapper
306+
<box>
307+
<ShimmerText text="hello" />
308+
</box>
309+
```
310+
311+
**The solution:** Parent components must wrap Fragment-returning components in `<text>` elements:
312+
313+
```tsx
314+
// ✅ CORRECT: Parent wraps in <text>
315+
<box>
316+
<text wrap={false}>
317+
<ShimmerText text="hello" />
318+
</text>
319+
</box>
320+
```
321+
322+
**Why components shouldn't self-wrap in `<text>`:**
323+
1. Creates composition issues - you can't combine multiple components in one `<text>` element
324+
2. Prevents flexibility in how the component is used
325+
3. Can cause reconciliation errors when the component updates
326+
4. Goes against React's composition principles
327+
328+
**Best practice:**
329+
- Child components that render styled text should return Fragments with `<span>` elements
330+
- Parent components are responsible for providing the `<text>` wrapper
331+
- This follows React's pattern of "dumb" presentational components
332+
333+
**Component design pattern:**
334+
```tsx
335+
// Child component - returns Fragment
336+
export const StyledText = ({ text, color }) => {
337+
return (
338+
<>
339+
<span fg={color}>{text}</span>
340+
</>
341+
)
342+
}
343+
344+
// Parent component - provides <text> wrapper
345+
const Parent = () => {
346+
return (
347+
<text wrap={false}>
348+
<StyledText text="hello" color="#ff0000" />
349+
<StyledText text="world" color="#00ff00" />
350+
</text>
351+
)
352+
}
353+
```
354+
355+
This pattern allows multiple styled components to be composed together within a single `<text>` element while avoiding the "Text must be created inside of a text node" error.

0 commit comments

Comments
 (0)