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.

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

webassembly: Add proxying between Python and JavaScript objects, and change top-level interface for PyScript use #13583

Merged
merged 19 commits into from Mar 22, 2024

Conversation

dpgeorge
Copy link
Member

@dpgeorge dpgeorge commented Feb 2, 2024

This PR extends, improves and changes the webassembly port so that it's suitable for use in PyScript, (https://pyscript.net/, https://github.com/pyscript/pyscript).

The main things in this PR are:

  • Add transparent proxying of Python and JavaScript objects between JavaScript and Python. That allows passing objects and making function/method calls between the two languages.
  • Change the top-level interface for this port to generate a micropython.mjs module, rather than micropython.js. This module exposes a loadMicroPython() function which returns an object that can be used to execute Python, access the Python globals dict, and other things.
  • Improve run-tests.py so it can automatically run webassembly tests via node, both .py tests and .js/.mjs tests.
  • Add a set of .js/.mjs tests to test the proxying in the webassembly port.
  • Enhance the random and time modules on the webassembly port.

A lot of the proxying and top-level interface is made to match the API of Pyodide (https://github.com/pyodide/pyodide/).

TODO:

  • remove remaining stray TODO's and commented-out code
  • finish cleaning up commits
  • possibly add support for variants, and make standard and pyscript variants

@dpgeorge dpgeorge marked this pull request as draft February 2, 2024 02:31
@dpgeorge
Copy link
Member Author

dpgeorge commented Feb 2, 2024

@ntoll @WebReflection FYI

Copy link

github-actions bot commented Feb 2, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (acbdbcd) to head (35b2edf).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13583   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21200    21200           
=======================================
  Hits        20860    20860           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpgeorge dpgeorge added this to the release-1.23.0 milestone Feb 2, 2024
//console.debug("APPLY", target, argumentsList);
let args = 0;

// TODO: is this the correct thing to do, strip trailing "undefined" args?
Copy link

Choose a reason for hiding this comment

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

while (argumentList.length && argumentList.at(-1) === void 0) // or undefined
  argumentList.pop();

Choose a reason for hiding this comment

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

alternatively ...

let { length } = argumentList;
while (length && argumentList[length - 1] === undefined) length--;

// drop all at once if needed
if (length !== argumentList.length)
  argumentList.splice(length);

Choose a reason for hiding this comment

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

... or even terser ...

let { length } = argumentList;
while (length-- && argumentList[length] === undefined);

// drop all at once if needed
if (++length !== argumentList.length)
  argumentList.splice(length);

you pick whatever you prefer :-)

