-
Notifications
You must be signed in to change notification settings - Fork 1.5k
support different pyodide versions #328
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
support different pyodide versions #328
Conversation
…ript loading operations are done
… concept of app config so users can set if they want to autoclose the loader of handle it themselves
@@ -14,6 +14,11 @@ | |||
- paths: | |||
- ./utils.py | |||
</py-env> | |||
<py-config> |
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.
i like this pattern
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.
Okay, sorry for taking so long, this ping got lost in my email box.
I've tested with all examples! Seems to work fine.
One thing though is that I'm getting:
(!) Plugin typescript: @rollup/plugin-typescript TS2488: Type 'unknown' must have a '[Symbol.iterator]()' method that returns an iterator.
src/components/pyconfig.ts: (157:48)
157 this.values = Object.assign({}, ...loadedValues);
~~~~~~~~~~~~
(!) Plugin typescript: @rollup/plugin-typescript: Rollup 'sourcemap' option must be set to generate source maps.
While running the server. I don't know if it's relevant...? Cause as I said all is working smoothly.
@@ -52,4 +174,17 @@ export class PyConfig extends BaseEvalElement { | |||
close() { | |||
this.remove(); | |||
} | |||
|
|||
loadRuntimes(){ | |||
console.log("Initializing runetimes...") |
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.
Typo? runetimes
@@ -1,76 +1,5 @@ | |||
<script lang="ts"> | |||
import Tailwind from './Tailwind.svelte'; | |||
import { loadInterpreter } from './interpreter'; |
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 was this moved? So it's easier to use with the new array of runtimes that should be on the pyconfig
? Or just in general this is the wrong place for this kind of config?
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.
I removed it because the whole logic was moved to pyconfig.ts
and not used here anymore, if needed, other parts can import directly from ./interpreter
The reason I moved to pyconfig was that it helps isolate the whole logic around declaring and initializing runtimes somewhere more contained than just the App. Not sure if pyconfig is the best place for it to be, but it's better than before.
I think there's less and less need for us to have the svelte app... and this also helps us understand how much we depend on it. Atm, afaict, we are only relying on its stores.
Thanks @marimeireles , will fix the issues you pointed out before merging. The one related to the |
Fixes #172