Skip to content
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

Pyodide fails to load after require.js #4863

Closed
seragunn opened this issue Jun 14, 2024 · 4 comments · Fixed by #4866
Closed

Pyodide fails to load after require.js #4863

seragunn opened this issue Jun 14, 2024 · 4 comments · Fixed by #4866
Labels
bug Something isn't working

Comments

@seragunn
Copy link

🐛 Bug

When loading require.js before Pyodide, calling loadPyodide results in an error: Uncaught (in promise) TypeError: z.default.parse is not a function

This corresponds to

pyodide/src/js/compat.ts

Line 263 in f1acd57

let fileName = ErrorStackParser.parse(err)[0].fileName!;

No error occurs if Pyodide is loaded first (at least not apparently).

To Reproduce

It's pretty much the first example in the docs but with requirejs loaded ahead of pyodide. I'm loading pyodide in a module as that helps track the source but the error is the same even without modules.

<!doctype html>
<html>

<head>
  <script src="https://cdn.jsdelivr.net/npm/requirejs@2.3.6/require.min.js"></script>

  <script type="module">
    import { loadPyodide } from "https://cdn.jsdelivr.net/pyodide/v0.26.1/full/pyodide.mjs"

    async function main() {
      let pyodide = await loadPyodide();
      console.log(pyodide.runPython(`
              import sys
              sys.version
          `));
      let output = pyodide.runPython("1 + 2");
      document.getElementById("py-out").innerText = output;
    }
    main().catch((e) => {
      document.getElementById("py-out").innerText = "Error";
    });

    // export main so it can be tested in the dev console
    globalThis.main = main;
  </script>
</head>

<body>
  <div id="py-out">Running...</div>
</body>

</html>

Expected behavior

Pyodide should load

Environment

  • Pyodide Version 0.26.1
  • Browser version Firefox 127.0/Linux and Chromium 126.0.6478.55/Linux
  • Any other relevant information:

Platform: Arch Linux

Additional context

I tested this with each version of Pyodide from v0.21.0 onward. The latest version of Pyodide which is compatible seems to be 0.23.4
@seragunn seragunn added the bug Something isn't working label Jun 14, 2024
@ryanking13
Copy link
Member

ryanking13 commented Jun 15, 2024 •
edited
Loading

Thanks for the report. This is because loading require.js has a side effect of defining define, and define.amd variables, and ErrorStackParser assumes that it is in CommonJS environment when define is defined (code pointer, another related issue).

I get a headache when I see all this commonjs, umd, and amd stuff. I think it would be a good idea to remove the legacy code from the error-stack-parser and just embed the necessary parts into pyodide.

@ryanking13
Copy link
Member

cc: @RulerOfCakes I wonder if you have some experience in dealing with this kind of CommonJS / AMD issues.

@RulerOfCakes
Copy link
Contributor

Although I'm also not that familiar with these types of module conflict issues, it seems that esbuild isn't exactly friendly with the UMD syntax(evanw/esbuild#507), hence the UMD syntax 'leaking out' even from an ESM build of pyodide is non trivial to avoid. With the AMD globals present in the environment as you have pointed out, this ends up resolving to an AMD export. I'm not sure if there are any trivial fixes aside from embedding the dependency itself or messing around with regexes in the build step, or forcefully removing the AMD globals in the build output.

A better/safer way to prevent these types of unexpected conflicts caused by globals in the future would perhaps be something like using the upcoming ShadowRealm API, which is under development for V8 / Node.

@ryanking13
Copy link
Member

I see. Thanks for the help. I'll see if I can vendor error-stack-parser.

@ryanking13 ryanking13 mentioned this issue Jun 17, 2024
Merged
2 tasks
@coatless coatless mentioned this issue Aug 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Vendor error-stack-parser ryanking13/pyodide
3 participants
@seragunn @RulerOfCakes @ryanking13