Disallow creation of nested tracking scopes

src/graph.luau

@@ -10,7 +10,7 @@ export type StartNode<T> = {
 
 export type Node<T> =  {
     cache: T,
-    effect:  ((T) -> T) | false,
+    effect:  ((T) -> T) | "owner" | "untracked",
     cleanups: { () -> () } | false,
     parents: { owner: StartNode<T>?, [number]: StartNode<T> },
     [number]: Node<T>
@@ -50,8 +50,8 @@ local function get_owning_scope(): Node<unknown>
     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
+    elseif scope.effect ~= "owner" then
-        throw("owning scope is not stable; are you trying to derive a new source from within a side-effect?")
+        throw("reactive scope is not an owning scope; new effects cannot be created in side-effects")
     end
     return scope
 end
@@ -117,8 +117,6 @@ local function destroy<T>(node: Node<T>)
     run_cleanups(node)
     unparent(node)
 
-    node.effect = false
-
     if node.parents.owner then
         remove_child(node.parents.owner, node)
         node.parents.owner = nil
@@ -168,24 +166,13 @@ local function update_from<T>(node: StartNode<T>, n0: number)
 
     -- unparent all children and queue for eval
     do
-        local i = 1
+        local child = node[1]
-        local child = node[i]
         while child do
+            --assert(child.parents.owner)
             unparent(child)
-
             n += 1
             update_queue[n] = child
-
+            child = node[1]
-            local next_child = node[i]
-
-            -- children who have this parent as an owner will not be unparented
-            -- if such a child is encountered then skip it
-            if next_child == child then
-                i += 1
-                next_child = node[i]
-            end
-
-            child = next_child
         end
     end
 
@@ -194,7 +181,7 @@ local function update_from<T>(node: StartNode<T>, n0: number)
     -- evaluate all queued children
     for i = n0 + 1, n do
         local child = update_queue[i]
-        if not child.effect then continue end
+        assert(type(child.effect) == "function")
 
         if evaluate_node(child) then
             update_from(child, n)
@@ -212,12 +199,12 @@ end
 
 local function track<T>(node: StartNode<T>)
     local scope = get_scope()
-    if scope and scope.effect then -- do not track nodes with no effect
+    if scope and type(scope.effect) == "function" then -- do not track nodes with no effect
         add_child(node, scope)
     end
 end
 
-local function create_node<T>(value: T, effect: false | (T) -> T): Node<T>
+local function create_node<T>(value: T, effect: "owner" | (T) -> T): Node<T>
     return {
         cache = value,
         effect = effect,

src/maps.luau

@@ -30,7 +30,7 @@ end
 local function indexes<K, VI, VO>(input: () -> Map<K, VI>, transform: (() -> VI, K) -> VO): () -> { VO }
     local owner = get_owning_scope()
 
-    local subowner = create_node(false, false)
+    local subowner = create_node(false, "owner")
     set_owner(subowner, owner)
 
     local input_cache = {} :: Map<K, VI>
@@ -67,7 +67,7 @@ local function indexes<K, VI, VO>(input: () -> Map<K, VI>, transform: (() -> VI,
 
             if cv ~= v then
                 if cv == nil then -- create new scope and run transform
-                    local scope = create_node(false, false)
+                    local scope = create_node(false, "owner")
                     scopes[i] = scope :: Node<any>
 
                     local node = create_start_node(v)
@@ -112,6 +112,7 @@ local function indexes<K, VI, VO>(input: () -> Map<K, VI>, transform: (() -> VI,
     local node = create_node(false :: any, function()
         return update_children(input())
     end)
+    set_owner(node, owner)
 
     evaluate_node(node)
 
@@ -124,7 +125,7 @@ end
 local function values<K, VI, VO>(input: () -> Map<K, VI>, transform: (VI, () -> K) -> VO): () -> { VO }
     local owner = get_owning_scope()
 
-    local subowner = create_node(false, false)
+    local subowner = create_node(false, "owner")
     set_owner(subowner, owner)
     
     local cur_input_cache_up = {} :: Map<VI, K>
@@ -155,7 +156,7 @@ local function values<K, VI, VO>(input: () -> Map<K, VI>, transform: (VI, () ->
             local cv = cur_input_cache[v]
             
             if cv == nil then -- create new scope and run transform
-                local scope = create_node(false, false)
+                local scope = create_node(false, "owner")
                 scopes[v] = scope :: Node<any>
 
                 local node = create_start_node(i)
@@ -214,6 +215,7 @@ local function values<K, VI, VO>(input: () -> Map<K, VI>, transform: (VI, () ->
     local node = create_node(false :: any, function()
         return update_children(input())
     end)
+    set_owner(node, owner)
 
     evaluate_node(node)
 

src/root.luau

@@ -11,7 +11,7 @@ local destroy = graph.destroy
 local refs = {}
 
 local function root<T...>(fn: (destroy: () -> ()) -> T...): T...
-    local node = create_node(false, false)
+    local node = create_node(false, "owner")
 
     refs[node] = true -- prevent gc of root node
 

src/switch.luau

@@ -38,7 +38,7 @@ local function switch<T, U>(source: () -> T): (map: Map<T, ((() -> U)?)>) -> ()
                 throw("map must map a value to a function")
             end
 
-            local new_scope = create_node(false, false)
+            local new_scope = create_node(false, "owner")
             last_scope = new_scope :: Node<any>
             
             set_owner(new_scope, owner)

src/untrack.luau

@@ -13,7 +13,7 @@ local function untrack<T>(source: () -> T): T
 
     -- sources are only tracked if the node in scope has an effect
     local effect = scope.effect
-    scope.effect = false
+    scope.effect = "untracked"
 
     local ok, result = pcall(source)
 

test/tests.luau

@@ -41,6 +41,7 @@ TEST("graph", function()
     local get_scope = graph.get_scope
     local open_scope = graph.open_scope
     local close_scope = graph.close_scope
+    local set_owner = graph.set_owner
     local get_children = graph.get_children
     local add_cleanup = graph.add_cleanup
     local destroy = graph.destroy
@@ -50,7 +51,7 @@ TEST("graph", function()
     end
 
     local function scope()
-        return create_node(false, false)
+        return create_node(false, "owner")
     end
 
     local function cleanup(fn: () -> ())
@@ -75,10 +76,14 @@ TEST("graph", function()
     end
 
     do CASE "rerun linked nodes"
+        local root = node()
         local a = node()
         local b = node()
         local c = node()
 
+        set_owner(b, root)
+        set_owner(c, root)
+
         local count = 0
 
         local function effect(x)
@@ -104,8 +109,15 @@ TEST("graph", function()
     end
 
     do CASE "diamond graph"
+        -- a -> b -> d
+        --   -> c
+        local root = node()
         local a, b, c, d = node(), node(), node(), node()
 
+        set_owner(b, root)
+        set_owner(c, root)
+        set_owner(d, root)
+
         local b_cnt, c_cnt, d_cnt = 0, 0, 0
         function b.effect(x) b_cnt += 1; return not x end
         function c.effect(x) c_cnt += 1; return not x end
@@ -122,16 +134,52 @@ TEST("graph", function()
         CHECK(d_cnt == 1)
     end
 
+    do CASE "diamond graph 2"
+        -- todo: include cached value from parent nodes to confirm update order
+        -- a -> b -> c -> e
+        --   -> d
+        local root = node()
+        local a, b, c, d, e = node(), node(), node(), node(), node()
+
+        set_owner(b, root)
+        set_owner(c, root)
+        set_owner(d, root)
+        set_owner(e, root)
+
+        local b_cnt, c_cnt, d_cnt, e_cnt = 0, 0, 0, 0
+        function b.effect(x) b_cnt += 1; return not x end
+        function c.effect(x) c_cnt += 1; return not x end
+        function d.effect(x) d_cnt += 1; return not x end
+        function e.effect(x) e_cnt += 1; return not x end
+
+        open_scope(b); track(a); close_scope()
+        open_scope(c); track(b); close_scope()
+        open_scope(d); track(a); close_scope()
+        open_scope(e); track(c); track(d); close_scope()
+
+        update(a)
+
+        CHECK(b_cnt == 1)
+        CHECK(c_cnt == 1)
+        CHECK(d_cnt == 1)
+        CHECK(e_cnt == 1)
+    end
+
     do CASE "duplicate child on rerun"
+        local root = node()
         local a, b, c = node(), node(), node()
 
+        set_owner(a, root)
+        set_owner(b, root)
+        set_owner(c, root)
+
         function c.effect(x)
             track(a)
             track(b)
             return not x
         end
 
-        open_scope(c); assert(c.effect)(NIL); close_scope()
+        open_scope(c); assert(type(c.effect) == "function" and c.effect)(NIL); close_scope()
 
         update(a)
 
@@ -168,28 +216,28 @@ TEST("graph", function()
             items_updated = node()
             track(items_updated) -- should not
 
-            add_child(root, items_updated)
+            set_owner(items_updated, root)
             do open_scope(items_updated)
                 track(items)
 
                 do open_scope(root)
-                    add_child(root, scope1)
+                    set_owner(scope1, root)
                     do open_scope(scope1)
                         clean "scope1"
                         bind1 = node()
 
-                        add_child(scope1, bind1)
+                        set_owner(bind1, scope1)
                         do open_scope(bind1)
                             clean "bind1"
                             track(selected)
                         close_scope() end
                     close_scope() end
 
-                    add_child(root, scope2)
+                    set_owner(scope2, root)
                     do open_scope(scope2)
                         clean "scope2"
                         bind2 = node()
-                        add_child(scope2, bind2)
+                        set_owner(bind2, scope2)
                         do open_scope(bind2)
                             clean "bind2"
                             track(selected)
@@ -284,6 +332,14 @@ TEST("graph", function()
 
         local a, b, c, d, e, f = node(), node(), node(), node(), node(), node()
 
+        local root = node()
+        set_owner(a, root)
+        set_owner(b, root)
+        set_owner(c, root)
+        set_owner(d, root)
+        set_owner(e, root)
+        set_owner(f, root)
+
         function b.effect(x)
             update(d)
             return not x
@@ -418,7 +474,6 @@ TEST("derive()", wrap_root(function()
     local derive = vide.derive
     local effect = vide.effect
     local cleanup = vide.cleanup
-    local untrack = vide.untrack
 
     do CASE "derive new value on source change"
         local a = source(1)
@@ -524,38 +579,43 @@ TEST("derive()", wrap_root(function()
         CHECK(count == 2)
     end
 
-    do CASE "child with parent as owner not lost"
+    -- do CASE "behavior of effect within an effect"
-        local num = source(0)
+    --     local num = source(1)
 
-        local cleaned = {}
+    --     local ran = table.create(100, 0)
+    --     local cleaned = table.create(100, 0)
 
-        local destroy = vide.mount(function()
+    --     local destroy = vide.mount(function()
-            local owner = derive(function()
+    --         local owner = derive(function()
-                local i = num()
+    --             local i = num()
 
-                return untrack(function()
+    --             return untrack(function()
-                    return derive(function()
+    --                 return derive(function()
-                        cleanup(function()
+    --                     ran[i] += 1
-                            cleaned[i] = true
+    --                     cleanup(function()
-                        end)
+    --                         cleaned[i] += 1
-                        return i
+    --                     end)
-                    end)
+    --                     return i
-                end)
+    --                 end)
-            end)
+    --             end)
+    --         end)
 
-            local child1 = owner()
+    --         local child1 = owner()
-            num(1)
+    --         num(2)
-            local child2 = owner()
+    --         CHECK(cleaned[1] == 1)
+    --         local child2 = owner()
 
-            CHECK(child1() == 0)
+    --         CHECK(child1() == 1)
-            CHECK(child2() == 1)
+    --         CHECK(child2() == 2)
-        end)
+    --     end)
 
-        destroy()
+    --     destroy()
         
-        CHECK(cleaned[0])
+    --     CHECK(ran[1] == 1)
-        CHECK(cleaned[1])
+    --     CHECK(ran[2] == 1)
-    end
+    --     CHECK(cleaned[1] == 1)
+    --     CHECK(cleaned[2] == 1)
+    -- end
 
     do CASE "garbage collection"
         -- check that `b` does not allow gc of `a`        
@@ -1070,11 +1130,15 @@ TEST("indexes()", wrap_root(function()
 
         local count = table.create(3, 0)
 
+        local output = vide.root(function()
             local output = indexes(input, function(v, i)
                 count[i] += 1
                 return v
             end)
 
+            return output
+        end)
+
         input { 1, 2, 4 }
 
         CHECK(output()[1]() == 1)
@@ -1453,11 +1517,8 @@ TEST("spring()", wrap_root(function()
 end))
 
 TEST("untrack()", wrap_root(function()
-    local root = vide.root
     local source = vide.source
-    local derive = vide.derive
     local effect = vide.effect
-    local cleanup = vide.cleanup
     local untrack = vide.untrack
 
     do CASE "does not register dependency"
@@ -1506,56 +1567,69 @@ TEST("untrack()", wrap_root(function()
         CHECK(count == 2)
     end
 
-    do CASE "outer scope"
+    -- do CASE "outer scope"
-        local outer_count = 0
+    --     local outer_count = 0
-        local inner_count = 0
+    --     local inner_count = 0
-        local cleaned_count = 0
+    --     local cleaned_count = 0
 
-        local input = source(0)
+    --     local input = source(0)
 
-        local output, destroy = root(function(destroy)
+    --     local output, destroy = root(function(destroy)
-            local output = derive(function()
+    --         local output = derive(function()
-                outer_count += 1
+    --             outer_count += 1
 
-                return untrack(function()
+    --             return untrack(function()
-                    return derive(function()
+    --                 return derive(function()
-                        inner_count += 1
+    --                     inner_count += 1
     
-                        cleanup(function()
+    --                     cleanup(function()
-                            cleaned_count += 1
+    --                         cleaned_count += 1
-                        end)
+    --                     end)
     
-                        return tostring(input())
+    --                     return tostring(input())
+    --                 end)
+    --             end)
+    --         end)
+
+    --         return output, destroy
+    --     end)
+
+    --     CHECK(outer_count == 1)
+    --     CHECK(inner_count == 1)
+    --     CHECK(cleaned_count == 0)
+
+    --     local output2 = output()
+
+    --     CHECK(output2() == "0")
+
+    --     input(1)
+
+    --     CHECK(outer_count == 1)
+    --     CHECK(inner_count == 2)
+    --     CHECK(cleaned_count == 1)
+
+    --     local output3 = output()
+    --     CHECK(output2() == "1")
+    --     CHECK(output3() == "1")
+
+    --     CHECK(output2 == output3)
+
+    --     destroy()
+
+    --     CHECK(cleaned_count == 2)
+    -- end
+
+    do CASE "cannot create effect within untrack()"
+        local ok = pcall(function()
+            effect(function()
+                untrack(function()
+                    effect(function() end)
+                    return  nil
                 end)
             end)
         end)
 
-            return output, destroy
+        CHECK(not ok)
-        end)
-
-        CHECK(outer_count == 1)
-        CHECK(inner_count == 1)
-        CHECK(cleaned_count == 0)
-
-        local output2 = output()
-
-        CHECK(output2() == "0")
-
-        input(1)
-
-        CHECK(outer_count == 1)
-        CHECK(inner_count == 2)
-        CHECK(cleaned_count == 1)
-
-        local output3 = output()
-        CHECK(output2() == "1")
-        CHECK(output3() == "1")
-
-        CHECK(output2 == output3)
-
-        destroy()
-
-        CHECK(cleaned_count == 2)
     end
 end))
 
@@ -1690,6 +1764,90 @@ TEST("read()", wrap_root(function()
     end
 end))
 
+TEST("nested effects cases", function()
+    -- local vide = require "src/init"
+    -- local source = vide.source
+    -- local effect = vide.effect
+    -- local untrack = vide.untrack
+    -- local cleanup = vide.cleanup
+    -- local root = vide.root
+
+    -- local ran = 0
+    -- local cleaned = 0
+
+    -- local function Count()
+    --     local count = source(0)
+
+    --     effect(function()
+    --         count()
+    --         ran += 1
+    --         cleanup(function() cleaned += 1 end)
+    --     end)
+
+    --     return nil
+    -- end
+
+    -- local function App(destroy)
+    --     local name = source "a"
+
+    --     effect(function()
+    --         name()
+    --         untrack(Count)
+    --     end)
+
+    --     CHECK(ran == 1)
+    --     CHECK(cleaned == 0)
+
+    --     name "b"
+
+    --     CHECK(ran == 2)
+    --     CHECK(cleaned == 1)
+    --     print(cleaned)
+    -- end
+
+    -- root(App)
+
+    local vide = require "src/init"
+    local source = vide.source
+    local effect = vide.effect
+    local untrack = vide.untrack
+    local cleanup = vide.cleanup
+    local root = vide.root
+
+    local ran = 0
+    local cleaned = 0
+
+    local function Count()
+        local count = source(0)
+
+        effect(function()
+            count()
+            ran += 1
+            cleanup(function() cleaned += 1 end)
+        end)
+
+        return nil
+    end
+
+    local function App(destroy)
+        local name = source "a"
+
+        effect(function()
+            name()
+            untrack(Count)
+        end)
+
+        CHECK(ran == 1)
+        CHECK(cleaned == 0)
+    end
+
+    local ok = pcall(function()
+        root(App)
+    end)
+
+    CHECK(not ok)
+end)
+
 vide.strict = true
 
 TEST("strict", wrap_root(function()