diff --git a/.changeset/chained-with-carries-all-args.md b/.changeset/chained-with-carries-all-args.md new file mode 100644 index 00000000..54f17992 --- /dev/null +++ b/.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. diff --git a/.changeset/server-rendered-with-uses-registered-action.md b/.changeset/server-rendered-with-uses-registered-action.md new file mode 100644 index 00000000..a49d738b --- /dev/null +++ b/.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). diff --git a/src/data/action.ts b/src/data/action.ts index d1c3c83c..489de8ce 100644 --- a/src/data/action.ts +++ b/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 | 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 (`/`) — not the server function convention; // nothing can run it, resubmit natively (submit() bypasses the delegated // handler) @@ -362,12 +386,15 @@ function toAction, 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( invoke, (uri.origin === "https://action" ? uri.origin : "") + uri.pathname + uri.search, - [...boundArgs, ...args], + bound, base, submitHooks, settledHooks diff --git a/test/data/action.spec.ts b/test/data/action.spec.ts index 58e3e640..86c52c76 100644 --- a/test/data/action.spec.ts +++ b/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();