Skip to content

[Draft] Fix usePythonConsole #490

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.

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

LamHo220
Copy link
Contributor

@LamHo220 LamHo220 commented Mar 7, 2025

About this pull request

There are several bugs or enhancement in the hook

  1. Invalid url in getInput() in next.js project for Chromium (SyntaxError: The string did not match the expected pattern for Safari)
  2. We can handle the output for users and the output stream should be stdout line 1 -> stdout line 2 -> ... -> stdout line n -> stderr.

Fixing the bug in 1.

  1. Adding workerUrl in props for the PythonProvider in next.js. This allowes the provider use public/react-py-sw.js for the service-worker;
  2. use windows.location.origin (location.origin for python-worker.ts) as the base url of the url in PythonProvider.
<PythonProvider workerUrl="react-py-sw.js">
    {children}
</PythonProvider>

Enhancing the output

For example, the python script

for i in range(10):
  print(i)
  if i == 2:
    raise ValueError()

should results in

0
1
2
Traceback (most recent call last):
  File "<console>", line 4, in <module>
ValueError

but the code in the docs will leads because the stdout is still updateing the results.

Traceback (most recent call last):
  File "<console>", line 4, in <module>
ValueError
0
1
2

Due to the rendering bug in Safari. By observation, updating output array in stdout stream will end faster than updating the stdout. We can wait until the stdout stream is ended which is stdout === output.join('') and add the stderr at the end.

TODO

  • fix input error for next.js
  • handle the outputs
  • show stderr after appending the previous stdout
  • testing in other browser
  • update docs
  • test in other frameworks

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.

1 participant