Skip to content

fix: "Download as .py" in WASM run-mode exports#9268

Merged
mscolnick merged 1 commit intomarimo-team:mainfrom
moble:download_in_wasm_run
Apr 20, 2026
Merged

fix: "Download as .py" in WASM run-mode exports#9268
mscolnick merged 1 commit intomarimo-team:mainfrom
moble:download_in_wasm_run

Conversation

@moble
Copy link
Copy Markdown
Contributor

@moble moble commented Apr 19, 2026

📝 Summary

(I got a lot of help writing the test from Claude.)

When a notebook is exported with marimo export html-wasm --mode run, the resulting HTML embeds the notebook source in a <marimo-code> element, but the "Download as .py" menu item fails with "Failed to read code / Not implemented".

The reason is that PyodideBridge.readCode() calls this.saveRpc.readNotebook(). In read mode, getSaveWorker() returns a stub where every method is throwNotImplemented, which is what causes that failure.

This PR fixes the problem by checking in readCode if the app mode is "read", and if so it just reads the file from domElementFileStore.

📋 Pre-Review Checklist

  • 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 (N/A).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional). (N/A).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Copilot AI review requested due to automatic review settings April 19, 2026 19:55
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Apr 20, 2026 1:09pm

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes “Download as .py” for marimo export html-wasm --mode run by making the WASM bridge read notebook source from the embedded <marimo-code> element when running in read-mode, avoiding the read-mode SaveWorker stub that throws “Not implemented”.

Changes:

  • Import and use domElementFileStore in PyodideBridge.readCode() when getInitialAppMode() === "read".
  • Keep existing SaveWorker-based readNotebook() path for non-read modes.

Comment thread frontend/src/core/wasm/bridge.ts Outdated
Comment thread frontend/src/core/wasm/bridge.ts
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread frontend/src/core/wasm/__tests__/bridge.test.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 20, 2026

Deployment failed with the following error:

Creating the Deployment Timed Out.

@Light2Dark Light2Dark added the bug Something isn't working label Apr 20, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread frontend/src/core/wasm/__tests__/bridge.test.ts Outdated
Comment thread frontend/src/core/wasm/__tests__/bridge.test.ts Outdated
Comment thread frontend/src/core/wasm/__tests__/bridge.test.ts Outdated
@Light2Dark
Copy link
Copy Markdown
Collaborator

Thanks @moble.
Not sure why we disabled it before, deferring to @dmadisetti or @mscolnick .

On the implementation, is it possible to implement this in the readNotebook path? Instead of throwNotImplemented?

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@moble
Copy link
Copy Markdown
Contributor Author

moble commented Apr 20, 2026

On the implementation, is it possible to implement this in the readNotebook path? Instead of throwNotImplemented?

Yeah, I wasn't sure that it wouldn't mess up something else. But I've switched to that now. Thanks.

@mscolnick mscolnick requested a review from Light2Dark April 20, 2026 14:36
@mscolnick mscolnick merged commit da2fb63 into marimo-team:main Apr 20, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants