feat: update alien-signals, add recursion flag

README.md

@@ -542,16 +542,17 @@ max(-1) --> 1
 
 ### `globals`
 
-Global flags that customize the behavior of Charm.
+You can customize the behavior of Charm through the `globals` table:
+
+| Flag              | Default        | Description                                                                                                                                                              |
+| ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+| strict            | `true`/`false` | Enforces synchronous, non-yielding behavior in signals, effects, and other critical code.                                                                                |
+| frozen            | `true`/`false` | Enforces data immutability by deep-freezing tables passed to signals.                                                                                                    |
+| trackInnerEffects | `true`         | Whether nested effects should be tracked and cleaned up when the parent effect re-runs. This should only be disabled to debug issues during migration.                   |
+| allowRecursion    | `false`        | Allows recursion in effects and computed signals, which is intentionally disallowed in the [alien-signals](https://github.com/stackblitz/alien-signals) reactive system. |
 
 The `strict` and `frozen` flags are automatically enabled when the Luau optimization level is less than `2`, which is true in Roblox Studio.
 
-| Flag              | Default        | Description                                                                                                                                       |
-| ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| strict            | `true`/`false` | Enforces synchronous, non-yielding behavior in signals, effects, and other critical code.                                                         |
-| frozen            | `true`/`false` | Enforces data immutability by deep-freezing tables passed to signals.                                                                             |
-| trackInnerEffects | `true`         | Whether nested effects should be tracked and cleaned up when the parent effect re-runs. Should only be disabled to debug issues during migration. |
-
 ---
 
 ## Client-Server Sync
@@ -830,14 +831,17 @@ Charm v0.11 introduces _a lot_ of breaking changes, so below are some tips that
 4. Nested effects automatically clean up when the parent effect re-runs or gets disposed. In other words, all effects created during the execution of another effect will be added as a "child" and clean up with the parent effect. This might cause issues in code that relied on the old behavior, where effects were detached from the parent.
     - This feature applies to all reaction APIs, including the listener function in `subscribe()` and the observer function in `observe()`.
     - Effects that should not be tracked by a parent effect/scope should be wrapped in [`untracked()`](#untrackedcallback).
-    - This feature can cause runtime bugs that are hard to track! If you need to narrow down a cause, you can disable this feature by setting [`globals.trackInnerEffects`](#globals) to `false`.
+    - This feature can introduce runtime bugs in legacy code. If you need to narrow down a cause, you can disable this feature by setting [`globals.trackInnerEffects`](#globals) to `false`.
 
 > [!NOTE]
 > An example of nested effects causing a bug is an old implementation of [`VideCharm.useAtom`](./packages/vide-charm/src/init.luau) that did not wrap the source update in `untracked()`. Because Vide effects run immediately after a source updates, Vide will notify components during the execution of the Charm effect in `useAtom`.
 >
 > This meant effects created as a side effect of a source update would implicitly get added as a child of the `useAtom` effect, and they could get disposed at the wrong time and desync UI.
 
-5. Consider refactoring your code to use some new quality-of-life features. Many of these are made possible with many thanks to [alien-signals](https://github.com/stackblitz/alien-signals)!
+5. Recursion is disallowed in effects and computed signals now that Charm uses the [alien-signals algorithm](https://github.com/stackblitz/alien-signals). This change may introduce bugs in code relying on the old behavior.
+    - You can disable recursion checks by setting [`globals.allowRecursion`](#globals) to `true`, but try to consider other options first, as recursion isn't fully supported and is not well optimized.
+
+6. Consider refactoring your code to use some new quality-of-life features. Many of these are made possible thanks to [alien-signals](https://github.com/stackblitz/alien-signals)!
     - Added [`signal()`](#signalinitialvalue-equals) to make reads and writes more explicit
     - Added [`listen()`](#listengetter-callback) for running a subscription once immediately
     - Added [`effectScope()`](#effectscopecallback) for collecting and cleaning up multiple effects at once
@@ -845,7 +849,6 @@ Charm v0.11 introduces _a lot_ of breaking changes, so below are some tips that
     - The [`computed()`](#computedgetter) callback now gets called with the previous computed value
     - Optimized `computed()` to use lazy evaluation instead of eager updates
     - Optimized `mapped()` to only map values that changed in the original table
-    - Effect execution and ordering is more reliable
 
 ---
 

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

@@ -269,6 +269,27 @@ test("should handle flags are indirectly updated during checkDirty", function()
 	assert(triggers == 2, "did not update indirectly")
 end)
 
+test("should handle effect recursion for the first execution", function()
+	local src1, setSrc1 = signal(0)
+	local src2, setSrc2 = signal(0)
+
+	local triggers1 = 0
+	local triggers2 = 0
+
+	effect(function()
+		triggers1 += 1
+		setSrc1(math.min(src1() + 1, 5))
+	end)
+	effect(function()
+		triggers2 += 1
+		setSrc2(math.min(src2() + 1, 5))
+		src2()
+	end)
+
+	assert(triggers1 == 1, `bad triggers1: {triggers1}`)
+	assert(triggers2 == 1, `bad triggers2: {triggers2}`)
+end)
+
 test("should run cleanups in sequence", function()
 	local order = {}
 

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

@@ -90,6 +90,29 @@ test("should queue addition and removal", function()
 	assert(table.concat(edits) == "+1+2+3+4-2-3-4", `bad edits: {table.concat(edits)}`)
 end)
 
+test("should handle recursive additions and removals", function()
+	local items, setItems = signal({ 1 })
+	local edits = {}
+
+	observe(items, function(item)
+		if item < 5 then
+			setItems(function(state)
+				state = table.clone(state)
+				table.insert(state, item + 1)
+				return state
+			end)
+		else
+			setItems({})
+		end
+		table.insert(edits, `+{item}`)
+		return function()
+			table.insert(edits, `-{item}`)
+		end
+	end)
+
+	assert(table.concat(edits) == "+1+2+3+4+5-1-2-3-4-5", `bad edits: {table.concat(edits)}`)
+end)
+
 test("should handle self removal", function()
 	local items, setItems = signal({ 1, 2, 3, 4 })
 	local edits = {}

packages/charm/src/alien.luau

@@ -1,3 +1,4 @@
+local globals = require("./globals")
 --[[
 	Reactive system adapted from alien-signals at
 	https://github.com/stackblitz/alien-signals/blob/master/src/system.ts
@@ -167,10 +168,17 @@ local function createReactiveSystem(props: {
 	end
 
 	local function propagate(link: Link): ()
+		local allowRecursion = globals.allowRecursion
+
 		repeat
 			local sub = link.sub
 			local flags = sub.flags
 
+			if allowRecursion then
+				flags = bit32.band(flags, bit32.bnot(bit32.bor(RECURSED, RECURSED_CHECK)))
+				sub.flags = flags
+			end
+
 			if bit32.band(flags, bit32.bor(RECURSED_CHECK, RECURSED, DIRTY, PENDING)) == 0 then
 				sub.flags = bit32.bor(flags, PENDING)
 			elseif bit32.band(flags, bit32.bor(RECURSED_CHECK, RECURSED)) == 0 then
@@ -185,7 +193,7 @@ local function createReactiveSystem(props: {
 				flags = NONE
 			end
 
-			if bit32.btest(flags, WATCHING) then
+			if bit32.band(flags, bit32.bor(WATCHING, RECURSED_CHECK)) == WATCHING then
 				notify(sub)
 			end
 

packages/charm/src/core.luau

@@ -51,7 +51,7 @@ type EffectScope = ReactiveNode & {
 }
 
 type Effect = ReactiveNode & {
-	fn: () -> (() -> ())?,
+	fn: () -> ...(() -> ())?,
 	cleanups: { () -> () },
 }
 
@@ -116,7 +116,7 @@ local function run(e: Effect): ()
 		e.depsTail = nil
 		e.flags = bit32.bor(WATCHING, RECURSED_CHECK)
 		local prevSub = setActiveSub(e)
-		local success, result: (() -> ())? = (if globals.strict then noYield else pcall)(e.fn)
+		local success, result = (if globals.strict then noYield else pcall)(e.fn)
 		activeSub = prevSub
 		e.flags = bit32.band(e.flags, bit32.bnot(RECURSED_CHECK))
 		purgeDeps(e)
@@ -162,7 +162,7 @@ local function updateComputed(c: Computed): boolean
 	c.flags = bit32.bor(MUTABLE, RECURSED_CHECK)
 	local prevSub = setActiveSub(c)
 	local oldValue = c.value
-	local success, result: unknown = (if globals.strict then noYield else pcall)(c.getter, oldValue)
+	local success, result = (if globals.strict then noYield else pcall)(c.getter, oldValue)
 	activeSub = prevSub
 	c.flags = bit32.band(c.flags, bit32.bnot(RECURSED_CHECK))
 	purgeDeps(c)
@@ -209,10 +209,11 @@ local function computedOper<T>(this: Computed<T>): () -> T
 				end
 			end
 		elseif flags == NONE then
-			this.flags = MUTABLE
+			this.flags = bit32.bor(MUTABLE, RECURSED_CHECK)
 			local prevSub = setActiveSub(this)
 			local success, result = pcall(this.getter)
 			activeSub = prevSub
+			this.flags = bit32.band(this.flags, bit32.bnot(RECURSED_CHECK))
 			if success then
 				this.value = result
 			else
@@ -235,7 +236,7 @@ local function signalOper<T>(
 		local value: T
 		if type(update) == "function" then
 			local prevSub = setActiveSub(nil)
-			local success, result: T = (if globals.strict then noYield else pcall)(update, this.pendingValue)
+			local success, result = (if globals.strict then noYield else pcall)(update, this.pendingValue)
 			activeSub = prevSub
 			if not success then
 				error(result)
@@ -365,18 +366,19 @@ end
 --- @param fn The function to run when dependencies change.
 --- @return A function for disposing the effect.
 --- @see https://github.com/littensy/charm?tab=readme-ov-file#effectcallback
-local function effect(fn: () -> ()): () -> ()
+local function effect(fn: () -> ...(() -> ())?): () -> ()
 	local e: Effect = {
 		fn = fn,
-		flags = WATCHING,
+		flags = bit32.bor(WATCHING, RECURSED_CHECK),
 		cleanups = {},
 	}
 	local prevSub = setActiveSub(e)
 	if prevSub and (globals.trackInnerEffects or prevSub.flags == NONE) then
 		link(e, prevSub, 0)
 	end
-	local success, result: (() -> ())? = (if globals.strict then noYield else pcall)(fn)
+	local success, result = (if globals.strict then noYield else pcall)(fn)
 	activeSub = prevSub
+	e.flags = bit32.band(e.flags, bit32.bnot(RECURSED_CHECK))
 	if not success then
 		error(result)
 	elseif result then
@@ -392,7 +394,7 @@ end
 --- @param detached If true, the scope will not be automatically disposed of when the parent scope is disposed. Defaults to false.
 --- @return A function for disposing all effects created within the scope.
 --- @see https://github.com/littensy/charm?tab=readme-ov-file#effectscopecallback-detached
-local function effectScope(fn: () -> (), detached: boolean?): () -> ()
+local function effectScope(fn: () -> ...(() -> ())?, detached: boolean?): () -> ()
 	local e: EffectScope = {
 		flags = NONE,
 		cleanups = {},
@@ -401,7 +403,7 @@ local function effectScope(fn: () -> (), detached: boolean?): () -> ()
 	if prevSub and not detached then
 		link(e, prevSub, 0)
 	end
-	local success, result: (() -> ())? = (if globals.strict then noYield else pcall)(fn)
+	local success, result = (if globals.strict then noYield else pcall)(fn)
 	activeSub = prevSub
 	if not success then
 		error(result)

packages/charm/src/globals.luau

@@ -17,4 +17,8 @@ return {
 	--- when the parent effect re-runs. This is usually desirable, but can be
 	--- disabled to revert to the old behavior. Defaults to `true`.
 	trackInnerEffects = true,
+	--- Whether to allow recursive updates in effects and computed signals.
+	--- Disabling this can help catch certain classes of bugs, but may also
+	--- cause valid code to error. Defaults to `false`.
+	allowRecursion = false,
 }

packages/charm/src/index.d.ts

@@ -36,6 +36,12 @@ export const globals: {
 	 * disabled to revert to the old behavior. Defaults to `true`.
 	 */
 	trackInnerEffects: boolean;
+	/**
+	 * Whether to allow recursive updates in effects and computed signals.
+	 * Disabling this can help catch certain classes of bugs, but may also
+	 * cause valid code to error. Defaults to `false`.
+	 */
+	allowRecursion: boolean;
 };
 
 /**

packages/charm/src/observe.luau

@@ -42,16 +42,19 @@ local function observe<K, V>(getter: () -> { [K]: V }, callback: (item: V, key:
 
 	return effectScope(function()
 		effect(function()
-			-- Batch so that side effects run after all scopes are created. This
+			local data = getter()
-			-- ensures that a scope creating or removing items doesn't interfere
-			-- with the current update cycle.
-			batch(updateScopes, getter())
 
-			-- Effects can't trigger themselves more than once, so if a scope
+			-- Batch so that side effects run after all scopes are created.
-			-- creates or removes items, we need to check for updates again. The
+			-- This ensures that a scope creating or removing items doesn't
-			-- way we do this abuses a weird property of signal accesses that
+			-- interfere with the current update cycle.
-			-- causes the effect to re-run.
+			batch(updateScopes, data)
-			getter()
+
+			-- Effects can't trigger themselves, so if a scope creates or
+			-- removes items, we need to check for updates again.
+			local newData = getter()
+			if newData ~= data then
+				batch(updateScopes, newData)
+			end
 		end)
 
 		return function()