fix(serverRouteComponent): hand a bare GET preload's call to the render

.changeset/server-route-get-preload-handoff.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/router": patch
+---
+
+`serverRouteComponent` with a bare `GET(fn)` source sends one request per link visit instead of two (#677). The router hands a preload's call to the next render with the same args, so the route mounts the result the hover or focus preload already started. The hand-off happens once, within the same 5 s window `query()` gives a preload. A later visit calls fresh, a failed preload makes the render retry, and concurrent preloads of the same args share one call. `query()` and `liveQuery()` sources are unchanged.

src/serverRouteComponent.ts

@@ -36,9 +36,10 @@
 // A server component that takes other client positions — handlers, refs,
 // named slots — has a client half, and that half is a client component; the
 // helper does not pretend otherwise. Write the wrapper for that route.
-import { dynamicComponent } from "@solidjs/web";
+import { dynamicComponent, getServerFunctionMetadata, isServer } from "@solidjs/web";
 import type { JSX } from "@solidjs/web";
-import { createComponent, DEV } from "solid-js";
+import { createComponent, DEV, untrack } from "solid-js";
+import { getIntent } from "./routing.js";
 import type { Component } from "solid-js";
 import { SERVER_ROUTE, type BrandedRouteComponent } from "./serverRouteShared.js";
 import type {
@@ -72,7 +73,7 @@ export function serverRouteComponent<P extends Params | TypedRouteConfig = Param
   // once `query()` has typed it; that is the same call, so accept it too.
   source: ServerRouteFunction<P, S> | ((...args: never[]) => ReturnType<ServerRouteFunction<P, S>>)
 ): Component<RouteSectionProps<unknown, ServerRouteParams<P>>> {
-  const call = source as ServerRouteFunction<any, any>;
+  const call = handOffPreloads(source as ServerRouteFunction<any, any>);
 
   // The value is a component only so it fits the `component` field; the
   // router core never calls it — it mounts through the brand, with THIS
@@ -106,3 +107,38 @@ export function serverRouteComponent<P extends Params | TypedRouteConfig = Param
   route[SERVER_ROUTE] = { call, render };
   return route as Component<RouteSectionProps<unknown, ServerRouteParams<P>>>;
 }
+
+// How long a preload's call stays claimable by the render it warmed — the
+// same window `query()` gives a preloaded entry.
+const PRELOAD_TIMEOUT = 5000;
+
+// A bare `GET(fn)` source has no cache of its own, so a link preload and the
+// render it warms would each send the request (#677). Hand the preload's
+// call to the next render with the same args, once: the render mounts the
+// result the preload already started, and a later render calls fresh, as a
+// bare GET always does. Concurrent preloads of the same args share one call.
+// `query()`/`liveQuery()` sources and plain functions keep their own path.
+function handOffPreloads(source: ServerRouteFunction<any, any>): ServerRouteFunction<any, any> {
+  if (isServer || getServerFunctionMetadata(source)?.method !== "GET") return source;
+  const preloads = new Map<string, [time: number, result: ReturnType<typeof source>]>();
+  return args => {
+    const now = Date.now();
+    for (const [k, [time]] of preloads) now - time >= PRELOAD_TIMEOUT && preloads.delete(k);
+    const key = JSON.stringify(args);
+    const entry = preloads.get(key);
+    if (untrack(getIntent) === "preload") {
+      if (entry) return entry[1];
+      const result = source(args);
+      preloads.set(key, [now, result]);
+      // A failed preload is not handed on: the render retries.
+      const drop = () => preloads.get(key)?.[1] === result && preloads.delete(key);
+      result && typeof (result as any).then === "function" && (result as any).then(undefined, drop);
+      return result;
+    }
+    if (entry) {
+      preloads.delete(key);
+      return entry[1];
+    }
+    return source(args);
+  };
+}

test/server-routes.spec.tsx

@@ -30,10 +30,13 @@ const SERVER_FUNCTION_METADATA = Symbol.for("solid.ServerFunctionMetadata");
 
 let nextId = 0;
 /** Brand `fn` as a server function reference (what compiled `"use server"` output produces). */
-function serverFunction<F extends (...args: any[]) => any>(fn: F): F & { id: string } {
+function serverFunction<F extends (...args: any[]) => any>(
+  fn: F,
+  meta: Record<string, unknown> = {}
+): F & { id: string } {
   const branded = fn as F & { id: string };
   branded.id = "srv#" + nextId++;
-  (branded as any)[SERVER_FUNCTION_METADATA] = {};
+  (branded as any)[SERVER_FUNCTION_METADATA] = meta;
   return branded;
 }
 
@@ -290,6 +293,109 @@ describe("server component routes", () => {
     cleanup();
   });
 
+  describe("a bare GET source", () => {
+    function setup(fn: (args: ServerRouteArgs<{ id: string }>) => unknown) {
+      let navigate!: Navigator;
+      let preload!: ReturnType<typeof usePreloadRoute>;
+      const Router = createRouter({
+        routes: [
+          defineRoute({
+            path: "/stories/:id",
+            component: serverRouteComponent(serverFunction(fn, { method: "GET" }) as any)
+          }),
+          defineRoute({
+            path: "/",
+            component: () => {
+              navigate = useNavigate();
+              preload = usePreloadRoute();
+              return <p>home</p>;
+            }
+          })
+        ],
+        history: memoryHistory("/")
+      });
+      const mounted = mount(() => (
+        <Router>{props => <Loading fallback="loading">{props.children}</Loading>}</Router>
+      ));
+      return {
+        ...mounted,
+        navigate: (to: string) => navigate(to),
+        preload: (to: string) => preload(to, { preloadData: true })
+      };
+    }
+
+    test("the render mounts the preload's call instead of sending its own (#677)", async () => {
+      const fn = vi.fn(async ({ params }: ServerRouteArgs<{ id: string }>) => {
+        await new Promise(r => setTimeout(r, 10));
+        return () => <article data-story={params.id} />;
+      });
+      const app = setup(fn);
+      await settle();
+      app.preload("/stories/5");
+      app.preload("/stories/5");
+      expect(fn).toHaveBeenCalledTimes(1);
+
+      app.navigate("/stories/5");
+      await settle(30);
+      expect(app.div.querySelector("[data-story]")!.getAttribute("data-story")).toBe("5");
+      expect(fn).toHaveBeenCalledTimes(1);
+      app.cleanup();
+    });
+
+    test("a preload is handed on once: a later visit calls fresh", async () => {
+      const fn = vi.fn(async ({ params }: ServerRouteArgs<{ id: string }>) => {
+        return () => <article data-story={params.id} />;
+      });
+      const app = setup(fn);
+      await settle();
+      app.preload("/stories/5");
+      app.navigate("/stories/5");
+      await settle(20);
+      expect(fn).toHaveBeenCalledTimes(1);
+
+      app.navigate("/");
+      await settle(20);
+      app.navigate("/stories/5");
+      await settle(20);
+      expect(fn).toHaveBeenCalledTimes(2);
+      expect(app.div.querySelector("[data-story]")!.getAttribute("data-story")).toBe("5");
+      app.cleanup();
+    });
+
+    test("a failed preload is not handed on: the render retries", async () => {
+      let fail = true;
+      const fn = vi.fn(async ({ params }: ServerRouteArgs<{ id: string }>) => {
+        if (fail) throw new Error("upstream down");
+        return () => <article data-story={params.id} />;
+      });
+      const app = setup(fn);
+      await settle();
+      app.preload("/stories/5");
+      await settle();
+      fail = false;
+      app.navigate("/stories/5");
+      await settle(20);
+      expect(fn).toHaveBeenCalledTimes(2);
+      expect(app.div.querySelector("[data-story]")!.getAttribute("data-story")).toBe("5");
+      app.cleanup();
+    });
+
+    test("a preload for other args is not handed to this render", async () => {
+      const fn = vi.fn(async ({ params }: ServerRouteArgs<{ id: string }>) => {
+        return () => <article data-story={params.id} />;
+      });
+      const app = setup(fn);
+      await settle();
+      app.preload("/stories/4");
+      app.navigate("/stories/5");
+      await settle(20);
+      expect(fn).toHaveBeenCalledTimes(2);
+      expect(fn.mock.calls[1][0]).toEqual({ params: { id: "5" } });
+      expect(app.div.querySelector("[data-story]")!.getAttribute("data-story")).toBe("5");
+      app.cleanup();
+    });
+  });
+
   test("the key is the app's: `revalidate(key)` refetches the route", async () => {
     let version = 0;
     const fn = vi.fn(async ({ params }: ServerRouteArgs<{ id: string }>) => {