Skip to content

Commit 86a2809

Browse files
akshaykapre-commit-ci[bot]
authored andcommitted
fix: query parameters in ws (marimo-team#5381)
This fixes a regression that caused `mo.query_params()` to stop working. We need to send all query parameters to the backend, not just known ones, as part of the websocket URL. Tested by playing with the query parameters smoke test (`_marimo/smoke_tests`). Fixes marimo-team#5376. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 4c4628c commit 86a2809

File tree

2 files changed

+63
-9
lines changed

2 files changed

+63
-9
lines changed

frontend/src/core/runtime/__tests__/runtime.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,50 @@ describe("RuntimeManager", () => {
271271
expect(aiUrl.pathname).toBe("/nested/path/api/ai/completion");
272272
});
273273

274+
it("should preserve all window-level query parameters in URLs", () => {
275+
// Mock window.location.search with custom query parameters
276+
const originalLocation = window.location;
277+
Object.defineProperty(window, "location", {
278+
value: {
279+
...originalLocation,
280+
search: "?custom_param=value123&user_token=abc&theme=dark",
281+
},
282+
writable: true,
283+
});
284+
285+
const runtime = new RuntimeManager({
286+
url: "https://example.com/path?base_param=existing",
287+
});
288+
289+
const wsUrl = runtime.getWsURL("test" as SessionId);
290+
const httpUrl = runtime.formatHttpURL(
291+
"api/test",
292+
new URLSearchParams(),
293+
false,
294+
);
295+
296+
// Should preserve base URL query params
297+
expect(wsUrl.searchParams.get("base_param")).toBe("existing");
298+
expect(httpUrl.searchParams.get("base_param")).toBe("existing");
299+
300+
// Should preserve all window-level query params (including custom ones)
301+
expect(wsUrl.searchParams.get("custom_param")).toBe("value123");
302+
expect(wsUrl.searchParams.get("user_token")).toBe("abc");
303+
expect(wsUrl.searchParams.get("theme")).toBe("dark");
304+
expect(httpUrl.searchParams.get("custom_param")).toBe("value123");
305+
expect(httpUrl.searchParams.get("user_token")).toBe("abc");
306+
expect(httpUrl.searchParams.get("theme")).toBe("dark");
307+
308+
// Should also include session_id for WebSocket URLs
309+
expect(wsUrl.searchParams.get("session_id")).toBe("test");
310+
311+
// Restore original location
312+
Object.defineProperty(window, "location", {
313+
value: originalLocation,
314+
writable: true,
315+
});
316+
});
317+
274318
it("should handle URLs with query parameters and fragments", () => {
275319
const runtime = new RuntimeManager({
276320
url: "https://example.com/path?existing=param#fragment",

frontend/src/core/runtime/runtime.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ export class RuntimeManager {
3838
/**
3939
* The base URL of the runtime.
4040
*/
41-
formatHttpURL(path?: string, searchParams?: URLSearchParams): URL {
41+
formatHttpURL(
42+
path?: string,
43+
searchParams?: URLSearchParams,
44+
restrictToKnownQueryParams: boolean = true,
45+
): URL {
4246
if (!path) {
4347
path = "";
4448
}
@@ -52,14 +56,14 @@ export class RuntimeManager {
5256
}
5357
}
5458

55-
// Move over window level parameters to the WebSocket URL
56-
// if they are "known" query params.
57-
for (const lookup in KnownQueryParams) {
58-
const key = KnownQueryParams[lookup as keyof typeof KnownQueryParams];
59-
const value = currentParams.get(key);
60-
if (value !== null) {
61-
baseUrl.searchParams.set(key, value);
59+
for (const [key, value] of currentParams.entries()) {
60+
if (
61+
restrictToKnownQueryParams &&
62+
!Object.values(KnownQueryParams).includes(key)
63+
) {
64+
continue;
6265
}
66+
baseUrl.searchParams.set(key, value);
6367
}
6468

6569
const cleanPath = baseUrl.pathname.replace(/\/$/, "");
@@ -69,7 +73,13 @@ export class RuntimeManager {
6973
}
7074

7175
formatWsURL(path: string, searchParams?: URLSearchParams): URL {
72-
const url = this.formatHttpURL(path, searchParams);
76+
// We don't restrict to known query parameters, since mo.query_params()
77+
// can accept arbitrary parameters.
78+
const url = this.formatHttpURL(
79+
path,
80+
searchParams,
81+
/* restrictToKnownQueryParams =*/ false,
82+
);
7383
return asWsUrl(url.toString());
7484
}
7585

0 commit comments

Comments
 (0)