-
Notifications
You must be signed in to change notification settings - Fork 9
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
Pyodide headers #1104
Pyodide headers #1104
Changes from 8 commits
a28999d
75878f1
de5068d
60c9c3f
a674de4
732c77e
5f2998a
28a493a
9dba0f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,4 @@ | ||
/* eslint import/no-webpack-loader-syntax: off */ | ||
/* eslint-disable react-hooks/exhaustive-deps */ | ||
|
||
import "../../../../../assets/stylesheets/PythonRunner.scss"; | ||
import React, { useContext, useEffect, useMemo, useRef, useState } from "react"; | ||
import { useDispatch, useSelector } from "react-redux"; | ||
|
@@ -20,27 +18,25 @@ import OutputViewToggle from "../OutputViewToggle"; | |
import { SettingsContext } from "../../../../../utils/settings"; | ||
import RunnerControls from "../../../../RunButton/RunnerControls"; | ||
|
||
const PyodideRunner = ({ active }) => { | ||
const getWorkerURL = (url) => { | ||
const content = ` | ||
/* global PyodideWorker */ | ||
console.log("Worker loading"); | ||
importScripts("${url}"); | ||
const pyodide = PyodideWorker(); | ||
console.log("Worker loaded"); | ||
`; | ||
const blob = new Blob([content], { type: "application/javascript" }); | ||
return URL.createObjectURL(blob); | ||
}; | ||
const getWorkerURL = (url) => { | ||
const content = ` | ||
/* global PyodideWorker */ | ||
console.log("Worker loading"); | ||
importScripts("${url}"); | ||
const pyodide = PyodideWorker(); | ||
console.log("Worker loaded"); | ||
`; | ||
const blob = new Blob([content], { type: "application/javascript" }); | ||
return URL.createObjectURL(blob); | ||
}; | ||
|
||
const workerUrl = getWorkerURL(`${process.env.PUBLIC_URL}/PyodideWorker.js`); | ||
const PyodideRunner = (props) => { | ||
const { active } = props; | ||
|
||
// Blob approach + targeted headers - no errors but headers required in host app to interrupt code | ||
const workerUrl = getWorkerURL(`${process.env.PUBLIC_URL}/PyodideWorker.js`); | ||
const pyodideWorker = useMemo(() => new Worker(workerUrl), []); | ||
|
||
if (!pyodideWorker) { | ||
console.error("PyodideWorker is not initialized"); | ||
} | ||
|
||
const interruptBuffer = useRef(); | ||
const stdinBuffer = useRef(); | ||
const stdinClosed = useRef(); | ||
|
@@ -134,8 +130,6 @@ const PyodideRunner = ({ active }) => { | |
}; | ||
|
||
const handleInput = async () => { | ||
// TODO: Sk.sense_hat.mz_criteria.noInputEvents = false; | ||
|
||
if (stdinClosed.current) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this comment deleted on purpose or has it been slashed by Copilot? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. copilot 🤦 will restore |
||
stdinBuffer.current[0] = -1; | ||
return; | ||
|
@@ -157,7 +151,7 @@ const PyodideRunner = ({ active }) => { | |
stdinBuffer.current[0] = currentLength; | ||
|
||
if (ctrlD) { | ||
stdinClosed.current = true; // Don't accept any more stdin this run. | ||
stdinClosed.current = true; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as my other question about deleted comment above |
||
}; | ||
|
||
|
@@ -218,20 +212,19 @@ const PyodideRunner = ({ active }) => { | |
writeFile([name, extension].join("."), content); | ||
} | ||
|
||
// program is the content of the component with name main and extension py | ||
const program = projectCode.find( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another deleted comment |
||
(component) => component.name === "main" && component.extension === "py", | ||
).content; | ||
|
||
if (interruptBuffer.current) { | ||
interruptBuffer.current[0] = 0; // Clear previous signals. | ||
interruptBuffer.current[0] = 0; | ||
} | ||
pyodideWorker.postMessage({ method: "runPython", python: program }); | ||
}; | ||
|
||
const handleStop = () => { | ||
if (interruptBuffer.current) { | ||
interruptBuffer.current[0] = 2; // Send a SIGINT signal. | ||
interruptBuffer.current[0] = 2; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More comments |
||
pyodideWorker.postMessage({ method: "stopPython" }); | ||
disableInput(); | ||
|
@@ -312,6 +305,11 @@ const PyodideRunner = ({ active }) => { | |
} | ||
}; | ||
|
||
if (!pyodideWorker) { | ||
console.error("PyodideWorker is not initialized"); | ||
return; | ||
} | ||
|
||
return ( | ||
<div | ||
className={`pythonrunner-container pyodiderunner${ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,11 @@ const Dotenv = require("dotenv-webpack"); | |
const HtmlWebpackPlugin = require("html-webpack-plugin"); | ||
const WorkerPlugin = require("worker-plugin"); | ||
|
||
let publicUrl = process.env.PUBLIC_URL || "/"; | ||
if (!publicUrl.endsWith("/")) { | ||
publicUrl += "/"; | ||
} | ||
|
||
module.exports = { | ||
entry: { | ||
"web-component": path.resolve(__dirname, "./src/web-component.js"), | ||
|
@@ -73,6 +78,8 @@ module.exports = { | |
output: { | ||
path: path.resolve(__dirname, "./build"), | ||
filename: "[name].js", | ||
publicPath: publicUrl, | ||
workerPublicPath: publicUrl, | ||
}, | ||
devServer: { | ||
host: "0.0.0.0", | ||
|
@@ -88,6 +95,25 @@ module.exports = { | |
"Access-Control-Allow-Methods": "GET, POST, PUT, DELETE, PATCH, OPTIONS", | ||
"Access-Control-Allow-Headers": | ||
"X-Requested-With, content-type, Authorization", | ||
// Pyodide - required for input and code interruption - needed on the host app | ||
"Cross-Origin-Opener-Policy": "same-origin", | ||
"Cross-Origin-Embedder-Policy": "require-corp", | ||
Comment on lines
+98
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So are we just assuming that host pages will have to add these themselves for now, or am I not understanding? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea that's right. So on editor.raspberrypi.org we've already got these set 👍 |
||
}, | ||
setupMiddlewares: (middlewares, devServer) => { | ||
devServer.app.use((req, res, next) => { | ||
// PyodideWorker scripts - cross origin required on scripts needed for importScripts | ||
if ( | ||
[ | ||
"/pyodide/shims/_internal_sense_hat.js", | ||
"/pyodide/shims/pygal.js", | ||
"/PyodideWorker.js", | ||
].includes(req.url) | ||
) { | ||
res.setHeader("Cross-Origin-Resource-Policy", "cross-origin"); | ||
} | ||
next(); | ||
}); | ||
return middlewares; | ||
}, | ||
}, | ||
plugins: [ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this one
ASSETS_URL
when the ones above arePUBLIC_URL
? I've gotten a bit confused about what the difference is between the two vars to be honest with youThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah they can both probably be merged into one env var now. When the site was deployed and assets were in the bucket but on the same domain it was causing a problem so I exposed the bucket via a URL and hit that directly a while back. But now its all bucket based
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing them all to
ASSETS_URL
in this file then we can remove that later if we want to 👍