fix recursive queue flush (#28)

* fix recursive queue flush

* remove print from test

* swap order of flush

* simplified test case and remove error

* write more in-depth tests

* change approach flush_update_queue

* double diamond test case

* fix indent

* change naming of function

* subsequent updates do not batch test case

* simplify test case

src/batch.luau

@@ -6,17 +6,18 @@ local graph = require(script.Parent.graph)
 
 local function batch(setter: () -> ())
     local already_batching = flags.batch
+    local from
 
-    flags.batch = true
+    if not already_batching then
+        flags.batch = true
+        from = graph.get_update_queue_length()
+    end
 
     local ok, err: string? = pcall(setter)
 
     if not already_batching then
         flags.batch = false
-        
+        graph.flush_update_queue(from)
-        if not already_batching then
-            graph.flush_update_queue()
-        end
     end
 
     if not ok then throw(`error occured while batching updates: {err}`) end

src/graph.luau

@@ -184,14 +184,12 @@ local function queue_children_for_update<T>(node: SourceNode<T>)
     update_queue.n = i
 end
 
-local _flushing = false
+local function get_update_queue_length()
-local function flush_update_queue()
+    return update_queue.n
-    assert(not _flushing, "recursive queue flush occured") -- todo
+end
-    _flushing = true
 
-    local n0 = 0
+local function flush_update_queue(from: number)
-
+    local i = from + 1
-    local i = n0 + 1
     while i <= update_queue.n do
         local node = update_queue[i]
         --assert(node.effect)
@@ -203,10 +201,8 @@ local function flush_update_queue()
         update_queue[i] = false :: any
         i += 1
     end
-
+    
-    update_queue.n = n0
+    update_queue.n = from
-
-    _flushing = false
 end
 
 local function update_descendants<T>(root: SourceNode<T>)
@@ -286,5 +282,6 @@ return table.freeze {
     create_source_node = create_source_node,
     get_children = get_children,
     flush_update_queue = flush_update_queue,
+    get_update_queue_length = get_update_queue_length,
     scopes = scopes
 }

test/tests.luau

@@ -1842,6 +1842,7 @@ end))
 TEST("batch()", wrap_root(function()
     local source = vide.source
     local derive = vide.derive
+    local effect = vide.effect
     local batch = vide.batch
 
     do CASE "evaluation deferred"
@@ -1920,6 +1921,210 @@ TEST("batch()", wrap_root(function()
         CHECK(b2() == 3)
         CHECK(b3() == 4)
     end
+
+    do CASE "subsequent updates do not batch"
+
+        local a = source(0)
+        local b = source(0)
+        local c = source(0)
+        local d_n = 0
+
+        effect(function()
+            b()
+            c()
+            d_n += 1
+        end)
+
+        effect(function()
+            b(a())
+            c(a())
+        end)
+        
+        batch(function()
+            a(1)
+        end)
+
+        CHECK(d_n == 3)
+
+    end
+
+    do CASE "recursive queue flush diamond A,B,C,D"
+        --[[
+
+        a > b > d
+          > c >
+
+        ]]
+
+        local a = source(0)
+
+        local b = source(0)
+        local c = source(0)
+        local d = source(0)
+
+        local count = { b = 0, c = 0, d = 0 }
+        effect(function()
+            batch(function()
+                b(a() % 2 == 0 and 1 or 0)
+                c(a() * 2)
+            end)
+            count.b += 1
+            count.c += 1
+        end)
+
+        effect(function()
+            batch(function()
+                d(b() + c())
+            end)
+            count.d += 1
+        end)
+
+        a(1)
+        CHECK(count.b == 2)
+        CHECK(count.c == 2)
+        CHECK(count.d == 2)
+        CHECK(d() == 2)
+
+        a(3)
+        CHECK(count.b == 3)
+        CHECK(count.c == 3)
+        CHECK(count.d == 3)
+        CHECK(d() == 6)
+    end
+
+    do CASE "recursive queue flush diamond A,B,C,D,E"
+        --[[
+        where b and c batches d
+        
+        a > b >     e
+          > c > d >
+        
+        ]]
+
+        local a = source(0)
+
+        local b = source(0)
+        local c = source(0)
+        local d = source(0)
+        local e = source(0)
+
+        local count = { b = 0, c = 0, d = 0, e = 0 }
+        effect(function()
+            batch(function()
+                b(a() % 2 == 0 and 1 or 0)
+                c(a() * 2)
+            end)
+            count.b += 1
+            count.c += 1
+        end)
+
+        effect(function()
+            batch(function()
+                d(c() * 2)
+            end)
+            count.d += 1
+        end)
+
+        effect(function()
+            batch(function()
+                e(b() + d())
+            end)
+            count.e += 1
+        end)
+
+        CHECK(e() == 1)
+
+        a(1)
+
+        CHECK(count.b == 2)
+        CHECK(count.c == 2)
+        CHECK(count.d == 2)
+        CHECK(count.e == 3)
+        CHECK(e() == 4)
+
+        a(3)
+        CHECK(count.b == 3)
+        CHECK(count.c == 3)
+        CHECK(count.d == 3)
+        CHECK(count.e == 4)
+        CHECK(e() == 12)
+
+    end
+
+    do CASE "recursive queue flush diamond A,B,C,D,E,F,G"
+        --[[
+
+        a > b > d > E > G
+          > c ^   > F  
+
+        ]]
+
+        local a = source(0)
+
+        local b = source(0)
+        local c = source(0)
+        local d = source(0)
+        
+        local e = source(0)
+        local f = source(0)
+        local g = source(0)
+
+        local count = { b = 0, c = 0, d = 0, e = 0, f = 0, g = 0 }
+        effect(function()
+            batch(function()
+                b(a() % 2 == 0 and 1 or 0)
+                c(a() * 2)
+            end)
+            count.b += 1
+            count.c += 1
+        end)
+
+        effect(function()
+            batch(function()
+                d(b() + c())
+            end)
+            count.d += 1
+        end)
+
+        effect(function()
+            batch(function()
+                e(d() % 2 == 0 and 1 or 0)
+                f(d() * 2)
+            end)
+            count.e += 1
+            count.f += 1
+        end)
+
+        effect(function()
+            batch(function()
+                g(e() + f())
+            end)
+            count.g += 1
+        end)
+
+        a(1)
+        CHECK(count.b == 2)
+        CHECK(count.c == 2)
+        CHECK(count.d == 2)
+        CHECK(count.e == 2)
+        CHECK(count.f == 2)
+        CHECK(count.g == 2)
+        CHECK(d() == 2)
+        CHECK(g() == 5)
+
+        a(3)
+        CHECK(count.b == 3)
+        CHECK(count.c == 3)
+        CHECK(count.d == 3)
+        CHECK(count.e == 3)
+        CHECK(count.f == 3)
+        CHECK(count.g == 3)
+        CHECK(d() == 6)
+        CHECK(g() == 13)
+
+
+    end
+
 end))
 
 TEST("read()", wrap_root(function()