fix(action): server-rendered and chained .with() urls

.changeset/chained-with-carries-all-args.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/router": patch
+---
+
+A chained `.with()` (`action.with(a).with(b)`) now puts every bound argument in its url (`?args=[a,b]`). Previously the url carried only the last call's arguments, so a server-rendered or no-JavaScript submission ran with arguments missing, and two chains ending in the same argument shared one url and overwrote each other's client registration.

.changeset/server-rendered-with-uses-registered-action.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/router": patch
+---
+
+A form rendered with `action.with(...args)` on the server now submits through the registered action on the client, so its `onSubmit` and `onSettled` hooks run with the bound arguments and `useSubmissions(action)` sees the outcome. Previously the rendered `?args` url missed the registry and fell back to a generic invocation (server actions) or native submission (client actions).

src/data/action.ts

@@ -113,7 +113,7 @@ export function handleFormAction(evt: SubmitEvent, router: RouterContext, action
   // stays a no-JS fallback.
   // Client-only actions (`https://action/`) are their module's JS by
   // definition, so a miss there falls through to native submission.
-  const handler = actions.get(actionRef) || (serverAction && createServerFormAction(actionRef));
+  const handler = findAction(actionRef) || (serverAction && createServerFormAction(actionRef));
   if (handler) {
     evt.preventDefault();
     const data = new FormData(form, evt.submitter);
@@ -124,6 +124,30 @@ export function handleFormAction(evt: SubmitEvent, router: RouterContext, action
   }
 }
 
+/**
+ * Looks a rendered action url up in the registry. A server-rendered
+ * `.with()` url is only registered if this client made the same binding
+ * itself; otherwise its base action (the url without `?args`) is rebound to
+ * the rendered arguments, so the submission runs through that action's
+ * submit and settled hooks and is recorded under its base.
+ */
+function findAction(url: string): Action<any, any> | undefined {
+  const handler = actions.get(url);
+  if (handler) return handler;
+  const query = url.indexOf("?");
+  if (query < 0) return undefined;
+  const base = actions.get(url.slice(0, query));
+  const args = new URLSearchParams(url.slice(query)).get("args");
+  if (!base || args === null) return undefined;
+  let bound: unknown;
+  try {
+    bound = JSON.parse(args);
+  } catch {
+    return undefined;
+  }
+  return Array.isArray(bound) ? base.with(...bound) : undefined;
+}
+
 /**
  * Synthesizes a router action for a server-rendered action url. The url
  * carries everything an invocation needs — the function id in the path
@@ -171,7 +195,7 @@ export function submitServerForm(
   form: HTMLFormElement,
   data: FormData
 ) {
-  const handler = actions.get(url) || createServerFormAction(url);
+  const handler = findAction(url) || createServerFormAction(url);
   // not an address (`<endpoint>/<id>`) — not the server function convention;
   // nothing can run it, resubmit natively (submit() bypasses the delegated
   // handler)
@@ -362,12 +386,15 @@ function toAction<T extends Array<any>, U, V = T>(
     this: InternalAction<[...A, ...B], U, V>,
     ...args: A
   ) {
+    const bound = [...boundArgs, ...args];
     const uri = new URL(url, mockBase);
-    uri.searchParams.set("args", hashKey(args));
+    // the server prepends `args` to the submitted arguments, so it must carry
+    // the whole binding, not just this call's part of a chain
+    uri.searchParams.set("args", hashKey(bound));
     return toAction<B, U, V>(
       invoke,
       (uri.origin === "https://action" ? uri.origin : "") + uri.pathname + uri.search,
-      [...boundArgs, ...args],
+      bound,
       base,
       submitHooks,
       settledHooks

test/data/action.spec.ts

@@ -139,6 +139,18 @@ describe("action", () => {
     expect(curriedAction.url).toMatch(/with-test\?args=/);
   });
 
+  test("a chained `.with` url carries every bound argument", () => {
+    const move = action(async (from: string, to: string, data: string) => data, "chained-with");
+    const fromA = move.with("a").with("x");
+    const fromB = move.with("b").with("x");
+
+    expect(new URL(fromA.url, "http://localhost").searchParams.get("args")).toBe('["a","x"]');
+    // same last argument, different binding: the urls (and registrations) must differ
+    expect(fromA.url).not.toBe(fromB.url);
+    expect(actions.get(fromA.url)).toBe(fromA);
+    expect(fromA.url).toBe(move.with("a", "x").url);
+  });
+
   // actions are invoked outside `createRoot` — as of Solid 2.0.0-beta.18 calling an
   // action inside an owned scope throws ACTION_CALLED_IN_OWNED_SCOPE in dev
   test("should execute action and create submission", async () => {
@@ -883,6 +895,53 @@ describe("generic server actions", () => {
     expect(fetchMock.mock.calls[0][0]).toBe("/_server/data/bound%230?args=%5B7%5D");
   });
 
+  test("a server-rendered `.with()` form runs through its registered base action", async () => {
+    // The server renders `toggle.with("a")` as the form's action. The client
+    // loaded `toggle`'s module, so `toggle` is registered, but it never called
+    // `.with("a")` itself, so the rendered url is not in the registry. The
+    // submission must still run as `toggle`: its submit hooks see the bound
+    // arguments, and `useSubmissions(toggle)` sees the outcome.
+    const serverFn = Object.assign(
+      vi.fn(async (_id: string, _form: unknown) => ({ ok: true })),
+      { url: "/_server/toggle%230" }
+    );
+    const toggle = action(serverFn);
+    const hook = vi.fn();
+    toggle.onSubmit(hook);
+    const ref = `/_server/toggle%230?args=${encodeURIComponent(JSON.stringify(["a"]))}`;
+
+    handleFormAction(createSubmitEvent(createServerForm(ref)), mockRouterContext, ACTION_BASE);
+
+    await vi.waitFor(() => expect(serverFn).toHaveBeenCalled());
+    expect(serverFn.mock.calls[0][0]).toBe("a");
+    expect(hook).toHaveBeenCalledWith("a", expect.anything());
+    expect(fetchMock).not.toHaveBeenCalled();
+    await vi.waitFor(() => expect(mockRouterContext.submissions[0]()).toHaveLength(1));
+    const [submission] = mockRouterContext.submissions[0]();
+    // recorded under the unbound action, which is what useSubmissions(toggle) matches
+    expect(submission.url).toBe(toggle.url);
+    expect(submission.input[0]).toBe("a");
+    expect(submission.result).toEqual({ ok: true });
+  });
+
+  test("a server-rendered `.with()` form on a client action runs through its base action", async () => {
+    const clientFn = vi.fn(async (_id: string, _form: unknown) => ({ ok: true }));
+    const toggle = action(clientFn, "toggle");
+    const hook = vi.fn();
+    toggle.onSubmit(hook);
+    const ref = `${toggle.url}?args=${encodeURIComponent(JSON.stringify(["a"]))}`;
+    const event = createSubmitEvent(createServerForm(ref));
+
+    handleFormAction(event, mockRouterContext, ACTION_BASE);
+
+    expect(event.preventDefault).toHaveBeenCalled();
+    await vi.waitFor(() => expect(clientFn).toHaveBeenCalled());
+    expect(clientFn.mock.calls[0][0]).toBe("a");
+    expect(hook).toHaveBeenCalledWith("a", expect.anything());
+    await vi.waitFor(() => expect(mockRouterContext.submissions[0]()).toHaveLength(1));
+    expect(mockRouterContext.submissions[0]()[0].url).toBe(toggle.url);
+  });
+
   test("a registered action takes precedence over synthesis", () => {
     const ref = "/_server/real%230";
     const mockActionFn = vi.fn();