Content-Length: 320449 | pFad | http://github.com/micropython/micropython/pull/17729

41 webassembly: Fix binding of self to JavaScript methods. by dpgeorge · Pull Request #17729 · micropython/micropython · GitHub
Skip to content

webassembly: Fix binding of self to JavaScript methods. #17729

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dpgeorge
Copy link
Member

Summary

Fixes a bug in the binding of self/this to JavaScript methods.

The new semantics match Pyodide's behaviour, at least for the included tests.

Testing

A test has been added for the case that is fixed.

Signed-off-by: Damien George <damien@micropython.org>
Copy link

codecov bot commented Jul 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.41%. Comparing base (17fbc5a) to head (7571718).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17729      +/-   ##
==========================================
- Coverage   98.44%   98.41%   -0.04%     
==========================================
  Files         171      171              
  Lines       22208    22210       +2     
==========================================
- Hits        21863    21857       -6     
- Misses        345      353       +8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the webassembly-change-bind-to-call branch from 7ebe484 to 95a2769 Compare July 21, 2025 05:22
@dpgeorge
Copy link
Member Author

Add test for ar.push.call(ar, 3) and ar.push.call(ar2, 3).

@dpgeorge
Copy link
Member Author

@ntoll @WebReflection FYI

@WebReflection
Copy link
Contributor

I had a look and if the test works fine then I think it's OK if also these two work as expected #17729 (comment) which I believe is the case, as it shouldn't be too different from borrowing methods via the prototype ... however, I am also not super sure why there are so many invariants for the same thing but in general, at least in JS, I would use the apply trap for functions or something like __call__ trap in Python per each accessed reference, something to consider for the future?

In short, if you end up always with fn.call(js_context or js_undefined, ...args) a lot of code might be simplified but also with a proper trap there would be less guessing around and just natural consequences out of the meant code.

arrow functions and/or most global constructors would ignore the context, but in case of namespaces, context is ignored too, that is methods from Math or JSON or even window (or globalThis) so it feels like this dance might be reduced by far to me, but I let you decide if or when it'd be the right time to do so, also I need to find time myself to eventually contribute with a POC or something.

Again, if latest tests work, I think for the time being we're all good!

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
Copy link
Member Author

Thanks @WebReflection for the review.

I've now added those two extra tests and they pass.

so it feels like this dance might be reduced by far to me, but I let you decide if or when it'd be the right time to do so, also I need to find time myself to eventually contribute with a POC or something.

If the code can be simplified so it doesn't have to select between f(args) and f.call(args) that would be great, and I'd change it. But I just don't know how else to do it.

Anyway, the existing and new tests pass, so at least the semantics seem correct, even if they are not optimally implemented.

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.

2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/micropython/micropython/pull/17729

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy