From 4a48dacfbdd6d746fe2686d953bafcb4b52673fa Mon Sep 17 00:00:00 2001
From: littensy <56808540+littensy@users.noreply.github.com>
Date: Mon, 30 Mar 2026 04:11:57 -0700
Subject: [PATCH] fix: check if cleanup functions disposed effect

---
 packages/charm/src/init.luau         |  8 +++
 packages/charm/test/effect.test.luau | 76 ++++++++++++++++++++++++++++
 rokit.toml                           |  2 +-
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/packages/charm/src/init.luau b/packages/charm/src/init.luau
index dfaa6f4..2273f1b 100644
--- a/packages/charm/src/init.luau
+++ b/packages/charm/src/init.luau
@@ -202,6 +202,10 @@ local function run(effect: EffectNode)
 	if bit32.btest(flags, DIRTY) or (bit32.btest(flags, PENDING) and checkDirty(effect.deps :: Link, effect)) then
 		runCleanups(effect.cleanups)
 
+		if effect.flags == NONE then
+			return
+		end
+
 		cycle += 1
 		effect.depsTail = nil
 		effect.flags = WATCHING
@@ -217,6 +221,10 @@ local function run(effect: EffectNode)
 		elseif result then
 			table.insert(effect.cleanups, noYield(result))
 		end
+
+		if effect.flags == NONE then
+			runCleanups(effect.cleanups)
+		end
 	else
 		effect.flags = WATCHING
 	end
diff --git a/packages/charm/test/effect.test.luau b/packages/charm/test/effect.test.luau
index 9b526c4..6ed18fe 100644
--- a/packages/charm/test/effect.test.luau
+++ b/packages/charm/test/effect.test.luau
@@ -375,4 +375,80 @@ test("infinite recursion should eventually throw", function()
 	assert(not success, "should have thrown")
 end)
 
+test("should not retrigger the effect if the cleanup modifies one of the dependencies", function()
+	local a, setA = signal(0)
+	local calls = {}
+
+	effect(function()
+		table.insert(calls, a())
+		return function()
+			setA(2)
+		end
+	end)
+	assert(#calls == 1, "should have run once initially")
+	table.clear(calls)
+
+	setA(1)
+	assert(#calls == 1, "should run once after an update")
+	assert(calls[1] == 2, `bad call: {calls[1]}`)
+end)
+
+test("should run the cleanup if the effect disposes itself", function()
+	local a, setA = signal(0)
+	local calls = 0
+	local dispose
+
+	dispose = effect(function()
+		if a() > 0 then
+			dispose()
+			return function()
+				calls += 1
+			end
+		end
+	end)
+	assert(calls == 0, "should not have called cleanup initially")
+	setA(1)
+	assert(calls == 1, "should have called cleanup once after first update")
+	setA(2)
+	assert(calls == 1, "should not call cleanup again after second update")
+end)
+
+test("should not run the effect if the cleanup function disposes it", function()
+	local a, setA = signal(0)
+	local calls = 0
+	local dispose
+
+	dispose = effect(function()
+		a()
+		calls += 1
+		return function()
+			dispose()
+		end
+	end)
+	assert(calls == 1, "should have run effect once initially")
+	setA(1)
+	assert(calls == 1, "should not run effect again after update")
+end)
+
+test("should reset the cleanup if the effect throws", function()
+	local a, setA = signal(0)
+	local calls = 0
+
+	effect(function()
+		if a() == 0 then
+			return function()
+				calls += 1
+			end
+		else
+			error("hello")
+		end
+	end)
+	assert(calls == 0, "should not have called cleanup initially")
+	local success, err = pcall(setA, 1)
+	assert(not success and string.find(tostring(err), "hello"), `bad error: {err}`)
+	assert(calls == 1, "should have called cleanup once after throwing")
+	setA(0)
+	assert(calls == 1, "should not call cleanup again after resetting value")
+end)
+
 return {}
diff --git a/rokit.toml b/rokit.toml
index 331e403..5334d43 100644
--- a/rokit.toml
+++ b/rokit.toml
@@ -3,5 +3,5 @@ wally = "UpliftGames/wally@0.3.2"
 rojo = "rojo-rbx/rojo@7.6.1"
 stylua = "johnnymorganz/stylua@2.3.1"
 selene = "Kampfkarren/selene@0.30.0"
-luau-lsp = "johnnymorganz/luau-lsp@1.62.0"
+luau-lsp = "johnnymorganz/luau-lsp@1.63.0"
 lute = "luau-lang/lute@0.1.0-nightly.20260219"