fix(solid): a rejected fragment's fresh render claims what the server sent

.changeset/abandoned-records-compute-on-fresh-render.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/web": patch
+---
+
+A streamed boundary that fails on the server can leave nested reads still pending. The server abandons those reads, and their records used to settle as `undefined`, which the client's fresh render of the failed fragment would then adopt as the server's answer. The server now unregisters abandoned records, so those reads compute on the client. The fresh render also spends an id on each element's hydration key, as the server did, so its reads line up with the server's records by id.

.changeset/fresh-render-dynamic-portal.md

@@ -0,0 +1,6 @@
+---
+"solid-js": patch
+"@solidjs/web": patch
+---
+
+A boundary whose server render failed is rendered fresh on the client while adopting the server's records. Inside that render, a `<Dynamic>` tag now creates its element instead of throwing a hydration mismatch, and a `<Portal>` mounts. Hydration also completes when the adopted error has no client boundary to catch it.

.changeset/uncontained-failure-adopts-server-error.md

@@ -0,0 +1,5 @@
+---
+"solid-js": patch
+---
+
+Hydration never asks again for what the server sent, even when a streamed boundary's fragment rejects with no outcome (no `<Errored>` on the server could draw the failure). The client still renders that fragment fresh, so its graph is created, but every read in it now claims its serialized record: reads the server resolved take their values, and a read whose record is an error throws that error to the nearest client boundary, or is an uncaught error when there is none. Only reads with no record, such as those cut off by a truncated stream, compute. Recovery comes from a real input change or a boundary's `reset()`, as for any hydrated failure.

packages/solid/src/client/hydration.ts

@@ -54,8 +54,6 @@ import { IS_DEV, IS_OBSERVE } from "./core.js";
 // enableHydration().
 import { hydrateStoreLike } from "./store-hydration.js";
 
-declare const _$HY: any;
-
 type HydrationSsrFields = {
   /**
    * Defer the SSR stream flush until this primitive's first value is
@@ -590,8 +588,9 @@ export function syncThenable(value: any) {
 /**
  * A served pass: `write(x)` lands a served answer — the server's answer for a
  * node arriving (the async runtime writes it and flushes, synchronously),
- * or a rejected fragment's fresh render over the outcome the server drew
- * (the boundary's trigger) — and the flush round that carries it is
+ * or a boundary's fresh render (its trigger; `adopt`: hydrating, so what it
+ * creates adopts its server answer by id, claiming nothing) — and the
+ * flush round that carries it is
  * hydration's own pass over served answers (`_claimOwner`, that round). A
  * node that adopted its own server answer re-runs there because the
  * server's answer for something it reads arrived, not because a client
@@ -601,12 +600,27 @@ export function syncThenable(value: any) {
  * the flush, where a node it reaches computes. Runs from a settled promise,
  * outside any claim window.
  */
-const servedPass = (write: (x?: any) => void) => (x?: any) => {
+const servedPass = (write: (x?: any) => void, adopt?: boolean) => (x?: any) => {
   flush();
+  // Adopting, hydration is on but nothing claims: the render is a client
+  // render, so it opens no snapshot scope either (the `hydrating` setter,
+  // which would release one, is bypassed here). A scope left open would hold
+  // every write made in it — a client-only node's gate (a <Portal>) never
+  // opening.
+  _hydratingValue = _snapshotRootOwner = adopt as any;
   _claimOwner = ~clock;
-  write(x);
-  flush();
-  _claimOwner = null;
+  // An uncaught error (no client boundary for a served error) leaves the
+  // flush by throwing: the pass still ends, and hydration still completes
+  // — a boundary's fresh render is its resume, done all the same. (Outside
+  // a resume the check is a no-op: hydration is done, or boundaries are
+  // pending.)
+  try {
+    write(x);
+    flush();
+  } finally {
+    _claimOwner = _hydratingValue = _snapshotRootOwner = null as any;
+    checkHydrationComplete();
+  }
 };
 
 /**
@@ -2306,7 +2320,7 @@ function resumeBoundaryHydration(
   id: string,
   set: () => void,
   release: () => boolean,
-  shouldHydrate = true
+  shouldHydrate: boolean | 0 = true
 ) {
   // Read before release(): releasing removes the boundaryScopes entry.
   const scope = sharedConfig.boundaryScopes?.get(id);
@@ -2334,12 +2348,15 @@ function resumeBoundaryHydration(
         scope,
         o
       )
-    : // The client renders the boundary fresh (recover): no claim. Over the
-      // outcome the server drew for the failure (an `<Errored>` fallback the
-      // swap brought, listed in the reveal record, #4287) it is that
-      // failure's content run, as a hydrated `<Errored>`'s is: a served pass.
-      // A recover only runs while hydrating, so the page's `_$HY` exists.
-      (_$HY.v?.[id]?.[0] ? servedPass(set) : set)();
+    : // The client renders the boundary fresh (recover): no claim, a served
+      // pass. After a rejection (0: the server's, or the stream cut short)
+      // the render adopts what the server sent by id, as a hydrated
+      // `<Errored>`'s content run does: the failing read's error included,
+      // thrown to the nearest client boundary — the outcome the server drew
+      // for it, if any (#4287), or an uncaught error — instead of asking
+      // again. What the server never sent (a truncated ref is dropped from
+      // the registry) computes.
+      servedPass(set, shouldHydrate === 0)();
   flush();
   checkHydrationComplete();
 }
@@ -2355,7 +2372,7 @@ function resumeBoundaryHydration(
 function recover(
   id: string,
   registeredAt: number,
-  resume: (shouldHydrate?: boolean) => void
+  resume: (shouldHydrate?: boolean | 0) => void
 ): void {
   if (registeredAt < 0 || !OBSERVE!.records.observed("recovery")) {
     resume(false);
@@ -2373,7 +2390,7 @@ function recover(
 function initBoundaryResume(
   o: Owner,
   id: string
-): [trigger: () => void, resume: (shouldHydrate?: boolean) => void, release: () => boolean] {
+): [trigger: () => void, resume: (shouldHydrate?: boolean | 0) => void, release: () => boolean] {
   _pendingBoundaries++;
   (o as any)._hp = 1;
   // Capture the current root's registry/gather pair for this boundary's
@@ -2733,17 +2750,17 @@ function fragmentAbort(id: string): Promise<never> {
 
 function waitAndResume(
   p: any,
-  resume: (shouldHydrate?: boolean) => void,
+  resume: (shouldHydrate?: boolean | 0) => void,
   assetPromise?: Promise<void>,
-  hydrateRejected = true,
+  hydrateRejected: boolean | 0 = true,
   abort?: Promise<never>
 ) {
   // Settle data and assets independently: an asset error must not be written
   // into the data ref's rejected state, and a data rejection must still keep
   // its own hydrate semantics. (`p` may be an exotic thenable — coerce first.)
   // An abort promise (fragment truncation) races the data: its rejection
   // flows through the same rejected-state write as a server-sent rejection.
-  const data: Promise<boolean> = (
+  const data: Promise<boolean | 0> = (
     abort ? Promise.race([Promise.resolve(p), abort]) : Promise.resolve(p)
   ).then(
     () => {
@@ -2786,7 +2803,7 @@ function reportAssetFailure(err: any) {
 
 function scheduleResumeAfterAssets(
   id: string,
-  resume: (shouldHydrate?: boolean) => void,
+  resume: (shouldHydrate?: boolean | 0) => void,
   assetPromise?: Promise<void>
 ): boolean {
   sharedConfig.gather?.(id);
@@ -2973,7 +2990,7 @@ function hydratedCreateLoadingBoundary<T, U>(
           // branch and `recover` with it.
           waitAndResume(
             1,
-            IS_OBSERVE ? () => recover(id, registeredAt, resume) : () => resume(false),
+            IS_OBSERVE ? () => recover(id, registeredAt, () => resume(0)) : () => resume(0),
             assetPromise
           );
           return undefined;
@@ -3016,9 +3033,13 @@ function hydratedCreateLoadingBoundary<T, U>(
               // (#4287) — beside the range the fresh render takes: clear
               // them first.
               (revealed || hy.v?.[id]?.forEach?.((n: ChildNode) => n.remove()),
-              IS_OBSERVE ? recover(id, registeredAt, resume) : resume(false)),
+              IS_OBSERVE
+                ? recover(id, registeredAt, () => resume(shouldHydrate))
+                : resume(shouldHydrate)),
         assetPromise,
-        false,
+        // A rejection (the server's, or the stream cut short) resumes with 0:
+        // the fresh render adopts what the server sent (resumeBoundaryHydration).
+        0,
         fragmentAbort(id)
       );
       return revealed ? undefined : fallback();
@@ -3057,7 +3078,10 @@ export function NoHydration(props: { children: SolidElement }): SolidElement {
   const o = createOwner();
   return runWithOwner(o, () => {
     setContext(NoHydrateContext, true);
-    if (sharedConfig.hydrating) return undefined as unknown as SolidElement;
+    // Claiming, not the bare flag: a rejected fragment's fresh render
+    // (adopting by id, claiming nothing) renders the children like any
+    // client render.
+    if (isHydrating()) return undefined as unknown as SolidElement;
     return props.children;
   }) as unknown as SolidElement;
 }

packages/solid/test/client-hydration.spec.ts

@@ -4697,7 +4697,7 @@ describe("Loading boundary: fragment registration channel (_fr)", () => {
     expect(memo()).toBe(99);
   });
 
-  test("rejected _fr resumes without hydrating serialized children", async () => {
+  test("rejected _fr resumes fresh, adopting serialized children", async () => {
     let rejectFr!: (e: any) => void;
     const frPromise: any = new Promise<boolean>((_, rej) => (rejectFr = rej));
     frPromise.s = 0;
@@ -4739,7 +4739,8 @@ describe("Loading boundary: fragment registration channel (_fr)", () => {
     await new Promise<void>(r => setTimeout(r, 50));
     flush();
 
-    expect(memo()).toBe(0);
+    // Fresh DOM, but the server's answer is used: hydration never refetches.
+    expect(memo()).toBe(7);
   });
 
   test("_fr with assets waits for both fragment and assets", async () => {

packages/solid/test/client-recovery-record.spec.ts

@@ -119,8 +119,8 @@ describe.skipIf(OBSERVE === undefined)("recovery record", () => {
     rejectFr(new Error("stream error"));
     await new Promise<void>(r => setTimeout(r, 20));
     flush();
-    // Fresh client DOM: the serialized 7 was never adopted.
-    expect(b.memo()).toBe(0);
+    // Fresh client DOM, the serialized 7 adopted: hydration never refetches.
+    expect(b.memo()).toBe(7);
     expect(records).toEqual([{ id: "t0", at: T0, waitedMs: 30, renderMs: RENDER_MS }]);
   });
 
@@ -133,7 +133,7 @@ describe.skipIf(OBSERVE === undefined)("recovery record", () => {
     const b = boundary();
     await new Promise<void>(r => setTimeout(r, 10));
     flush();
-    expect(b.memo()).toBe(0);
+    expect(b.memo()).toBe(7);
     // The rejection had already arrived when the boundary registered: the
     // clock did not move between registration and the recovery, so the
     // person saw no fallback — `waitedMs` is exactly 0.
@@ -163,7 +163,7 @@ describe.skipIf(OBSERVE === undefined)("recovery record", () => {
     const b = boundary();
     await new Promise<void>(r => setTimeout(r, 10));
     flush();
-    expect(b.memo()).toBe(0);
+    expect(b.memo()).toBe(7);
     // The runtime never read the clock: `registeredAt` is -1 with no listener.
     expect(performance.now).not.toHaveBeenCalled();
   });

packages/web/src/client.ts

@@ -2234,10 +2234,12 @@ function gateHeadResource(props) {
       // errored) sheet must acquire synchronously — cached sheets add zero
       // wait, adopted server-emitted sheets never stall — and even a
       // settled promise costs a microtask through the async machinery.
-      // Hydrating content is already visible; the server gated its reveal.
+      // Claimed content is already visible; the server gated its reveal. A
+      // fresh render under hydration (a rejected fragment's) gates like any
+      // client render.
       if (
         gateable &&
-        !sharedConfig.hydrating &&
+        !isHydrating() &&
         entry.loadState === "pending" &&
         typeof waitAsset === "function"
       )
@@ -2580,20 +2582,22 @@ export function hydrate(code, element, options = {}) {
 export function getNextElement(template?: () => Element): Element;
 
 export function getNextElement(template) {
-  let node,
-    key,
-    hydrating = isHydrating();
-  if (!hydrating || !(node = sharedConfig.registry.get((key = getHydrationKey())))) {
+  // The key is consumed whenever hydration is on, claiming or not: a
+  // rejected fragment's fresh render adopts its server answers by id, and
+  // the server spent one on this element, so ids after it line up.
+  let key = sharedConfig.hydrating && getHydrationKey(),
+    node = isHydrating() && sharedConfig.registry.get(key);
+  if (!node) {
     if (!template) {
       throw new Error(`Hydration Mismatch. Unable to find DOM nodes for hydration key: ${key}`);
     }
     // A key miss during the hydration walk is a real mismatch: fresh renders
-    // (portals, rejected fragments, post-hydration content) all run with the
-    // hydrating flag off, so there is no legitimate way to get here. The
+    // (portals, rejected fragments, post-hydration content) all run outside
+    // the claim, so there is no legitimate way to get here. The
     // detached element returned below keeps the render alive but never lands
     // in the document — without a report that reads as a silently frozen
     // page (solidjs/solid#3000).
-    if ("_SOLID_DEV_" && hydrating) {
+    if ("_SOLID_DEV_" && isHydrating()) {
       hydrationKeyMisses++;
       console.warn(
         `Hydration key miss for "${key}": no server-rendered element carries this key` +

packages/web/src/index.ts

@@ -325,14 +325,16 @@ function staticElement(tag: string, props: any): JSX.Element {
   // CREATED (customized built-in / namespace), so they are read once here,
   // untracked — the DOM can't change either after creation — and then flow
   // through spread() like any attribute. Hydration claims the
-  // parser-namespaced node, so neither applies.
-  const el = hydrating
-    ? getNextElement()
-    : createElement(
-        tag,
-        untrack(() => props.is),
-        untrack(() => props.xmlns)
-      );
+  // parser-namespaced node, so neither applies — when it claims: a fresh
+  // render under hydration (a rejected fragment's, adopting by id) spends
+  // the key and creates, like a compiled template.
+  const create = () =>
+    createElement(
+      tag,
+      untrack(() => props.is),
+      untrack(() => props.xmlns)
+    );
+  const el = hydrating ? getNextElement(create) : create();
   spread(el, props);
   // Compiled JSX emits runHydrationEvents() after an element that carries
   // event handlers. Handlers bound through spread() here need the same call,

packages/web/src/server.ts

@@ -2662,9 +2662,10 @@ export function renderToStream(code, options = {}) {
   // resume loops may be parked on promises that never settle), and pending
   // serialized values under the errored key would gate seroval's onDone the
   // same way. Settle both. Descendant `_fr` stubs resolve clean (not
-  // rejected) and abandoned data ids resolve undefined: the client re-renders
-  // the errored region fresh off the OUTER fragment's rejection, so nothing
-  // consumes these — a rejection would only raise unhandled-rejection noise.
+  // rejected) and abandoned data ids resolve undefined, and are unregistered
+  // on the client: it re-renders the errored region fresh off the OUTER
+  // fragment's rejection, so nothing consumes these — a rejection would only
+  // raise unhandled-rejection noise.
   //
   // A finding (`SSR_SUBTREE_ABANDONED`) when the discard took work with it —
   // nested fragments still rendering, serialized values still pending: the
@@ -2686,13 +2687,24 @@ export function renderToStream(code, options = {}) {
         fragments++;
       }
     }
+    const unregister = [];
     for (const [id, settle] of pendingSerialized) {
       if (id.startsWith(key)) {
         pendingSerialized.delete(id);
         settle();
         serialized++;
+        // Not yet on the wire: never registered. Otherwise the client drops
+        // it (below).
+        if (!stubBatch?.delete(id)) unregister.push(`delete _$HY.r[${JSON.stringify(id)}]`);
       }
     }
+    // The settle above only releases seroval: `undefined` is not the
+    // server's answer. The client renders this subtree fresh, adopting what
+    // the server sent by id, so an abandoned record must not be there to
+    // adopt: unregister it (a document task, ahead of the `_fr` rejection
+    // that triggers the fresh render) and the read computes. A frame sink
+    // has no reader for these records.
+    if (unregister.length && !options.sink) pushTask(unregister.join(";"));
     if ("_SOLID_OBSERVE_" && (fragments || serialized))
       emitFinding(
         {

packages/web/test/harness/__artifacts__/abandoned-sibling-computes.json

@@ -0,0 +1,5 @@
+{
+  "name": "abandoned-sibling-computes",
+  "shell": "<div _hk=0><!--$--><template id=\"pl-1\"></template><i _hk=10>loading</i><!--pl-1--><!--/--><span>tail</span></div><script>(self.$R=self.$R||{})[\"\"]=[];_$HY.r[\"1006_fr\"]=$R[0]=($R[1]=($R[2]=() => {\n\tconst resolver = {\n\t\tp: 0,\n\t\ts: 0,\n\t\tf: 0\n\t};\n\tresolver.p = new Promise((resolve, reject) => {\n\t\tresolver.s = resolve;\n\t\tresolver.f = reject;\n\t});\n\treturn resolver;\n})()).p;_$HY.r[\"1_fr\"]=$R[3]=($R[4]=$R[2]()).p;_$HY.r[\"1000\"]=$R[5]=($R[6]=$R[2]()).p;_$HY.r[\"1003\"]=$R[7]=($R[8]=$R[2]()).p;_$HY.r[\"1006000\"]=$R[9]=($R[10]=$R[2]()).p;</script>",
+  "rest": "<script>($R[11]=(resolver, data) => {\n\tresolver.s(data);\n\tresolver.p.s = 1;\n\tresolver.p.v = data;\n})($R[8],\"d\");</script><template id=\"1\" e></template><script>$R[11]($R[6],\"a\");delete _$HY.r[\"1006000\"];$df(\"1\");function $df(e){return _$HY.f?_$HY.f(e):$dfr(e)}function $dfr(e,n,o,t){if(!(n=document.getElementById(e)))return 0;if(!(o=document.getElementById(\"pl-\"+e)))return(_$HY.dq=_$HY.dq||{})[e]=1,0;for(;o&&(8!==o.nodeType||o.nodeValue!==\"pl-\"+e);)t=o.nextSibling,o.remove(),o=t;t=o.parentNode,(_$HY.v=_$HY.v||{})[e]=n.hasAttribute(\"e\")?[...n.content.childNodes]:1,o.replaceWith(n.content),n.remove(),_$HY.fe(e,t),_$HY.hp&&_$HY.hp[e]&&($dh(_$HY.hp[e]),delete _$HY.hp[e]),$dfd();return 1}function $dfl(e,o,n){if(!(o=document.getElementById(\"pl-\"+e)))return(_$HY.dlq=_$HY.dlq||{})[e]=1,0;if(o._$fl)return 1;for(n=o.nextSibling;n;){if(8===n.nodeType&&n.nodeValue===\"pl-\"+e){o.parentNode&&o.parentNode.insertBefore(o.content.cloneNode(!0),n),o._$fl=1,$dfd();return 1}n=n.nextSibling}return 0}function $dflj(e,i){for(i=0;i<e.length;i++)$dfl(e[i])}function $dfd(e,i){if(e=_$HY.dq){_$HY.dq=0;for(i in e)$df(i)}if(e=_$HY.dlq){_$HY.dlq=0;for(i in e)$dfl(i)}}function $dfs(e,c,d){(_$HY.sc=_$HY.sc||{})[e]=c,d&&((_$HY.sd=_$HY.sd||{})[e]=1)}function $dfg(e,g,i,k){if(!(g=_$HY.sg&&_$HY.sg[e]))return;for(i=0;i<g.length;i++)if(_$HY.sc&&_$HY.sc[g[i]]>0)return;for(i=0;i<g.length;i++)k=g[i],delete _$HY.sg[k],$df(k)}function $dfc(e){if(--_$HY.sc[e]<=0){delete _$HY.sc[e],_$HY.sg&&_$HY.sg[e]?$dfg(e):!(_$HY.sd&&_$HY.sd[e])&&$df(e);_$HY.sd&&delete _$HY.sd[e]}}function $dfj(e,i,n){for(i=0;i<e.length;i++)if(_$HY.sc&&_$HY.sc[e[i]]>0){for(n=0;n<e.length;n++)(_$HY.sg=_$HY.sg||{})[e[n]]=e;return}for(i=0;i<e.length;i++)$df(e[i])}function $dfe(e,t,k){(t=e.target)&&t.getAttribute&&!t._$dc&&(k=t.getAttribute(\"data-dfc\"))&&(t._$dc=1,$dfc(k))}document.addEventListener(\"load\",$dfe,!0);document.addEventListener(\"error\",$dfe,!0);</script><script>$R[11]($R[10],void 0);$R[11]($R[1],!0);($R[13]=(resolver, data) => {\n\tresolver.f(data);\n\tresolver.p.s = 2;\n\tresolver.p.v = data;\n})($R[4],$R[12]=new Error(\"server render\"));</script>"
+}