fix(signals): defer onSettled fallback cleanup to owner disposal (closes #2766)

packages/solid-signals/src/signals.ts

@@ -745,15 +745,33 @@ export function createOptimistic<T>(
  */
 export function onSettled(callback: () => void | (() => void)): void {
   const owner = getOwner();
-  owner && !(owner._config & CONFIG_CHILDREN_FORBIDDEN)
-    ? createTrackedEffect(() => untrack(callback), __DEV__ ? { name: "onSettled" } : undefined)
-    : globalQueue.enqueue(EFFECT_USER, () => {
-        const cleanup = callback();
-        if (__DEV__ && cleanup !== undefined && typeof cleanup !== "function") {
-          throw new Error(
-            "onSettled callback returned an invalid cleanup value. Return a cleanup function or undefined."
-          );
-        }
-        cleanup?.();
-      });
+  if (owner && !(owner._config & CONFIG_CHILDREN_FORBIDDEN)) {
+    createTrackedEffect(() => untrack(callback), __DEV__ ? { name: "onSettled" } : undefined);
+    return;
+  }
+  // Fallback path: either no owner, or the current owner is a children-
+  // forbidden scope (the most common case is `onSettled()` called from
+  // inside another `onSettled()` callback, which itself runs as a
+  // tracked effect). The original implementation invoked the returned
+  // cleanup eagerly inside the same flush, which immediately tore down
+  // setup helpers (#2766). Defer cleanup via the captured owner's
+  // disposal chain so it fires on real disposal, not on settle.
+  const capturedOwner = owner;
+  globalQueue.enqueue(EFFECT_USER, () => {
+    const result = callback();
+    if (__DEV__ && result !== undefined && typeof result !== "function") {
+      throw new Error(
+        "onSettled callback returned an invalid cleanup value. Return a cleanup function or undefined."
+      );
+    }
+    if (typeof result === "function") {
+      if (capturedOwner) {
+        runWithOwner(capturedOwner, () => cleanup(result as Disposable));
+      } else {
+        // No owner to scope the cleanup to — the helper is genuinely
+        // orphaned, so we have no point at which to fire it. Drop it
+        // rather than run it immediately and break the setup contract.
+      }
+    }
+  });
 }

packages/solid-signals/tests/onSettled.test.ts

@@ -129,14 +129,17 @@ it("should call cleanup when disposed", () => {
   expect(cleanup).toHaveBeenCalledTimes(1);
 });
 
-it("should call cleanup immediately when no owner", () => {
+it("should drop fallback cleanup when no owner is available", () => {
   const cleanup = vi.fn();
 
   onSettled(() => cleanup);
 
   expect(cleanup).toHaveBeenCalledTimes(0);
   flush();
-  expect(cleanup).toHaveBeenCalledTimes(1);
+  // No owner means no scope to anchor disposal to. Eager firing during the
+  // same flush is what #2766 explicitly fixed; dropping the cleanup is the
+  // intentional fallback so setup helpers are not torn down on settle.
+  expect(cleanup).toHaveBeenCalledTimes(0);
 });
 
 it("should throw on invalid cleanup values", () => {
@@ -224,3 +227,30 @@ it("should call cleanup on re-run after async settles", async () => {
   // Now callback runs
   expect(effect).toHaveBeenCalledWith("done");
 });
+
+it("defers cleanup from nested onSettled to owner disposal (#2766)", () => {
+  // Helper that uses onSettled internally to install + tear down a
+  // subscription. The parent calls the helper from inside another
+  // onSettled — under the bug, the helper's cleanup was invoked eagerly
+  // in the same flush, immediately tearing down the subscription.
+  const log: string[] = [];
+  function useSubscription() {
+    onSettled(() => {
+      log.push("subscribed");
+      return () => log.push("unsubscribed");
+    });
+  }
+
+  const dispose = createRoot(stop => {
+    onSettled(() => {
+      useSubscription();
+    });
+    return stop;
+  });
+
+  flush();
+  expect(log).toEqual(["subscribed"]);
+
+  dispose();
+  expect(log).toEqual(["subscribed", "unsubscribed"]);
+});