Bug: remove() does not reliably emit tree change signals
Summary
remove() halts the spawned task that owns a node, which triggers cleanup in the task's finally block (removing from parent._children, deleting from tree.nodes, calling tree.markDirty()). However, task.halt() is asynchronous relative to the useTree eval loop's dirty-check. The finally block may run after the eval loop has already checked state.dirty and found it false, causing output.send() to never fire. Any subscriber listening via each(tree) will not be notified of the removal.
How remove() works today
append() spawns a task per child (freedom.ts)
*append(name, component): Operation<Node> {
const parent = yield* NodeContext.expect();
const tree = yield* TreeContext.expect();
const child = new NodeImpl(tree.nextId(), name, parent);
const ready = withResolvers<void>();
const task = yield* spawn(function* () {
parent._children.add(child);
tree.nodes.set(child.id, child);
yield* NodeContext.set(child);
yield* spawnEvalLoop(child._channel);
ready.resolve();
try {
yield* component();
yield* suspend();
} finally {
parent._children.delete(child); // cleanup on halt
tree.nodes.delete(child.id); // remove from global map
tree.markDirty(); // mark tree dirty
}
});
child.data.set(Halt, task.halt);
child.remove = () => FreedomApi.operations.remove(child);
yield* ready.operation;
return child;
}
remove() halts the task (freedom.ts)
*remove(node: Node): Operation<void> {
const halt = node.data.expect(Halt);
yield* halt();
}
The useTree notification pipeline (tree.ts)
const sub = yield* events;
yield* spawn(function* () {
while (true) {
const next = yield* sub.next();
state.dirty = false;
yield* rootNode.eval(() => DispatchApi.operations.dispatch(next.value));
if (state.dirty) {
output.send(); // ← subscribers see this
}
}
});
useFocus around-hook intercepts remove (focus.ts)
yield* FreedomApi.around({
*remove([node], next) {
if (node.props.focused === true) {
yield* FocusApi.operations.advance(); // advance focus FIRST
}
yield* next(node); // then halt the node
},
});
The race
The sequence of events when a dispatch triggers a remove():
events.send(dispatchEvent) — the eval loop picks it up.
state.dirty = false — reset at top of loop.
rootNode.eval(() => dispatch(...)) — runs the dispatch handler, which calls remove().
remove() calls task.halt() — schedules the task's finally block (asynchronous).
remove() returns — yield* halt() resolves.
- The dispatch handler returns.
rootNode.eval() resolves.
- The eval loop checks
if (state.dirty) — may be false because the finally block hasn't run yet.
- The
finally block runs (in a future microtask): parent._children.delete(child), tree.nodes.delete(child.id), tree.markDirty().
Result: markDirty() fires after the dirty-check. output.send() never fires. Subscribers are not notified of the removal.
The useFocus around-hook makes this worse: focus.advance() runs before next(node) (before halt()), mutating focused props on sibling nodes within the same dispatch. These mutations go through other nodes' eval channels, which can interleave with the dirty-check timing.
Expected behavior
After remove(), the tree should emit exactly one change notification so that any subscriber iterating via each(tree) is reliably informed that the tree structure has changed.
Specifically:
parent._children no longer contains the removed node.
tree.nodes no longer contains the removed node's id.
- A subscriber awaiting
each(tree) receives a notification.
Actual behavior
The notification is intermittently lost. The finally block runs asynchronously after the eval loop's dirty-check, so markDirty() has no effect on the current iteration. No output.send() fires. The subscriber never wakes up.
The tree's internal state is eventually consistent (the finally block does clean up), but the signal is lost. Subscribers see stale state.
Suggested fix
Option A: Synchronous remove — clean up before halting
Make remove() immediately clean up _children and tree.nodes before halting the task. This guarantees the dirty flag is set before the eval loop checks it:
*remove(node: Node) {
const impl = node as NodeImpl;
const parent = impl._parent;
if (!parent) throw new Error("Cannot remove root node");
// Synchronously clean up — dirty flag is guaranteed set before eval loop checks it
parent._children.delete(impl);
tree.nodes.delete(impl.id);
tree.markDirty();
// Then halt the task (finally block must become idempotent)
const halt = impl.data.expect(Halt);
yield* halt();
}
The finally block in append() must become idempotent:
finally {
if (parent._children.has(child)) {
parent._children.delete(child);
}
if (tree.nodes.has(child.id)) {
tree.nodes.delete(child.id);
tree.markDirty();
}
}
Option B: Drain pending halts in the eval loop
Ensure the eval loop waits for all spawned-task finally blocks to settle before checking state.dirty:
// tree.ts
yield* rootNode.eval(() => DispatchApi.operations.dispatch(next.value));
// ← some kind of "drain" or "settle" yield here
if (state.dirty) {
output.send();
}
This would require a mechanism to await completion of all in-flight halt cleanups, which may need support from Effection itself (e.g. a Scope.settle() operation).
Option C: Declarative reconcile() operation
Add a reconcile() that atomically updates the children set with a single markDirty(), avoiding the async-halt problem entirely:
*reconcile(desiredChildren: { name: string; key: string }[]) {
// Remove children not in desiredChildren
// Add children that are new
// One atomic markDirty() at the end
}
Bug:
remove()does not reliably emit tree change signalsSummary
remove()halts the spawned task that owns a node, which triggers cleanup in the task'sfinallyblock (removing fromparent._children, deleting fromtree.nodes, callingtree.markDirty()). However,task.halt()is asynchronous relative to theuseTreeeval loop's dirty-check. Thefinallyblock may run after the eval loop has already checkedstate.dirtyand found itfalse, causingoutput.send()to never fire. Any subscriber listening viaeach(tree)will not be notified of the removal.How
remove()works todayappend()spawns a task per child (freedom.ts)remove()halts the task (freedom.ts)The
useTreenotification pipeline (tree.ts)useFocusaround-hook intercepts remove (focus.ts)The race
The sequence of events when a dispatch triggers a
remove():events.send(dispatchEvent)— the eval loop picks it up.state.dirty = false— reset at top of loop.rootNode.eval(() => dispatch(...))— runs the dispatch handler, which callsremove().remove()callstask.halt()— schedules the task'sfinallyblock (asynchronous).remove()returns —yield* halt()resolves.rootNode.eval()resolves.if (state.dirty)— may befalsebecause thefinallyblock hasn't run yet.finallyblock runs (in a future microtask):parent._children.delete(child),tree.nodes.delete(child.id),tree.markDirty().Result:
markDirty()fires after the dirty-check.output.send()never fires. Subscribers are not notified of the removal.The
useFocusaround-hook makes this worse:focus.advance()runs beforenext(node)(beforehalt()), mutatingfocusedprops on sibling nodes within the same dispatch. These mutations go through other nodes' eval channels, which can interleave with the dirty-check timing.Expected behavior
After
remove(), the tree should emit exactly one change notification so that any subscriber iterating viaeach(tree)is reliably informed that the tree structure has changed.Specifically:
parent._childrenno longer contains the removed node.tree.nodesno longer contains the removed node's id.each(tree)receives a notification.Actual behavior
The notification is intermittently lost. The
finallyblock runs asynchronously after the eval loop's dirty-check, somarkDirty()has no effect on the current iteration. Nooutput.send()fires. The subscriber never wakes up.The tree's internal state is eventually consistent (the
finallyblock does clean up), but the signal is lost. Subscribers see stale state.Suggested fix
Option A: Synchronous remove — clean up before halting
Make
remove()immediately clean up_childrenandtree.nodesbefore halting the task. This guarantees the dirty flag is set before the eval loop checks it:The
finallyblock inappend()must become idempotent:Option B: Drain pending halts in the eval loop
Ensure the eval loop waits for all spawned-task
finallyblocks to settle before checkingstate.dirty:This would require a mechanism to await completion of all in-flight halt cleanups, which may need support from Effection itself (e.g. a
Scope.settle()operation).Option C: Declarative
reconcile()operationAdd a
reconcile()that atomically updates the children set with a singlemarkDirty(), avoiding the async-halt problem entirely: