Fix error in test

Also misc changes

src/graph.luau

@@ -42,12 +42,14 @@ end
 
 local function get_owning_scope(): Node<unknown>
     local scope = get_scope()
+
     if not scope then
         local caller_name = debug.info(2, "n")
         return throw(`cannot use {caller_name}() in non-reactive scope, must be used within a root() or mount() callback`)
     elseif scope.effect then
-        throw("reactive scope is not an owning scope; new effects cannot be created in side-effects")
+        throw("cannot create new reactive scope inside of a tracking scope") -- todo: allow this?
     end
+
     return scope
 end
 
@@ -96,8 +98,8 @@ local function run_cleanups<T>(node: Node<T>)
 end
 
 local function find_and_swap_pop<T>(t: { T }, v: T)
-    local idx = table.find(t, v)
-    assert(idx, "value not found")
+    local idx = table.find(t, v) :: number
+    --assert(idx, "value not found")
     local n = #t
     t[idx] = t[n]
     t[n] = nil
@@ -107,10 +109,9 @@ local function remove_child<T>(parent: StartNode<T>, child: Node<T>)
     find_and_swap_pop(parent, child)
 end
 
-local function remove_owner<T>(node: Node<T>)
-    local owner = node.owner :: Node<T>
-    if node.owner and owner.owned then
-        find_and_swap_pop(owner.owned, node)
+local function disown<T>(node: Node<T>)
+    if node.owner then
+        find_and_swap_pop(node.owner.owned :: { Node<T> }, node)
     end
 end
 
@@ -126,7 +127,7 @@ end
 local function destroy<T>(node: Node<T>)
     run_cleanups(node)
     unparent(node)
-    remove_owner(node)
+    disown(node)
 
     if node.owned then
         local owned = node.owned
@@ -137,7 +138,8 @@ end
 
 local function destroy_owned<T>(node: Node<T>)
     if node.owned then
-        while node.owned[1] do destroy(node.owned[1]) end
+        local owned = node.owned
+        while owned[1] do destroy(owned[1]) end
     end
 end
 
@@ -178,18 +180,15 @@ local function evaluate_node<T>(node: Node<T>)
 
     node.cache = new_value
 
-    return cur_value ~= new_value -- node has changed value
+    return cur_value ~= new_value
 end
 
 local function queue_children<T>(node: StartNode<T>)
     local i = update_queue.n
-    local child = node[1]
-    while child do
-        --assert(child.parents.owner)
-        unparent(child)
+    while node[1] do
         i += 1
-        update_queue[i] = child
-        child = node[1]
+        update_queue[i] = node[1]
+        unparent(node[1])
     end
     update_queue.n = i
 end
@@ -201,7 +200,7 @@ local function update<T>(root: StartNode<T>)
     local i = n0 + 1
     while i <= update_queue.n do
         local node = update_queue[i]
-        assert(node.effect)
+        --assert(node.effect)
 
         if evaluate_node(node) then
             queue_children(node)

src/spring.luau

@@ -161,7 +161,8 @@ local function spring<T>(source: () -> T, period: number?, damping_ratio: number
     local c_c = 2*w_n
     local c = z * c_c
 
-    -- todo: is there a solution to this other than upping step frequency?
+    -- todo: is there a solution other than reducing step size?
+    -- todo: this does not catch all solver exploding cases
     if c > UPDATE_RATE*2 then -- solver will explode if this is true
         throw("spring damping too high, consider reducing damping or increasing period")
     end

test/tests.luau

@@ -1,5 +1,5 @@
 local testkit = require("test/testkit")
-local TEST, CASE, CHECK, FINISH, SKIP = testkit.test()
+local TEST, CASE, CHECK, FINISH = testkit.test()
 
 local mock = require "test/mock"
 local Instance, Signal = mock.Instance, mock.Signal
@@ -33,8 +33,6 @@ local NIL = nil :: any
 
 vide.strict = false
 
---SKIP "graph edge cases"
-
 TEST("graph", function()
     local create_node = graph.create_node
     local track = graph.track
@@ -1918,37 +1916,39 @@ TEST("graph edge cases", wrap_root(function()
     do CASE "diamond A,B,C,D,E"
         --[[
 
-        a > b >   > e
+        a > b   >   e
           > c > d >
 
         ]]
 
         local a = source(0)
 
-        local b = derive(function() print "ran b"; return (a() % 2 == 0) and 1 or 0 end)
-        local c = derive(function() print "ran c"; return a() * 2 end)
-        local d = derive(function() print "ran d"; return c() * 2 end)
-        local e = derive(function() print "ran e"; return b() + c() end)
+        local b = derive(function() return (a() % 2 == 0) and 1 or 0 end)
+        local c = derive(function() return a() * 2 end)
+        local d = derive(function() return c() * 2 end)
+        local e = derive(function() return b() + d() end)
 
         local count = { b = 0, c = 0, d = 0, e = 0 }
         effect(function() b(); count.b += 1 end)
         effect(function() c(); count.c += 1 end)
         effect(function() d(); count.d += 1 end)
-        effect(function() d(); count.e += 1 end)
+        effect(function() e(); count.e += 1 end)
+
+        CHECK(e() == 1)
 
         a(1)
 
         CHECK(count.b == 2)
         CHECK(count.c == 2)
         CHECK(count.d == 2)
-        CHECK(count.e == 2)
-        CHECK(e() == 4) -- todo: solve e evaluating before d
+        CHECK(count.e == 3) -- todo: redundant re-eval
+        CHECK(e() == 4)
 
         a(3)
         CHECK(count.b == 2)
         CHECK(count.c == 3)
         CHECK(count.d == 3)
-        CHECK(count.e == 3)
+        CHECK(count.e == 4)
         CHECK(e() == 12)
     end
 

todo.md

@@ -1,12 +1,10 @@
 # todo
 
-- property binding optimization
-  - would no longer allow `cleanup()` usage in binding scopes
-- solution to nested reactivity, see: SolidJS stores
 - optimize wide graph updating
 - implement from solid:
-  - Portal
-  - batch
+  - stores
+  - portals
+  - batch()
 - optimize `indexes()` double-diffing
-- improve crash course, some sections feel like information dumps
 - cleanup source and tests
+- prevent redundant re-eval of nodes in a complex diamond graph