Skip to content

fix: include all query params in WebSocket connection #5507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 1, 2025
Merged

fix: include all query params in WebSocket connection #5507

merged 2 commits into from
Jul 1, 2025

Conversation

@harterrt-shopify
Copy link
Contributor

@harterrt-shopify harterrt-shopify commented Jul 1, 2025 •
edited
Loading

Fix: Include all query params in WebSocket connection

Summary

  • Fixed an issue where mo.query_params() was only returning file query parameters
  • The frontend now passes all query parameters from the browser URL to the WebSocket connection
  • Previously, only special parameters like session_id and file were being passed through

What this PR does

When visiting a marimo notebook with multiple query parameters like:

http://localhost:2718/?file=test.py&search=test&other=last

Previously, mo.query_params() would only return:

{"file": "test.py"}

Now it correctly returns all parameters:

{
  "file": "test.py",
  "search": "test", 
  "other": "last"
}

How to test

  1. Create a simple test notebook:
import marimo as mo

@app.cell
def _(mo):
    params = mo.query_params()
    params
  1. Run the notebook and open with query parameters:
http://localhost:2718/?file=test.py&search=test&other=last
  1. Verify that all three parameters are displayed

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable). - this is a bugfix.
  • I have added tests for the changes made.
  • I have run the code and verified that it works as expected.

📜 Reviewers

@mscolnick - this is a bugfix for the frontend.

@harterrt-shopify
fix: include all query params in WebSocket connection
6d1e0a2 
The frontend was only passing the session_id query parameter to the WebSocket
connection, ignoring all other query parameters from the current page URL.

This fix modifies getWsURL() and getWsSyncURL() to merge the current page's
query parameters with the base URL parameters before establishing the
WebSocket connection.

Fixes the issue where mo.query_params() would only return the first parameter
when multiple query parameters were present in the URL.
@vercel
Copy link

vercel bot commented Jul 1, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ❌ Failed (Inspect) Jul 1, 2025 5:23pm

@harterrt-shopify harterrt-shopify marked this pull request as ready for review July 1, 2025 17:28
@harterrt-shopify
Copy link
Contributor Author

harterrt-shopify commented Jul 1, 2025

Not sure why the vercel test is failing, and I don't have access to investigate further. Seems like this is failing for other PRs as well, so I suspect it's upstream of this change.

akshayka
akshayka approved these changes Jul 1, 2025
View reviewed changes
Copy link
Contributor

@akshayka akshayka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@akshayka akshayka added bash-focus Area to focus on during release bug bash bug Something isn't working labels Jul 1, 2025
@akshayka akshayka merged commit e431bd4 into marimo-team:main Jul 1, 2025
18 of 20 checks passed
@github-actions
Copy link

github-actions bot commented Jul 1, 2025

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.14.10-dev13

akshayka added a commit that referenced this pull request Jul 3, 2025
@akshayka @manzt
chore: script to generate bash focus PRs (#5520)
cb6bce4 
Run

`uv run scripts/generate_bash_focus.py <since-tag>`

to print a list of all PRs labeled with `bash-focus`.

Example output:

```
Using latest tag: 0.14.9
Fetching PRs with label 'bash-focus' since 0.14.9...

Found 4 PR(s) with label 'bash-focus':

#5515: fix: reuse ports that are available but recently closed
  Author: @akshayka
  Link: #5515
  Merged: 2025-07-02T15:52:50Z

#5507: fix: include all query params in WebSocket connection
  Author: @harterrt-shopify
  Link: #5507
  Merged: 2025-07-01T17:41:15Z

#5491: Ensure cell action tooltip appears above sidebar panel
  Author: @manzt
  Link: #5491
  Merged: 2025-06-30T18:09:19Z

#5474: register UIElement on change if it is missing from the UIElementRegistry
  Author: @buckley-w-david
  Link: #5474
  Merged: 2025-06-30T15:56:00Z
  ```

---------

Co-authored-by: Trevor Manz <trevor.j.manz@gmail.com>
sebbeutler pushed a commit to sebbeutler/marimo that referenced this pull request Jul 7, 2025
@harterrt-shopify @pre-commit-ci
fix: include all query params in WebSocket connection (marimo-team#5507)
3bbeaee 
# Fix: Include all query params in WebSocket connection

## Summary
- Fixed an issue where `mo.query_params()` was only returning `file`
query parameters
- The frontend now passes all query parameters from the browser URL to
the WebSocket connection
- Previously, only special parameters like `session_id` and `file` were
being passed through

## What this PR does

When visiting a marimo notebook with multiple query parameters like:
```
http://localhost:2718/?file=test.py&search=test&other=last
```

Previously, `mo.query_params()` would only return:
```python
{"file": "test.py"}
```

Now it correctly returns all parameters:
```python
{
  "file": "test.py",
  "search": "test", 
  "other": "last"
}
```

## How to test

1. Create a simple test notebook:
```python
import marimo as mo

@app.cell
def _(mo):
    params = mo.query_params()
    params
```

2. Run the notebook and open with query parameters:
```
http://localhost:2718/?file=test.py&search=test&other=last
```

3. Verify that all three parameters are displayed


## 📋 Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [x] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable). - this is a bugfix.
- [x] I have added tests for the changes made.
- [x] I have run the code and verified that it works as expected.

## 📜 Reviewers

@mscolnick - this is a bugfix for the frontend.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
sebbeutler pushed a commit to sebbeutler/marimo that referenced this pull request Jul 7, 2025
@akshayka @manzt
chore: script to generate bash focus PRs (marimo-team#5520)
6d4e683 
Run

`uv run scripts/generate_bash_focus.py <since-tag>`

to print a list of all PRs labeled with `bash-focus`.

Example output:

```
Using latest tag: 0.14.9
Fetching PRs with label 'bash-focus' since 0.14.9...

Found 4 PR(s) with label 'bash-focus':

marimo-team#5515: fix: reuse ports that are available but recently closed
  Author: @akshayka
  Link: marimo-team#5515
  Merged: 2025-07-02T15:52:50Z

marimo-team#5507: fix: include all query params in WebSocket connection
  Author: @harterrt-shopify
  Link: marimo-team#5507
  Merged: 2025-07-01T17:41:15Z

marimo-team#5491: Ensure cell action tooltip appears above sidebar panel
  Author: @manzt
  Link: marimo-team#5491
  Merged: 2025-06-30T18:09:19Z

marimo-team#5474: register UIElement on change if it is missing from the UIElementRegistry
  Author: @buckley-w-david
  Link: marimo-team#5474
  Merged: 2025-06-30T15:56:00Z
  ```

---------

Co-authored-by: Trevor Manz <trevor.j.manz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akshayka akshayka akshayka approved these changes

Assignees

No one assigned

Labels

bash-focus Area to focus on during release bug bash bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@harterrt-shopify @akshayka