Skip to content

Conversation

@akshayka
Copy link
Contributor

This PR fixes marimo development preview for previewing static HTML using local assets. Previously the generated HTML didn't include outputs (ie the notebook wasn't run), and also got stuck on attempting to connect to the frontend ("Connecting..." spinner)

  • actually generate the notebook session and outputs
  • for static notebooks, initialize the connection as open

@vercel
Copy link

vercel bot commented Jun 21, 2025

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

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 24, 2025 3:01am

@akshayka akshayka requested a review from dmadisetti June 21, 2025 20:59
*/
export const connectionAtom = atom<ConnectionStatus>({
state: WebSocketState.CONNECTING,
state: isStaticNotebook() ? WebSocketState.OPEN : WebSocketState.CONNECTING,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels suspicious because it only appears to be needed when serving HTML with local assets; might not be the right fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will test locally, but looks fine

manzt
manzt previously approved these changes Jun 22, 2025
Copy link
Contributor

@manzt manzt left a comment

Choose a reason for hiding this comment

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

This looks great! Wondering if instead of checking the type of notebook globally in the connection atom, we can try to "fix" the static case at the websocket implementation level?

// In frontend/src/core/websocket/StaticWebsocket.ts
addEventListener(type: string, listener: EventListener): void {
  if (type === "open") {
    this.onopen = listener;
    // Trigger open event immediately since static websockets are always "open"
    queueMicrotask(() => this.onopen?.(new Event("open")));
  }
}

This way the StaticWebsocket properly simulates a WebSocket that immediately opens, and we avoid leaking static mode awareness into the global connection state.

@akshayka
Copy link
Contributor Author

Yea that's much better. I'll make the change before merging. Thank you!

@manzt
Copy link
Contributor

manzt commented Jun 22, 2025

Hmm... just tried my change locally and still see the spinner. I can poke around a bit, but guessing the queueMicroTask probably introduces a race condition.

@akshayka
Copy link
Contributor Author

@manzt let me know if you want changes made, or if okay to merge as is

akshayka and others added 4 commits June 23, 2025 22:04
* actually generate the notebook session and outputs
* for static notebooks, initialize the "websocket" as open
@manzt
Copy link
Contributor

manzt commented Jun 24, 2025

Still getting used to procedure with taking over other's PRs... I was able to check this out locally and get the following to work with:

wget https://gist.githubusercontent.com/manzt/f2a9299d913dc9801c639134db6e06e8/raw/f9ca2d565a3df3597a0eb85fbec230eac71b17f7/mo-3226.py
make fe
uv run --with matplotlib,numpy marimo development preview mo-3226.py
diff --git i/frontend/src/core/network/connection.ts w/frontend/src/core/network/connection.ts
index daa8d339..67f58b5b 100644
--- i/frontend/src/core/network/connection.ts
+++ w/frontend/src/core/network/connection.ts
@@ -1,7 +1,6 @@
 /* Copyright 2024 Marimo. All rights reserved. */
 import { atom } from "jotai";
 import { waitFor } from "../state/jotai";
-import { isStaticNotebook } from "../static/static-state";
 import { type ConnectionStatus, WebSocketState } from "../websocket/types";
 
 /**
@@ -9,7 +8,7 @@ import { type ConnectionStatus, WebSocketState } from "../websocket/types";
  * Initialized to CONNECTING for normal mode, OPEN for static mode.
  */
 export const connectionAtom = atom<ConnectionStatus>({
-  state: isStaticNotebook() ? WebSocketState.OPEN : WebSocketState.CONNECTING,
+  state: WebSocketState.CONNECTING,
 });
 
 export function waitForConnectionOpen() {
diff --git i/frontend/src/core/websocket/StaticWebsocket.ts w/frontend/src/core/websocket/StaticWebsocket.ts
index d4120366..981a23e2 100644
--- i/frontend/src/core/websocket/StaticWebsocket.ts
+++ w/frontend/src/core/websocket/StaticWebsocket.ts
@@ -17,9 +17,20 @@ export class StaticWebsocket implements IReconnectingWebSocket {
   onmessage = null;
   onopen = null;
 
-  addEventListener(type: unknown, callback: unknown, options?: unknown): void {
-    // Noop
+  addEventListener(
+    type: string,
+    callback: EventListener,
+    _options?: unknown,
+  ): void {
+    // Normally this would be a no-op in a mock, but we simulate a synthetic "open" event
+    // to mimic the WebSocket transitioning from CONNECTING to OPEN state.
+    if (type === "open") {
+      queueMicrotask(() => {
+        callback(new Event("open"));
+      });
+    }
   }
+
   removeEventListener(
     type: unknown,
     callback: unknown,

I'm happy to apply the diff directly if that is preferred!

don't leak static notebook awareness to connection object
@akshayka
Copy link
Contributor Author

@manzt thanks! just applied it

@akshayka akshayka merged commit ff59d67 into main Jun 24, 2025
26 of 37 checks passed
@akshayka akshayka deleted the aka/fix-mo-dev-preview branch June 24, 2025 03:22
@github-actions
Copy link

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

dmadisetti added a commit that referenced this pull request Jun 24, 2025
## 📝 Summary

quarto `0.14.7` is broken.

I'm having trouble confirming this is the issue by replicating it
locally, since I can't replicate the breakage. My dev served assets seem
to work in general. But `being islands` != `being wasm` as is, and I
think some of the logic around sockets gets lost (0.14.6 works, so this
may be the offending PR: #5403)

@akshayka

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
kyrre pushed a commit to kyrre/marimo that referenced this pull request Jun 26, 2025
This PR fixes `marimo development preview` for previewing static HTML
using local assets. Previously the generated HTML didn't include outputs
(ie the notebook wasn't run), and also got stuck on attempting to
connect to the frontend ("Connecting..." spinner)

* actually generate the notebook session and outputs
* for static notebooks, initialize the connection as open

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dylan <dylan@marimo.io>
kyrre pushed a commit to kyrre/marimo that referenced this pull request Jun 26, 2025
## 📝 Summary

quarto `0.14.7` is broken.

I'm having trouble confirming this is the issue by replicating it
locally, since I can't replicate the breakage. My dev served assets seem
to work in general. But `being islands` != `being wasm` as is, and I
think some of the logic around sockets gets lost (0.14.6 works, so this
may be the offending PR: marimo-team#5403)

@akshayka

---------

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 Jun 28, 2025
This PR fixes `marimo development preview` for previewing static HTML
using local assets. Previously the generated HTML didn't include outputs
(ie the notebook wasn't run), and also got stuck on attempting to
connect to the frontend ("Connecting..." spinner)

* actually generate the notebook session and outputs
* for static notebooks, initialize the connection as open

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dylan <dylan@marimo.io>
sebbeutler pushed a commit to sebbeutler/marimo that referenced this pull request Jun 28, 2025
## 📝 Summary

quarto `0.14.7` is broken.

I'm having trouble confirming this is the issue by replicating it
locally, since I can't replicate the breakage. My dev served assets seem
to work in general. But `being islands` != `being wasm` as is, and I
think some of the logic around sockets gets lost (0.14.6 works, so this
may be the offending PR: marimo-team#5403)

@akshayka

---------

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
This PR fixes `marimo development preview` for previewing static HTML
using local assets. Previously the generated HTML didn't include outputs
(ie the notebook wasn't run), and also got stuck on attempting to
connect to the frontend ("Connecting..." spinner)

* actually generate the notebook session and outputs
* for static notebooks, initialize the connection as open

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dylan <dylan@marimo.io>
sebbeutler pushed a commit to sebbeutler/marimo that referenced this pull request Jul 7, 2025
## 📝 Summary

quarto `0.14.7` is broken.

I'm having trouble confirming this is the issue by replicating it
locally, since I can't replicate the breakage. My dev served assets seem
to work in general. But `being islands` != `being wasm` as is, and I
think some of the logic around sockets gets lost (0.14.6 works, so this
may be the offending PR: marimo-team#5403)

@akshayka

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants