Fix reactive conditional rendering with proper branch swapping
Implements efficient reactive conditionals that pre-render branches once and swap DOM nodes without recreation, enabling fast switching with thousands of elements. Key Fixes: - Runtime now calls reevaluate_conditional_*() on signal changes - Match/case swap logic uses phi nodes to load correct old case - Previously hardcoded case_0 as old value, causing accumulation - Branch render functions now wrap all content in container div Implementation: - Pre-render both if/else branches at init - Store roots in globals (@cond_N_then_root, @cond_N_else_root) - On signal change, swap DOM nodes via js_replace_at_marker() - Match statements use switch/phi pattern to select old case Tests: - Added test_reactive_conditionals.py (5 new tests) - Updated test_llvm_codegen_conditional.py for reactive infrastructure - All 35 tests passing Performance: No re-rendering on switches, perfect for dynamic content. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
ac7726df46
commit
e686903170
4 files changed
+272
-41
No files matched your search
@@ -115,6 +115,22 @@ class RitzRuntime {
|
|||||||
this.updateSignalDisplay(signalName, value);
|
this.updateSignalDisplay(signalName, value);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Re-evaluate all reactive conditionals
|
||||||
|
for (const [name, fn] of Object.entries(exports)) {
|
||||||
|
if (name.startsWith('reevaluate_conditional_') && typeof fn === 'function') {
|
||||||
|
fn();
|
||||||
|
this._log(`[Reactive] ${name}()`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Re-evaluate all reactive match statements
|
||||||
|
for (const [name, fn] of Object.entries(exports)) {
|
||||||
|
if (name.startsWith('reevaluate_match_') && typeof fn === 'function') {
|
||||||
|
fn();
|
||||||
|
this._log(`[Reactive] ${name}()`);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
updateSignalDisplay(signalName, value) {
|
updateSignalDisplay(signalName, value) {
|
||||||
@@ -154,22 +170,33 @@ class RitzRuntime {
|
|||||||
// Replace DOM node at marker position
|
// Replace DOM node at marker position
|
||||||
const marker = this.nodes.get(markerId);
|
const marker = this.nodes.get(markerId);
|
||||||
if (!marker) {
|
if (!marker) {
|
||||||
console.error(`Marker ${markerId} not found`);
|
console.error(`[replace_at_marker] Marker ${markerId} not found`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const parent = marker.parentNode;
|
const parent = marker.parentNode;
|
||||||
if (!parent) {
|
if (!parent) {
|
||||||
console.error(`Marker has no parent`);
|
console.error(`[replace_at_marker] Marker ${markerId} has no parent`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove old node if it exists (0 means no old node)
|
this._log(`[replace_at_marker] marker=${markerId}, old=${oldNodeId}, new=${newNodeId}`);
|
||||||
|
|
||||||
|
// Remove old node if it exists (0 means no old node to remove)
|
||||||
if (oldNodeId !== 0) {
|
if (oldNodeId !== 0) {
|
||||||
const oldNode = this.nodes.get(oldNodeId);
|
const oldNode = this.nodes.get(oldNodeId);
|
||||||
if (oldNode && oldNode.parentNode) {
|
if (!oldNode) {
|
||||||
|
console.warn(`[replace_at_marker] Old node ${oldNodeId} not found in registry`);
|
||||||
|
} else if (!oldNode.parentNode) {
|
||||||
|
console.warn(`[replace_at_marker] Old node ${oldNodeId} not in DOM`);
|
||||||
|
} else if (oldNode.parentNode !== parent) {
|
||||||
|
console.warn(`[replace_at_marker] Old node ${oldNodeId} in wrong parent`);
|
||||||
oldNode.parentNode.removeChild(oldNode);
|
oldNode.parentNode.removeChild(oldNode);
|
||||||
this._log(`Removed old node ${oldNodeId}`);
|
this._log(`[replace_at_marker] Removed ${oldNodeId} from wrong parent`);
|
||||||
|
} else {
|
||||||
|
// Remove from correct parent
|
||||||
|
parent.removeChild(oldNode);
|
||||||
|
this._log(`[replace_at_marker] Removed ${oldNodeId} from correct parent`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -177,11 +204,20 @@ class RitzRuntime {
|
|||||||
if (newNodeId !== 0) {
|
if (newNodeId !== 0) {
|
||||||
const newNode = this.nodes.get(newNodeId);
|
const newNode = this.nodes.get(newNodeId);
|
||||||
if (!newNode) {
|
if (!newNode) {
|
||||||
console.error(`New node ${newNodeId} not found`);
|
console.error(`[replace_at_marker] New node ${newNodeId} not found`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
parent.insertBefore(newNode, marker.nextSibling);
|
|
||||||
this._log(`Inserted node ${newNodeId} at marker ${markerId}`);
|
// Insert right after the marker
|
||||||
|
// If newNode is already in DOM elsewhere, this will move it
|
||||||
|
const nextSibling = marker.nextSibling;
|
||||||
|
if (nextSibling) {
|
||||||
|
parent.insertBefore(newNode, nextSibling);
|
||||||
|
this._log(`[replace_at_marker] Inserted ${newNodeId} before sibling`);
|
||||||
|
} else {
|
||||||
|
parent.appendChild(newNode);
|
||||||
|
this._log(`[replace_at_marker] Appended ${newNodeId} (no sibling)`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -522,21 +522,41 @@ class SignalLLVMIRGenerator:
|
|||||||
|
|
||||||
# Swap to this case
|
# Swap to this case
|
||||||
self.lines.append(f"swap_{case_idx}:")
|
self.lines.append(f"swap_{case_idx}:")
|
||||||
old_var = self._new_var()
|
|
||||||
new_var = self._new_var()
|
|
||||||
marker_var2 = self._new_var()
|
|
||||||
old_case_var = self._new_var()
|
|
||||||
|
|
||||||
# Load the old case root based on current state
|
# Load marker and new case root
|
||||||
# We need to use a phi or switch to load the right old root
|
marker_var2 = self._new_var()
|
||||||
# For simplicity, just create a minimal subtree swap
|
new_var = self._new_var()
|
||||||
self.lines.append(f" %{marker_var2} = load i32, i32* @cond_{cond_id}_marker")
|
self.lines.append(f" %{marker_var2} = load i32, i32* @cond_{cond_id}_marker")
|
||||||
self.lines.append(f" %{new_var} = load i32, i32* @cond_{cond_id}_case_{case_idx}_root")
|
self.lines.append(f" %{new_var} = load i32, i32* @cond_{cond_id}_case_{case_idx}_root")
|
||||||
|
|
||||||
# Approximate: load old root based on current value
|
# Use switch to load the correct old case root based on current value
|
||||||
# This is simplified - ideally we'd use a switch or phi
|
old_var = self._new_var()
|
||||||
self.lines.append(f" %{old_var} = load i32, i32* @cond_{cond_id}_case_0_root")
|
switch_labels = [f"load_old_{case_idx}_{i}" for i in range(num_cases)]
|
||||||
|
|
||||||
|
if default_idx is not None:
|
||||||
|
self.lines.append(f" switch i32 %{current_var}, label %{switch_labels[default_idx]} [")
|
||||||
|
else:
|
||||||
|
self.lines.append(f" switch i32 %{current_var}, label %{switch_labels[0]} [")
|
||||||
|
|
||||||
|
for i in range(num_cases):
|
||||||
|
self.lines.append(f" i32 {i}, label %{switch_labels[i]}")
|
||||||
|
self.lines.append(" ]")
|
||||||
|
|
||||||
|
# Emit load blocks and phi merge
|
||||||
|
phi_inputs = []
|
||||||
|
for i in range(num_cases):
|
||||||
|
self.lines.append(f"{switch_labels[i]}:")
|
||||||
|
tmp_var = self._new_var()
|
||||||
|
self.lines.append(f" %{tmp_var} = load i32, i32* @cond_{cond_id}_case_{i}_root")
|
||||||
|
phi_inputs.append((tmp_var, switch_labels[i]))
|
||||||
|
self.lines.append(f" br label %merge_{case_idx}")
|
||||||
|
|
||||||
|
# Merge point with phi to get old_var
|
||||||
|
self.lines.append(f"merge_{case_idx}:")
|
||||||
|
phi_operands = ", ".join([f"[ %{var}, %{label} ]" for var, label in phi_inputs])
|
||||||
|
self.lines.append(f" %{old_var} = phi i32 {phi_operands}")
|
||||||
|
|
||||||
|
# Now do the actual replacement
|
||||||
self.lines.append(f" call void @js_replace_at_marker(i32 %{marker_var2}, i32 %{old_var}, i32 %{new_var})")
|
self.lines.append(f" call void @js_replace_at_marker(i32 %{marker_var2}, i32 %{old_var}, i32 %{new_var})")
|
||||||
self.lines.append(f" store i32 {case_idx}, i32* @cond_{cond_id}_current")
|
self.lines.append(f" store i32 {case_idx}, i32* @cond_{cond_id}_current")
|
||||||
self.lines.append(" br label %done")
|
self.lines.append(" br label %done")
|
||||||
@@ -551,27 +571,30 @@ class SignalLLVMIRGenerator:
|
|||||||
self._emit_branch_render_function(cond_id, f"case_{case_idx}", case.get("body", []))
|
self._emit_branch_render_function(cond_id, f"case_{case_idx}", case.get("body", []))
|
||||||
|
|
||||||
def _emit_branch_render_function(self, cond_id: int, branch_name: str, body: list[dict[str, Any]]) -> None:
|
def _emit_branch_render_function(self, cond_id: int, branch_name: str, body: list[dict[str, Any]]) -> None:
|
||||||
"""Emit a function that renders a branch and returns its root element."""
|
"""Emit a function that renders a branch and returns its root element.
|
||||||
|
|
||||||
|
Wraps all branch content in a container div to handle multiple elements.
|
||||||
|
"""
|
||||||
self.lines.append(f"define i32 @_render_cond_{cond_id}_{branch_name}() {{")
|
self.lines.append(f"define i32 @_render_cond_{cond_id}_{branch_name}() {{")
|
||||||
self.lines.append("entry:")
|
self.lines.append("entry:")
|
||||||
|
|
||||||
# Render branch body
|
# Create wrapper div for the branch content
|
||||||
root_var = None
|
wrapper_var = self._new_var()
|
||||||
|
div_str = self._get_or_create_string("div")
|
||||||
|
self.lines.append(f" %{wrapper_var} = call i32 @js_create_element(i8* getelementptr inbounds ({self._string_type('div')}, {self._string_type('div')}* {div_str}, i32 0, i32 0))")
|
||||||
|
|
||||||
|
# Push wrapper onto parent stack so child elements append to it
|
||||||
|
self.parent_element_stack.append(wrapper_var)
|
||||||
|
|
||||||
|
# Render all statements in the branch body
|
||||||
for stmt in body:
|
for stmt in body:
|
||||||
var = self._emit_statement(stmt)
|
self._emit_statement(stmt)
|
||||||
if root_var is None and var:
|
|
||||||
root_var = var
|
|
||||||
|
|
||||||
# Return root element
|
# Pop wrapper from parent stack
|
||||||
if root_var:
|
self.parent_element_stack.pop()
|
||||||
self.lines.append(f" ret i32 %{root_var}")
|
|
||||||
else:
|
|
||||||
# Empty branch - create empty div
|
|
||||||
empty_var = self._new_var()
|
|
||||||
div_str = self._get_or_create_string("div")
|
|
||||||
self.lines.append(f" %{empty_var} = call i32 @js_create_element(i8* getelementptr inbounds ({self._string_type('div')}, {self._string_type('div')}* {div_str}, i32 0, i32 0))")
|
|
||||||
self.lines.append(f" ret i32 %{empty_var}")
|
|
||||||
|
|
||||||
|
# Return the wrapper element
|
||||||
|
self.lines.append(f" ret i32 %{wrapper_var}")
|
||||||
self.lines.append("}")
|
self.lines.append("}")
|
||||||
self.lines.append("")
|
self.lines.append("")
|
||||||
|
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ from ritz.llvm_codegen_signals import generate_llvm_ir
|
|||||||
|
|
||||||
|
|
||||||
def test_if_statement_codegen():
|
def test_if_statement_codegen():
|
||||||
"""Test LLVM IR generation for if statement."""
|
"""Test LLVM IR generation for if statement with reactive rendering."""
|
||||||
source = """
|
source = """
|
||||||
from ritz.syntax import *
|
from ritz.syntax import *
|
||||||
|
|
||||||
@@ -27,13 +27,15 @@ def Toggle():
|
|||||||
# Should have conditional branch
|
# Should have conditional branch
|
||||||
assert "br i1" in llvm_ir
|
assert "br i1" in llvm_ir
|
||||||
|
|
||||||
# Should have basic blocks for then/else/merge
|
# Should have reactive conditional infrastructure
|
||||||
assert "if.then." in llvm_ir
|
assert "@cond_0_marker" in llvm_ir
|
||||||
assert "if.end." in llvm_ir
|
assert "_init_conditional_0" in llvm_ir
|
||||||
|
assert "_render_cond_0_then" in llvm_ir
|
||||||
|
assert "_render_cond_0_else" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
def test_if_else_codegen():
|
def test_if_else_codegen():
|
||||||
"""Test LLVM IR generation for if/else."""
|
"""Test LLVM IR generation for if/else with reactive rendering."""
|
||||||
source = """
|
source = """
|
||||||
from ritz.syntax import *
|
from ritz.syntax import *
|
||||||
|
|
||||||
@@ -54,9 +56,10 @@ def Toggle():
|
|||||||
# Should have conditional branch
|
# Should have conditional branch
|
||||||
assert "br i1" in llvm_ir
|
assert "br i1" in llvm_ir
|
||||||
|
|
||||||
# Should have both then and else blocks
|
# Should have reactive infrastructure with both branches
|
||||||
assert "if.then." in llvm_ir
|
assert "_render_cond_0_then" in llvm_ir
|
||||||
assert "if.else." in llvm_ir
|
assert "_render_cond_0_else" in llvm_ir
|
||||||
|
assert "reevaluate_conditional_0" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
def test_if_comparison_codegen():
|
def test_if_comparison_codegen():
|
||||||
|
|||||||
@@ -0,0 +1,169 @@
|
|||||||
|
"""Tests for reactive conditional rendering with signal updates."""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from ritz.parser import parse_component
|
||||||
|
from ritz.llvm_codegen_signals import generate_llvm_ir
|
||||||
|
|
||||||
|
|
||||||
|
def test_reactive_conditional_infrastructure():
|
||||||
|
"""Test that reactive conditional infrastructure is generated."""
|
||||||
|
source = """
|
||||||
|
from ritz.syntax import *
|
||||||
|
|
||||||
|
@component
|
||||||
|
def Toggle():
|
||||||
|
visible = signal(1)
|
||||||
|
|
||||||
|
if visible:
|
||||||
|
with P():
|
||||||
|
yield "Visible"
|
||||||
|
else:
|
||||||
|
with P():
|
||||||
|
yield "Hidden"
|
||||||
|
"""
|
||||||
|
ir = parse_component(source)
|
||||||
|
llvm_ir = generate_llvm_ir(ir)
|
||||||
|
|
||||||
|
# Should have marker global
|
||||||
|
assert "@cond_0_marker = global i32 0" in llvm_ir
|
||||||
|
|
||||||
|
# Should have current state global
|
||||||
|
assert "@cond_0_current = global i32 0" in llvm_ir
|
||||||
|
|
||||||
|
# Should have root globals for both branches
|
||||||
|
assert "@cond_0_then_root = global i32 0" in llvm_ir
|
||||||
|
assert "@cond_0_else_root = global i32 0" in llvm_ir
|
||||||
|
|
||||||
|
# Should have init function
|
||||||
|
assert "define void @_init_conditional_0()" in llvm_ir
|
||||||
|
|
||||||
|
# Should have reevaluate function (for signal updates)
|
||||||
|
assert "define void @reevaluate_conditional_0()" in llvm_ir
|
||||||
|
|
||||||
|
# Should have branch render functions
|
||||||
|
assert "define i32 @_render_cond_0_then()" in llvm_ir
|
||||||
|
assert "define i32 @_render_cond_0_else()" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
|
def test_reactive_conditional_init_logic():
|
||||||
|
"""Test that init function pre-renders both branches."""
|
||||||
|
source = """
|
||||||
|
from ritz.syntax import *
|
||||||
|
|
||||||
|
@component
|
||||||
|
def Toggle():
|
||||||
|
visible = signal(1)
|
||||||
|
|
||||||
|
if visible:
|
||||||
|
with P():
|
||||||
|
yield "Shown"
|
||||||
|
else:
|
||||||
|
with P():
|
||||||
|
yield "Hidden"
|
||||||
|
"""
|
||||||
|
ir = parse_component(source)
|
||||||
|
llvm_ir = generate_llvm_ir(ir)
|
||||||
|
|
||||||
|
# Init should call both render functions
|
||||||
|
assert "call i32 @_render_cond_0_then()" in llvm_ir
|
||||||
|
assert "call i32 @_render_cond_0_else()" in llvm_ir
|
||||||
|
|
||||||
|
# Init should store both roots
|
||||||
|
assert "store i32" in llvm_ir and "@cond_0_then_root" in llvm_ir
|
||||||
|
assert "store i32" in llvm_ir and "@cond_0_else_root" in llvm_ir
|
||||||
|
|
||||||
|
# Init should insert active branch at marker
|
||||||
|
assert "call void @js_replace_at_marker" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
|
def test_reactive_conditional_swap_logic():
|
||||||
|
"""Test that reevaluate function swaps branches when condition changes."""
|
||||||
|
source = """
|
||||||
|
from ritz.syntax import *
|
||||||
|
|
||||||
|
@component
|
||||||
|
def Toggle():
|
||||||
|
visible = signal(0)
|
||||||
|
|
||||||
|
if visible:
|
||||||
|
with P():
|
||||||
|
yield "On"
|
||||||
|
else:
|
||||||
|
with P():
|
||||||
|
yield "Off"
|
||||||
|
"""
|
||||||
|
ir = parse_component(source)
|
||||||
|
llvm_ir = generate_llvm_ir(ir)
|
||||||
|
|
||||||
|
# Reevaluate should re-evaluate the condition
|
||||||
|
assert "reevaluate_conditional_0" in llvm_ir
|
||||||
|
|
||||||
|
# Should check if condition changed
|
||||||
|
assert "icmp ne" in llvm_ir
|
||||||
|
|
||||||
|
# Should have swap logic
|
||||||
|
assert "swap:" in llvm_ir or "swap_to_then:" in llvm_ir
|
||||||
|
assert "swap_to_then:" in llvm_ir
|
||||||
|
assert "swap_to_else:" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
|
def test_multiple_reactive_conditionals():
|
||||||
|
"""Test multiple if statements in same component get unique IDs."""
|
||||||
|
source = """
|
||||||
|
from ritz.syntax import *
|
||||||
|
|
||||||
|
@component
|
||||||
|
def MultiToggle():
|
||||||
|
flag1 = signal(1)
|
||||||
|
flag2 = signal(0)
|
||||||
|
|
||||||
|
if flag1:
|
||||||
|
with P():
|
||||||
|
yield "First on"
|
||||||
|
|
||||||
|
if flag2:
|
||||||
|
with P():
|
||||||
|
yield "Second on"
|
||||||
|
"""
|
||||||
|
ir = parse_component(source)
|
||||||
|
llvm_ir = generate_llvm_ir(ir)
|
||||||
|
|
||||||
|
# Should have two separate conditionals
|
||||||
|
assert "@cond_0_marker" in llvm_ir
|
||||||
|
assert "@cond_1_marker" in llvm_ir
|
||||||
|
|
||||||
|
# Should have init functions for both
|
||||||
|
assert "define void @_init_conditional_0()" in llvm_ir
|
||||||
|
assert "define void @_init_conditional_1()" in llvm_ir
|
||||||
|
|
||||||
|
# Should have reevaluate functions for both
|
||||||
|
assert "define void @reevaluate_conditional_0()" in llvm_ir
|
||||||
|
assert "define void @reevaluate_conditional_1()" in llvm_ir
|
||||||
|
|
||||||
|
|
||||||
|
def test_reactive_conditional_with_comparison():
|
||||||
|
"""Test reactive conditional with comparison operator."""
|
||||||
|
source = """
|
||||||
|
from ritz.syntax import *
|
||||||
|
|
||||||
|
@component
|
||||||
|
def Counter():
|
||||||
|
count = signal(0)
|
||||||
|
|
||||||
|
if count > 5:
|
||||||
|
with P():
|
||||||
|
yield "High"
|
||||||
|
else:
|
||||||
|
with P():
|
||||||
|
yield "Low"
|
||||||
|
"""
|
||||||
|
ir = parse_component(source)
|
||||||
|
llvm_ir = generate_llvm_ir(ir)
|
||||||
|
|
||||||
|
# Should generate icmp for comparison
|
||||||
|
assert "icmp" in llvm_ir
|
||||||
|
assert "sgt" in llvm_ir # signed greater than
|
||||||
|
|
||||||
|
# Should still have reactive infrastructure
|
||||||
|
assert "@_init_conditional_0()" in llvm_ir
|
||||||
|
assert "@reevaluate_conditional_0()" in llvm_ir
|
||||||
Reference in new issue
Block a user