From d890f4fc2cb5bf4eacf65ebfea268ebb0ec016e7 Mon Sep 17 00:00:00 2001 From: littensy <56808540+littensy@users.noreply.github.com> Date: Wed, 5 Nov 2025 02:30:41 -0800 Subject: [PATCH] feat: add `recursive()` --- README.md | 33 +++++++--- packages/charm/src/__tests__/effect.spec.luau | 13 ++++ packages/charm/src/alien.luau | 8 --- packages/charm/src/core.luau | 19 ++++++ packages/charm/src/globals.luau | 4 -- packages/charm/src/index.d.ts | 63 +++++++++++++++++-- packages/charm/src/init.luau | 1 + packages/charm/src/observe.luau | 17 ++--- 8 files changed, 120 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 7747b2d..b210cbd 100644 --- a/README.md +++ b/README.md @@ -540,16 +540,33 @@ max(-1) --> 1 --- +### `recursive()` + +By design, Charm's reactive system [does not allow recursion](https://github.com/stackblitz/alien-signals/issues/90#issuecomment-3489711800) in effects or computed signals. In case you need to opt out of recursion checks for an effect, you can call `recursive()` at the top of the effect callback: + +```luau +local getCounter, setCounter = signal(0) + +effect(function() + recursive() + print(getCounter()) + setCounter(function(count) + return math.min(count + 1, 3) + end) +end) --> 0, 1, 2, 3 +``` + +--- + ### `globals` 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. | +| 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. | The `strict` and `frozen` flags are automatically enabled when the Luau optimization level is less than `2`, which is true in Roblox Studio. @@ -831,7 +848,7 @@ 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 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`. + - This feature can introduce runtime bugs in migrated code. If you suspect this to be the cause, to help identify the issue, 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`. @@ -839,7 +856,7 @@ Charm v0.11 introduces _a lot_ of breaking changes, so below are some tips that > 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. 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. + - Recursive checks are opt-out. You can allow recursion for a specific effect by calling [`recursive()`](#recursive) at the top of the effect callback. 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 diff --git a/packages/charm/src/__tests__/effect.spec.luau b/packages/charm/src/__tests__/effect.spec.luau index 071d2a0..96b0256 100644 --- a/packages/charm/src/__tests__/effect.spec.luau +++ b/packages/charm/src/__tests__/effect.spec.luau @@ -6,6 +6,7 @@ local effect = core.effect local effectScope = core.effectScope local endBatch = core.endBatch local onCleanup = core.onCleanup +local recursive = core.recursive local setActiveSub = core.setActiveSub local signal = core.signal local startBatch = core.startBatch @@ -290,6 +291,18 @@ test("should handle effect recursion for the first execution", function() assert(triggers2 == 1, `bad triggers2: {triggers2}`) end) +test("should opt in to recursion", function() + local src, setSrc = signal(0) + local order = {} + + effect(function() + recursive() + table.insert(order, src()) + setSrc(math.min(src() + 1, 5)) + end) + assert(table.concat(order) == "012345", `bad order: {table.concat(order)}`) +end) + test("should run cleanups in sequence", function() local order = {} diff --git a/packages/charm/src/alien.luau b/packages/charm/src/alien.luau index 5a610e2..495a5c3 100644 --- a/packages/charm/src/alien.luau +++ b/packages/charm/src/alien.luau @@ -1,4 +1,3 @@ -local globals = require("./globals") --[[ Reactive system adapted from alien-signals at https://github.com/stackblitz/alien-signals/blob/master/src/system.ts @@ -168,17 +167,10 @@ 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 diff --git a/packages/charm/src/core.luau b/packages/charm/src/core.luau index cdcb6ac..9cf1467 100644 --- a/packages/charm/src/core.luau +++ b/packages/charm/src/core.luau @@ -555,6 +555,24 @@ local function subscribe(getter: () -> T, callback: (state: T, lastState: T) end) end +--- Disables recursive checks for the current effect or computed signal. Useful +--- for effects that intentionally update signals they depend on. +--- +--- @example +--- ```luau +--- effect(function() +--- recursive() +--- setCount(getCount() + 1) +--- end) +--- ``` +local function recursive() + if activeSub then + activeSub.flags = bit32.band(activeSub.flags, bit32.bnot(RECURSED_CHECK)) + else + warn(debug.traceback("recursive() can only be called inside an effect or computed signal.", 2)) + end +end + local reactiveSystem = createReactiveSystem({ update = function(node) if node.depsTail then @@ -626,4 +644,5 @@ return { batch = batch, subscribe = subscribe, listen = listen, + recursive = recursive, } diff --git a/packages/charm/src/globals.luau b/packages/charm/src/globals.luau index 26273d9..af6a381 100644 --- a/packages/charm/src/globals.luau +++ b/packages/charm/src/globals.luau @@ -17,8 +17,4 @@ 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, } diff --git a/packages/charm/src/index.d.ts b/packages/charm/src/index.d.ts index 8400040..82ca1d8 100644 --- a/packages/charm/src/index.d.ts +++ b/packages/charm/src/index.d.ts @@ -36,12 +36,6 @@ 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; }; /** @@ -238,3 +232,60 @@ export function observe( getter: () => { readonly [K in Key]: V }, callback: (value: V, key: K) => Cleanup | void, ): Cleanup; + +export namespace core { + export { signal, effect, effectScope, atom, computed, onCleanup, untracked, peek, batch, listen }; + + export enum ReactiveFlags { + None = 0b0000000, + Mutable = 0b0000001, + Watching = 0b0000010, + RecursedCheck = 0b0000100, + Recursed = 0b0001000, + Dirty = 0b0010000, + Pending = 0b0100000, + Peeking = 0b1000000, + } + + export interface ReactiveNode { + deps?: Link; + depsTail?: Link; + subs?: Link; + subsTail?: Link; + flags: ReactiveFlags; + } + + export interface Link { + version: number; + dep: ReactiveNode; + sub: ReactiveNode; + prevSub?: Link; + nextSub?: Link; + prevDep?: Link; + nextDep?: Link; + } + + /** + * Disables recursive checks for the current effect or computed signal. + * Useful for effects that intentionally update signals they depend on. + * + * @example + * ```ts + * effect(() => { + * recursive(); + * setCount(getCount() + 1); + * }) + * ``` + */ + export function recursive(): void; + + export function getActiveSub(): ReactiveNode | undefined; + + export function setActiveSub(sub?: ReactiveNode): ReactiveNode | undefined; + + export function getBatchDepth(): number; + + export function startBatch(): void; + + export function endBatch(): void; +} diff --git a/packages/charm/src/init.luau b/packages/charm/src/init.luau index 88f60ef..4aee63e 100644 --- a/packages/charm/src/init.luau +++ b/packages/charm/src/init.luau @@ -33,6 +33,7 @@ return { listen = core.listen, onCleanup = core.onCleanup, peek = core.peek, + recursive = core.recursive, signal = core.signal, subscribe = core.subscribe, untracked = core.untracked, diff --git a/packages/charm/src/observe.luau b/packages/charm/src/observe.luau index d169ac3..8511f43 100644 --- a/packages/charm/src/observe.luau +++ b/packages/charm/src/observe.luau @@ -1,10 +1,9 @@ local core = require("./core") -type ReactiveNode = core.ReactiveNode - local batch = core.batch local effect = core.effect local effectScope = core.effectScope +local recursive = core.recursive --- Calls the callback function for each unique key in the table returned by the --- getter. The callback is called once for each key, and again when a new key @@ -42,19 +41,13 @@ local function observe(getter: () -> { [K]: V }, callback: (item: V, key: return effectScope(function() effect(function() - local data = getter() - + -- Allow recursive updates in case the callback creates or removes + -- items in the observed table. + recursive() -- Batch so that side effects run after all scopes are created. -- This ensures that a scope creating or removing items doesn't -- interfere with the current update cycle. - batch(updateScopes, data) - - -- 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 + batch(updateScopes, getter()) end) return function()