return true;
},
has(target, prop) {
throw Error("has not implemented");

Choose a reason for hiding this comment

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

this would still throw for "test" in pyObject brand checks

Choose a reason for hiding this comment

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

I am a bit confused (but hadn't had my coffee yet ...) ... no change was made to the Proxy traps and this specific error has been reported all over the place ... is this code meant to actually go or something still need to be changed? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry, I meant to say that this bit was still to come.

The PR here is the current version that's published to npm.

But, that said, I've just now pushed a commit to implement has, along with a new test for this feature. Let me know what you think about it.

Choose a reason for hiding this comment

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

I'll have a look, thank you!

Choose a reason for hiding this comment

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

I've no idea where that is though ... 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

If you look at the most recent commits, the 4 most recent ones implement PyProxy.has and PyProxy.ownKeys and add tests for these.

@dpgeorge dpgeorge force-pushed the webassembly-add-modjs branch 6 times, most recently from 419ffd1 to cabe26e Compare February 3, 2024 13:23
@@ -0,0 +1,19 @@
// Test polyfill of a method on a built-in.

Choose a reason for hiding this comment

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

this reminded me I should explicitly put a license in here 🤣

jokes a part, glad my poly helped testing this stuff as we do use it all over the place 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

We take licensing/copyright seriously. Should I add anything here?

Choose a reason for hiding this comment

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

those 3 LOC aren't anything special and the purpose of that polyfill is to die ASAP (once all browsers support it) so don't worry about it, I haven't invented anything and the pattern is pretty simple too, the only trick there is the usage of var (on purpose) but that's also just part of the specs.

We're good, and you have the author of that poly stating that in here 👍

@WebReflection
Copy link

beside the terser suggestion to split all at once arguments (usually faster than N pop() but I don't think that's critical path for performance anyway) my only hint, from a JS maintainer point of view, is to somehow normalize the usage of ' VS " or add a linter but it's also not a blocker and not so crucial ... overall this would be a 👍 to me and I can maybe help later on implementing that deleteProperty and set trap in the near future.

Thanks!

@dpgeorge
Copy link
Member Author

dpgeorge commented Feb 5, 2024

normalize the usage of ' VS " or add a linter

Yes that would be a good thing to do.

What style do you use? Can you recommend a light-weight linter?

@WebReflection
Copy link

WebReflection commented Feb 5, 2024

we use eslint but not because we chose it (or I did) because it was there already and it's somehow the industry standard.

Although, I think these days I would instead consider quick-lint-js or biome as these are both faster and less opinionated or easier to configure / bootstrap.

As this project is used in polyscript I can at least point at how we configured it ... then again, mostly comes from original PyScript config: https://github.com/pyscript/polyscript/blob/main/.eslintrc.json

edit P.S. I don't have any strong opinion about any linting rule ... make yourself comfortable with whatever choice you have, my hint was about consistency and an easier way for possible contributors to not make the code look too diverse over time.

@WebReflection
Copy link

@dpgeorge so I've built latest and things are pretty exciting ... but still there's something missing on the MicroPython side ... assuming the micropython.mjs is the one generated by this branch (which is in my case) I have this successfully working:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>MicroPython - Latest</title>
    <link rel="stylesheet" href="https://pyscript.net/releases/2024.1.3/core.css">
    <script type="module" src="https://pyscript.net/releases/2024.1.3/core.js"></script>
</head>
<body>
    <script>
        function ownKeys(py_or_js) {
            console.log("a" in py_or_js);
            console.log(Reflect.ownKeys(py_or_js));
        }
    </script>
    <mpy-config>
        interpreter = "http://localhost:8080/micropython.mjs"
    </mpy-config>
    <script type="mpy" worker>
        from pyscript import window
        import sys
        print(sys.version)

        window.ownKeys({ "a": 1 })
        # true
        # ["length", "a"]
        #  "length" should not be there but it's OK
    </script>
</body>
</html>

but as soon as I remove the worker attribute I have:

false
['clear', 'copy', 'get', 'items', 'keys', 'pop', 'popitem', 'setdefault', 'update', 'values', 'fromkeys', 'set', 'delete']

Which is somehow expected as I get all the dictionary keys but I don't have a way to convert via Python directly to JS objects, which happens instead automatically with workers, as those references are converted before being proxied around via the PyProxy.toJs() ... without any ffi.to_js on Python side, and without a worker, I can't easily pass around dictionaries to JS functions that are expecting just plain JS object literals, produced by that PyProxy.toJs(pythonThing).

Is there any plan or chance that the PyProxy, or better its static toJs() utility, can be used also within the Python code? 🤔

Thank you!

@dpgeorge dpgeorge force-pushed the webassembly-add-modjs branch 3 times, most recently from e00250f to 0f0cb62 Compare February 21, 2024 04:09
@dpgeorge
Copy link
Member Author

@WebReflection I have added formatting and linting using Biome (with indenting set to 4 spaces... at least that matches the Python and C code, and PyScript's style!).

But Biome lint is complaining about your promise with resolvers code:

  × The assignment should not be in an expression.
  
     3 │ Promise.withResolvers ||
   > 4 │     (Promise.withResolvers = function withResolvers() {
       │      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   > 5 │         let a;
        ...
  > 11 │         return { resolve: a, reject: b, promise: c };
  > 12 │     });
       │     ^
    13 │ 
    14 │ const mp = await (await import(process.argv[2])).loadMicroPython();
  
  i The use of assignments in expressions is confusing.
    Expressions are often considered as side-effect free.

Do you know a good way to fix that? (I already converted the var into let to make Biome happier.)

@dpgeorge
Copy link
Member Author

Is there any plan or chance that the PyProxy, or better its static toJs() utility, can be used also within the Python code?

I have now implemented jsffi.to_js() on the Python side. Hopefully it has the correct semantics for you.

As part of this I added a new jsffi module and moved things out of the existing pyodide module, and then removed the pyodide module. I don't think it makes sense for MicroPython to try and implement the pyodide module because our API is different in semantics. So I've put the relevant functionality in jsffi:

  • jsffi.create_proxy()
  • jsffi.to_js()
  • jsffi.JsProxy -- a type

Hopefully that change (removal of pyodide module) won't impact pyscript too much 😬

@dpgeorge
Copy link
Member Author

@WebReflection regarding your test above with in and ownKeys() applied (on the JS side) to a Python dictionary: I guess I must have misunderstood the semantics of has and ownKeys (as JS concepts).

In Python you have:

  • item in collection which is equivalent to collection.__contains__(item)
  • `hasattr(obj, method)

They are quite different although can be confused with each other. If the subject object is a dict, then in/__contains__ checks if the dict contains within it the given key. But hasattr will check if the dict responds to the given method (also it will check for attributes, but dict's don't have any attributes).

In the webassembly port at the moment, I implemented the latter, ie hasattr semantics. So that's why you are getting a list of methods rather than a list of the keys/elements of the dict.

Should I instead implement in/__contains__?

@WebReflection
Copy link

@dpgeorge the in operator in JS does something completely different from Python: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in

'key' in {key: false} is True in JS, it's about knowing if the object or anything down its inheritance / prototypal chain has a key with a name 'key', it's not about the value ... ever ... even with arrays, it's 0 in [] is false but 0 in [0] is true.

Reflect.ownKeys returns keys that belong to the current object, not keys inherited via class or anything else ... this is still about keys.

I think hasattr in that regard makes sense, or better, it is what in operator expects so thumb up here, thank you!

@WebReflection
Copy link

WebReflection commented Feb 21, 2024

@dpgeorge sorry for late reply here too ...

Do you know a good way to fix that? (I already converted the var into let to make Biome happier.)

you can't convert my code to a let expression or it breaks as var is hoisted (available in the whole scope out of the box) while let is not, so my patch won't work and it will fail hard on browsers that don't have Promise.withResolvers ... my code is rarely accidental 😉 actually it was instead a leftover previous code, you can use let there as the a and b are defined before c gets a chance so I will update my code, MY BAD! ... you need to revert that and you need to put an ignore comment for your linter right there and never care about that code ever again in your future: it doesn't just work and it's 100% code-covered, it is written exactly the way it should be written!

edit btw this is why foreign modules should be imported and not copy/pasted ... if imported no linter/tool would bother you ever about foreign code as every code might have different tools behind to produce still valid code your linter or toolchain should not / ever care about.

edit 2

let a, b;
const c = new this(function (resolve, reject) {
    a = resolve;
    b = reject;
});
return {resolve: a, reject: b, promise: c};

This code should work without linting complains ... please let me know if it doesn't.

@dpgeorge dpgeorge force-pushed the webassembly-add-modjs branch 2 times, most recently from fb08e33 to 6f58010 Compare March 15, 2024 00:38
@WebReflection
Copy link

@dpgeorge

That's now done and your test above now works.

do you mind publishing it to npm otherwise I can't grab latest automatically from there and current state is not ideal? Thanks!

@dpgeorge
Copy link
Member Author

@dpgeorge dpgeorge force-pushed the webassembly-add-modjs branch 3 times, most recently from c1a82f9 to 557c4ba Compare March 21, 2024 23:39
Enabled by MICROPY_COMPILE_ALLOW_TOP_LEVEL_AWAIT.  When enabled, this means
that scope such as module-level functions and REPL statements can yield.
The outer C code must then handle this yielded generator.

Signed-off-by: Damien George <damien@micropython.org>
Following other ports.

Signed-off-by: Damien George <damien@micropython.org>
This eliminates the need for wrapper.js to run to set up the time.

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
All output is now handled by Emscripten's stdio facility.

Signed-off-by: Damien George <damien@micropython.org>
When enabled the GC will not reclaim any memory on a call to
`gc_collect()`.  Instead it will grow the heap.

Signed-off-by: Damien George <damien@micropython.org>
This commit cleans up and generalises the Makefile, adds support for
variants (following the unix port) and adds the "standard" variant as the
default variant.

Signed-off-by: Damien George <damien@micropython.org>
This commit improves the webassembly port by adding:

- Proxying of Python objects to JavaScript with a PyProxy type that lives
  on the JavaScript side.  PyProxy implements JavaScript Proxy traps such
  as has, get, set and ownKeys, to make Python objects have functionality
  on the JavaScript side.

- Proxying of JavaScript objects to Python with a JsProxy type that lives
  on the Python side.  JsProxy passes through calls, attributes,
  subscription and iteration from Python to JavaScript.

- A top-level API on the JavaScript side to construct a MicroPython
  interpreter instance via `loadMicroPython()`.  That function returns an
  object that can be used to execute Python code, access the Python globals
  dict, access the Emscripten filesystem, and other things.  This API is
  based on the API provided by Pyodide (https://pyodide.org/).  As part of
  this, the top-level file is changed from `micropython.js` to
  `micropython.mjs`.

- A Python `js` module which can be used to access all JavaScript-side
  symbols, for example the DOM when run within a browser.

- A Python `jsffi` module with various helper functions like
  `create_proxy()` and `to_js()`.

- A dedenting lexer which automatically dedents Python source code if every
  non-empty line in that source starts with a common whitespace prefix.
  This is very helpful when Python source code is indented within a string
  within HTML or JavaScript for formatting reasons.

Signed-off-by: Damien George <damien@micropython.org>
With this commit, `interpreter.runPythonAsync(code)` can now be used to run
Python code that uses `await` at the top level.  That will yield up to
JavaScript and produce a thenable, which the JavaScript runtime can then
resume.  Also implemented is the ability for Python code to await on
JavaScript promises/thenables.  For example, outer JavaScript code can
await on `runPythonAsync(code)` which then runs Python code that does
`await js.fetch(url)`.  The entire chain of calls will be suspended until
the fetch completes.

Signed-off-by: Damien George <damien@micropython.org>
This allows running MicroPython webassembly from the command line using:

    node micropython.mjs

Signed-off-by: Damien George <damien@micropython.org>
This is the JavaScript API for starting and running a REPL.

Signed-off-by: Damien George <damien@micropython.org>
This commit adds a pyscript variant for use in https://pyscript.net/.

The configuration is:
- No ASYNCIFY, in order to keep the WASM size down and have good
  performance.
- MICROPY_CONFIG_ROM_LEVEL_FULL_FEATURES to enable most features.
- Custom manifest that includes many of the python-stdlib libraries.
- MICROPY_GC_SPLIT_HEAP_AUTO to increase GC heap size instead of doing a
  collection when memory is exhausted.  This is needed because ASYNCIFY is
  disabled.  Instead the GC collection is run at the top-level before
  executing any Python code.
- No MICROPY_VARIANT_ENABLE_JS_HOOK because there is no asynchronous
  keyboard input to interrupt a running script.

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
This allows running tests with a .js/.mjs suffix, and also .py tests using
node and the webassembly port.

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Enable Biome on all of webassembly port and tests.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge marked this pull request as ready for review March 22, 2024 03:32
@dpgeorge dpgeorge merged commit 35b2edf into micropython:master Mar 22, 2024
64 checks passed
@dpgeorge dpgeorge deleted the webassembly-add-modjs branch March 22, 2024 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants