fix(signals): a verdict-mounted Loading holds a nested render effect's first read (#4166)

.changeset/verdict-mount-nested-render-effect.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+A `<Loading>` that a verdict reader (`<Show when={latest(x) > 0}>`) mounts during a held action now holds a render effect nested in its content, not only a direct read: it shows its fallback beside the committed value until the action lands (#4166).

packages/signals/src/core/core.ts

@@ -232,6 +232,10 @@ export let stale = false;
 export let context: Owner | null = null;
 
 export let snapshotCaptureActive = false;
+/** The lane at the root of the running chain of first passes: the lane whose
+ * pass created the chain's first node (a mount), inherited by every first
+ * pass nested under it, whose seats are mainline (`recompute`, #4166). */
+let laneRoot: Transaction | null | false | undefined;
 export let snapshotSources: Set<any> | null = null;
 
 function ownerInSnapshotScope(owner: Owner | null): boolean {
@@ -317,6 +321,11 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
   // Lanes exist only with the lane engine installed (`laneStageHook`): a
   // bundle without it seats every pass on mainline, and folds the seat away.
   const prevLane = passLane;
+  // The mount a first pass belongs to: its creator's lane, or the one its
+  // creator's own mount came from — a render effect a verdict lane's mount's
+  // content creates is that mount's too (#4166). Lanes only, like the seat.
+  const prevMount = laneRoot;
+  const mount = laneStageHook && (laneRoot = create && (prevLane || prevMount));
   setPassLane(
     !laneStageHook
       ? null
@@ -455,7 +464,8 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
         : el._value;
   let oldHeight = el._height;
   let missedWake = false;
-  // L2: did this pass read the future (REACTIVE_JOINED, set by read)?
+  // L2: did this pass read the future (REACTIVE_JOINED, set by read) — or,
+  // a lane's mount, a staging of this flush (#3851, #4166)?
   let joined = false;
   let prevTracking = tracking;
   let prevStrictRead: string | false = false;
@@ -531,6 +541,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
     }
   } finally {
     tracking = prevTracking;
+    if (laneStageHook) laneRoot = prevMount;
     if (__DEV__) strictRead = prevStrictRead;
     if (isStaleEffect) stale = prevStale;
     // Consume the missed-wake latch (#3037, set by insertSubs): a dep write
@@ -539,7 +550,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
     // (markNode(c) in read()) marks the running node as part of ordinary
     // bookkeeping, and those marks are correctly discarded here.
     missedWake = (el._flags & REACTIVE_MISSED_WAKE) !== 0;
-    joined = (el._flags & REACTIVE_JOINED) !== 0;
+    joined = !!(el._flags & (mount ? REACTIVE_JOINED | REACTIVE_STAGED_READ : REACTIVE_JOINED));
     // A verdict reader is one for as long as it probes, or reads a memo
     // that does (`read`): a pass that did neither (REACTIVE_PROBED, the same
     // bit) is an ordinary derivation again.
@@ -719,9 +730,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
         create
           ? !(
               (flushTransaction !== null || passTx !== null) &&
-              (joined ||
-                (prevLane && el._flags & REACTIVE_STAGED_READ) ||
-                (creatorPass(oldcontext)?._flags as number) & REACTIVE_JOINED)
+              (joined || (creatorPass(oldcontext)?._flags as number) & REACTIVE_JOINED)
             )
           : isEffect && el._pendingValue === NOT_PENDING
       ) {
@@ -755,7 +764,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
           // #3851): the mount is part of the hold, and the boundary appears
           // at its commit, its fallback never seen (D-13, U-3) — a `<Show>`
           // mounting it over a landing included.
-          freshHook?.(el, !prevLane && flushTransaction);
+          freshHook?.(el, !mount && flushTransaction);
         }
         if (__DEV__) devTrackHeldPending(el);
       }
@@ -799,12 +808,7 @@ export function recompute(el: Computed<any>, create: boolean = false): void {
   // hold's commit (D-12) — but the hold does not wait for it: its only
   // readers are behind the fallback (`_hidden`; F-4, #3540). A boundary
   // re-armed by `on` owns what it collects outright (D-14): not held.
-  if (
-    create &&
-    lane === null &&
-    (joined || (prevLane && el._flags & REACTIVE_STAGED_READ)) &&
-    el._statusFlags & STATUS_PENDING
-  ) {
+  if (create && lane === null && joined && el._statusFlags & STATUS_PENDING) {
     const t = flushTransaction ?? passTx;
     if (t && !t._lane && freshHook?.(el) !== CAUGHT_REARMED) holdNode(el, t);
   }

packages/signals/tests/verdict-mount-render-effect-4166.test.ts

@@ -0,0 +1,100 @@
+/**
+ * #4166: #3851 for a nested render effect. An action holds `x = 1`;
+ * `<Show when={latest(x) > 0}>` mounts a `<Loading>` whose content creates
+ * a DOM render effect reading `x` (`<p>{x()}</p>`). The mount is mainline,
+ * so that effect's first read is held like a direct read: the boundary
+ * shows its fallback and the effect publishes nothing until the action
+ * lands. Before, the effect published `1` beside the committed `x = 0`.
+ */
+import { describe, expect, it } from "vitest";
+import {
+  action,
+  createLoadingBoundary,
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  flush,
+  latest,
+  untrack
+} from "../src/index.js";
+
+const settle = async () => {
+  for (let i = 0; i < 3; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+async function mountUnderVerdict(contentInMemo: boolean) {
+  const [x, setX] = createSignal(0);
+  let screenX: unknown;
+  let inner: unknown;
+  let slot: unknown;
+  createRoot(() => {
+    // <i>{x()}</i>
+    createRenderEffect(x, v => {
+      screenX = v;
+    });
+    // <Show when={latest(x) > 0}>
+    //   <Loading fallback="pending"><p>{x()}</p></Loading>
+    // </Show>
+    const p = () => {
+      createRenderEffect(x, v => {
+        inner = v;
+      });
+      return "p";
+    };
+    // A component that returns a memo: the render effect is created two
+    // creation passes below the verdict pass that mounted the boundary.
+    const content = contentInMemo ? () => createMemo(p)() : p;
+    const when = createMemo(() => latest(x) > 0);
+    const condition = createMemo(() => when());
+    const children = createMemo(() =>
+      condition() ? untrack(() => createLoadingBoundary(content, () => "pending")) : "closed"
+    );
+    createRenderEffect(
+      () => {
+        const c = children();
+        return typeof c === "function" ? c() : c;
+      },
+      v => {
+        slot = v;
+      }
+    );
+  });
+  flush();
+  const screens: unknown[] = [[screenX, slot, inner]];
+
+  let release!: () => void;
+  const done = action(function* () {
+    setX(1);
+    yield new Promise<void>(r => (release = r));
+  })();
+  await settle();
+  screens.push([screenX, slot, inner]);
+
+  release();
+  await done;
+  await settle();
+  screens.push([screenX, slot, inner]);
+  return screens;
+}
+
+describe("#4166: a verdict-mounted boundary holds a nested render effect's first read", () => {
+  it("the boundary shows its fallback while the action holds the value", async () => {
+    expect(await mountUnderVerdict(false)).toEqual([
+      [0, "closed", undefined],
+      [0, "pending", undefined],
+      [1, "p", 1]
+    ]);
+  });
+
+  it("same with the render effect inside a memo the content creates", async () => {
+    expect(await mountUnderVerdict(true)).toEqual([
+      [0, "closed", undefined],
+      [0, "pending", undefined],
+      [1, "p", 1]
+    ]);
+  });
+});

packages/web/test/show-latest-loading-render-effect-4166.spec.tsx

@@ -0,0 +1,55 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ *
+ * #4166: a `<Loading>` that `<Show when={latest(n) > 0}>` mounts during a
+ * held action holds its content's text binding like a direct read (#3851):
+ * the boundary shows its fallback beside the committed `n` until the action
+ * lands.
+ */
+import { expect, test } from "vitest";
+import { Loading, Show, action, createSignal, flush, latest } from "solid-js";
+import { render } from "../src/index.js";
+
+async function settle() {
+  for (let i = 0; i < 3; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+}
+
+test("a verdict-mounted Loading holds its content's binding until the action lands (#4166)", async () => {
+  const root = document.createElement("div");
+  const [n, setN] = createSignal(0);
+  let release!: () => void;
+  const save = action(function* () {
+    setN(1);
+    yield new Promise<void>(r => (release = r));
+  });
+
+  const dispose = render(
+    () => (
+      <>
+        <i>{n()}</i>
+        <Show when={latest(n) > 0}>
+          <Loading fallback="pending">
+            <p>{n()}</p>
+          </Loading>
+        </Show>
+      </>
+    ),
+    root
+  );
+  flush();
+  expect(root.innerHTML).toBe("<i>0</i>");
+
+  const done = save();
+  await settle();
+  expect(root.innerHTML).toBe("<i>0</i>pending");
+
+  release();
+  await done;
+  await settle();
+  expect(root.innerHTML).toBe("<i>1</i><p>1</p>");
+  dispose();
+});

scripts/size/floor-caps.json

@@ -1,23 +1,23 @@
 {
   "signals: core floor (createSignal/Memo/Effect/Root/flush)": {
     "cap": "7.39 KB",
-    "minified": 20021
+    "minified": 19991
   },
   "app: render + one signal (the simple-app floor)": {
     "cap": "9.70 KB",
-    "minified": 26911
+    "minified": 26881
   },
   "app: hydrating (no stores) with Show/For/Loading/Errored/lazy": {
-    "cap": "18.32 KB",
-    "minified": 53488
+    "cap": "18.36 KB",
+    "minified": 53454
   },
   "page: base server components (hydrating + dynamic + frames + sf reference)": {
     "cap": "33.20 KB",
-    "minified": 101444
+    "minified": 101429
   },
   "page: live server components (base + live/GET + action + isPending/latest)": {
-    "cap": "37.48 KB",
-    "minified": 114982
+    "cap": "37.53 KB",
+    "minified": 115031
   },
   "server: floor (getRequestEvent + isServer)": {
     "cap": "1.34 KB",