fix: prevent early reactive gc, alien-signals 3.1.1

packages/charm-sync/src/client.luau

@@ -8,7 +8,15 @@ type SyncPayload = types.SyncPayload
 type Setter<T = any> = (T | (T) -> any) -> ()
 
 local trackedSignals: { [string]: Setter } = {}
-local trackedReactives: { [string]: Setter } = {}
+local trackedReactives: { [string]: ReactiveProxy } = {}
+
+local function shallowMergeReactive(reactive: ReactiveProxy, source: { [any]: any })
+	local target = reactive.__target
+	for key, value in source do
+		target[key] = value
+	end
+	Charm.trigger(reactive.__track)
+end
 
 --[[
 	Registers a map of setters to the state with the corresponding keys on the
@@ -26,7 +34,7 @@ local function addSignals(signals: { [string]: Setter | {} })
 		if type(signal) == "function" then
 			trackedSignals[key] = signal
 		else
-			trackedReactives[key] = ((signal :: any) :: ReactiveProxy).__set
+			trackedReactives[key] = (signal :: any) :: ReactiveProxy
 		end
 	end
 end
@@ -54,9 +62,10 @@ end
 
 local function applyInitPayload(data: { [string]: any })
 	for key, value in data do
-		local trackedSignal: Setter? = trackedSignals[key] or trackedReactives[key]
-		if trackedSignal then
-			trackedSignal(value)
+		if trackedSignals[key] then
+			trackedSignals[key](value)
+		elseif trackedReactives[key] then
+			shallowMergeReactive(trackedReactives[key], value)
 		end
 	end
 end
@@ -71,9 +80,8 @@ local function applyPatchPayload(data: { [string]: any })
 				return patch.apply(value, diffs)
 			end)
 		elseif trackedReactive then
-			trackedReactive(function(value): ()
-				patch.applyMutable(value, diffs)
-			end)
+			patch.applyMutable(trackedReactive.__target, diffs)
+			Charm.trigger(trackedReactive.__track)
 		end
 	end
 end

packages/charm/src/__tests__/gc.spec.luau

@@ -66,33 +66,34 @@ test("should not release effect for atom", function()
 	assert(effectCalls == 2, "should call effect again")
 end)
 
-test("should release reactive tables", function()
+test("should release inner reactive proxy", function()
 	local valueRef = weakRef({})
-	local src = reactive({ value = valueRef.value })
+	local src: any = reactive({ value = valueRef.value })
 
-	effect(function()
+	local dispose = effect(function()
 		local _ = src.value -- create proxy
 	end)
 	src.value = nil :: any -- release proxy
+	dispose()
 
 	gc()
-	assert(valueRef.value == nil, "should release reactive table")
-	assert((src :: ReactiveProxy).__signals.value == nil, "should release reactive signals")
+	assert(src.__signals.value == nil, "should release signals")
+	assert(valueRef.value == nil, "should release inner proxy")
 end)
 
 test("should release reactive property deps", function()
-	local src = reactive({ foo = 1 })
+	local src: any = reactive({ foo = 1 })
 
 	local c = computed(function()
 		return src.foo
 	end)
 
 	c()
-	assert((src :: ReactiveProxy).__signals.foo ~= nil, "should have deps for foo")
+	assert(src.__signals.foo ~= nil, "should have deps for foo")
 
 	c = nil :: any
 	gc()
-	assert((src :: ReactiveProxy).__signals.foo == nil, "should release deps for foo")
+	assert(src.__signals.foo == nil, "should release deps for foo")
 end)
 
 test("should not release effect for reactive", function()
@@ -110,4 +111,17 @@ test("should not release effect for reactive", function()
 	assert(calls == 2, "should call effect again")
 end)
 
+test("should not release tracked reactive", function()
+	local src = Charm.reactive({ users = {} :: { [string]: {}? } })
+	local user
+
+	effect(function()
+		user = src.users.user
+	end)
+
+	gc()
+	src.users.user = {}
+	assert(user ~= nil, "src.users.user should not be released")
+end)
+
 return {}

packages/charm/src/__tests__/reactive.spec.luau

@@ -111,14 +111,18 @@ test("should not track unread properties", function()
 end)
 
 test("should overwrite with table passed to setter", function()
-	local src, setSrc = reactive({ a = 1, b = 2, c = 3 } :: { [string]: number })
+	local src, setSrc = reactive({ a = 1, b = 2, c = 3 } :: { [string]: number? })
 	local calls = {}
 
 	effect(function()
 		table.insert(calls, `{src.a}{src.b}{src.c}`)
 	end)
 
-	setSrc({ a = 2, c = 4 })
+	setSrc(function(src)
+		src.a = 2
+		src.b = nil
+		src.c = 4
+	end)
 	assert(table.concat(calls, ",") == "123,2nil4", table.concat(calls, ","))
 end)
 

packages/charm/src/__tests__/trigger.spec.luau

@@ -69,6 +69,22 @@ test("should trigger effect once", function()
 	assert(triggers == 2, "effect should run twice")
 end)
 
+test("should not notify the trigger function sub", function()
+	local src1 = signal({})
+	local src2 = computed(function()
+		return src1()
+	end)
+
+	effect(function()
+		src1()
+		src2()
+	end)
+	trigger(function()
+		src1()
+		src2()
+	end)
+end)
+
 test("should trigger updates for subscribe", function()
 	local src = signal({} :: { number })
 	local triggers = 0

packages/charm/src/init.luau

@@ -1,7 +1,9 @@
 local system = require("@self/system")
 
 export type ReactiveFlags = system.ReactiveFlags
+
 export type ReactiveNode = system.ReactiveNode
+
 export type Link = system.Link
 
 export type Atom<T = any> = (...((T) -> T) | T) -> T
@@ -9,11 +11,10 @@ export type Atom<T = any> = (...((T) -> T) | T) -> T
 export type ReactiveProxy<T = any> = typeof(setmetatable(
 	{} :: {
 		__track: () -> (),
-		__set: (T | ((T) -> ())?) -> (),
 		__target: T,
 		__signals: { [unknown]: () -> unknown },
 	},
-	{}
+	{} :: { __index: T }
 ))
 
 type EffectScopeNode = ReactiveNode & {
@@ -73,6 +74,10 @@ local propagate: (link: Link) -> ()
 local checkDirty: (link: Link, sub: ReactiveNode) -> boolean
 local shallowPropagate: (link: Link) -> ()
 
+local ReactiveProxy = {
+	proxyMap = setmetatable({} :: { [any]: ReactiveProxy }, { __mode = "v" }),
+}
+
 local cycle = 0
 local batchDepth = 0
 local notifyIndex = 0
@@ -80,9 +85,6 @@ local queued: { EffectNode? } = {}
 local queuedLength = 0
 local activeSub: ReactiveNode?
 
-local ReactiveProxy = {}
-local proxyMap = setmetatable({} :: { [any]: ReactiveProxy }, { __mode = "v" })
-
 local function noYield<Args..., Results...>(fn: (Args...) -> Results...): (Args...) -> Results...
 	if not flags.strict then
 		return fn
@@ -504,13 +506,15 @@ local function trigger(fn: () -> ())
 	local prevSub = setActiveSub(sub)
 	local success, result: any = pcall(noYield(fn))
 	activeSub = prevSub
-	while sub.deps do
-		local link = sub.deps :: Link
+	local link = sub.deps
+	while link do
 		local dep = link.dep
-		unlink(link, sub)
-		if dep.subs then
-			propagate(dep.subs)
-			shallowPropagate(dep.subs)
+		link = unlink(link, sub)
+		local subs = dep.subs
+		if subs then
+			sub.flags = NONE
+			propagate(subs)
+			shallowPropagate(subs)
 		end
 	end
 	if batchDepth == 0 then
@@ -848,7 +852,7 @@ end
 ]]
 local function toRaw<T>(value: T): T
 	if isReactive(value) then
-		local value: ReactiveProxy<T> = value :: any
+		local value = value :: ReactiveProxy<T> & T
 		value.__track()
 		return toRaw(value.__target)
 	end
@@ -881,42 +885,41 @@ end
 	@return A function for updating the original table.
 	@see https://github.com/littensy/charm?tab=readme-ov-file#reactiveinitialvalue
 ]]
-local function reactive<T>(initialValue: T & {}, trackParent: (() -> ())?): (T, (update: (T) -> () | T?) -> ())
+local function reactive<T>(initialValue: T & {}, trackParent: (() -> ())?): (T, (update: ((T) -> ())?) -> ())
 	initialValue = toRaw(initialValue)
 
-	local proxy = proxyMap[initialValue]
+	local proxy = ReactiveProxy.proxyMap[initialValue]
 
 	if not proxy then
-		local track = computed(function()
+		local track = computed(function(version: number?)
+			-- Reference the proxy to avoid GCing while an effect is actively
+			-- tracking this reactive object.
+			local _ref = proxy
+			-- Allow updates from the parent to propagate down when the
+			-- target is mutated directly through the setter.
 			if trackParent then
 				trackParent()
 			end
-			return {}
+			return (version or 0) + 1
 		end)
 
-		local function set(update: (T) -> () | T?)
-			if type(update) == "function" then
-				update(initialValue)
-			elseif type(update) == "table" then
-				table.clear(initialValue)
-				for key, value in update :: {} do
-					(initialValue :: {})[key] = value
-				end
-			end
-			trigger(track)
-		end
-
 		proxy = setmetatable({
 			__target = initialValue,
 			__signals = setmetatable({}, { __mode = "v" }) :: any,
 			__track = track,
-			__set = set,
 		}, ReactiveProxy)
 
-		proxyMap[initialValue] = proxy
+		ReactiveProxy.proxyMap[initialValue] = proxy
 	end
 
-	return proxy :: any, proxy.__set
+	local function setValue(update: ((T) -> ())?)
+		if update then
+			update(proxy.__target)
+		end
+		trigger(proxy.__track)
+	end
+
+	return proxy :: any, setValue
 end
 
 function ReactiveProxy.__index(self: ReactiveProxy, key: any): any