-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
base: master
Are you sure you want to change the base?
webassembly: Fix binding of self to JavaScript methods. #17729
Conversation
Signed-off-by: Damien George <damien@micropython.org>
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. 🚀 New features to boost your workflow:
|
Signed-off-by: Damien George <damien@micropython.org>
7ebe484
to
95a2769
Compare
Add test for |
@ntoll @WebReflection FYI |
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 In short, if you end up always with arrow functions and/or most global constructors would ignore the context, but in case of namespaces, context is ignored too, that is methods from Again, if latest tests work, I think for the time being we're all good! |
Signed-off-by: Damien George <damien@micropython.org>
Thanks @WebReflection for the review. I've now added those two extra tests and they pass.
If the code can be simplified so it doesn't have to select between Anyway, the existing and new tests pass, so at least the semantics seem correct, even if they are not optimally implemented. |
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.