Warn on a yield in strict mode instead of cancelling the thread

docs/api/strict-mode.md

@@ -15,7 +15,7 @@ and identifying improper usage.
 Currently, strict mode will:
 
 1. Run reactive scopes twice when a source updates.
-2. Throw an error if yields occur where they are not allowed.
+2. Warn if yields occur where they are not allowed.
 3. Checks for `values()` input having duplicate values.
 4. Checks for duplicate nested properties at same depth.
 5. Checks for destruction of an active scope.

src/graph.luau

@@ -1,5 +1,7 @@
 local flags = require "./flags"
 
+local warn = warn or print
+
 export type SourceNode<T> = {
     cache: T,
     [number]: Node<T>
@@ -42,28 +44,33 @@ local function effect_traceback(err: unknown): string
     return `effect error:\n{debug.traceback(message, 2)}`
 end
 
-local function ycall<T, U>(fn: (T) -> U, arg: T): (boolean, string|U)
+-- also returns whether fn yielded, its thread is then left to finish on its own
+local function ycall<T, U>(fn: (T) -> U, arg: T): (boolean, string|U, boolean)
 
     local n = scopes.n
-    local thread = coroutine.create(xpcall)
+    local yielded = false
+    local thread = coroutine.create(function()
+        local ok, result = xpcall(fn, efn, arg)
+        if yielded and not ok then error(result, 0) end -- no caller left to return it to
+        return ok, result
+    end)
     --local function efn(err: string) return debug.traceback(err, 3) end
-    local resume_ok, run_ok, result = coroutine.resume(thread, fn, efn, arg)
+    local resume_ok, run_ok, result = coroutine.resume(thread)
 
     assert(resume_ok)
         
     if coroutine.status(thread) ~= "dead" then
-        local trace = debug.traceback(thread, "attempt to yield in reactive scope")
+        yielded = true
+        warn(debug.traceback(thread, "attempt to yield in reactive scope"))
 
-        -- the thread can still be resumed (e.g. by task.wait), so stop it and
-        -- pop the scopes it pushed before yielding
-        if task then task.cancel(thread) else coroutine.close(thread) end
-        for i = scopes.n, n + 1, -1 do scopes[i] = nil end
-        scopes.n = n
+        -- the thread pops the scopes it pushed once resumed, so move the
+        -- caller's scope above them for the caller to pop
+        table.insert(scopes, table.remove(scopes, n) :: Node<any>)
 
-        return false, trace
+        return true, result, true
     end
 
-    return run_ok, result
+    return run_ok, result, false
 end
 
 local function get_scope(): Node<unknown>?
@@ -199,9 +206,11 @@ local function try_evaluate_node<T>(node: Node<T>): (boolean, string?)
             err = err or cleanup_err or owned_err
     
             push_scope(node)
-            local ok, new_value = ycall(node.effect :: (T) -> T, cur_value)
+            local ok, new_value, yielded = ycall(node.effect :: (T) -> T, cur_value)
             pop_scope()
 
+            if yielded then break end
+            
             if not ok then
                 return false, `effect error stacktrace\n{new_value :: string}`
             end

test/tests.luau

@@ -3146,7 +3146,7 @@ end))
 TEST("strict", wrap_root(function()
     vide.strict = true
 
-    do CASE "error on derived callback yield"
+    do CASE "warn on derived callback yield"
         local src = source(1)
 
         local ok = pcall(function()
@@ -3156,10 +3156,10 @@ TEST("strict", wrap_root(function()
             end)
         end)
 
-        CHECK(not ok)
+        CHECK(ok)
     end
 
-    do CASE "error on effecter callback yield"
+    do CASE "warn on effecter callback yield"
         local src = source(1)
 
         local ok = pcall(function()
@@ -3169,26 +3169,26 @@ TEST("strict", wrap_root(function()
             end)
         end)
 
-        CHECK(not ok)
+        CHECK(ok)
     end
 
     do CASE "yield in a component"
         local input = source(false)
         local thread
-        local resumed = false
+        local output
 
         local destroy = root(function()
-            show(input, function()
+            output = show(input, function()
                 thread = coroutine.running()
                 coroutine.yield()
-                resumed = true
-                return {}
+                return "component"
             end)
         end)
 
-        CHECK(not pcall(input, true))
-        CHECK(not coroutine.resume(thread))
-        CHECK(not resumed)
+        CHECK((pcall(input, true)))
+        CHECK(output() == nil)
+        CHECK((coroutine.resume(thread)))
+        CHECK(output() == "component")
         CHECK(pcall(input, false))
         CHECK(pcall(destroy))
     end