Skip to content

Make interpreters code async by default #112

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

Merged
merged 4 commits into from
Aug 5, 2024
Merged

Conversation

WebReflection
Copy link
Contributor

This MR would like to make all interpreters run async code by default. The current logic is that if the async attribute is present and its value is explicitly false then it goes back to sync mode, otherwise all code runs just with top-level await available by default.

This MR also already showed that while it's trivial at code level to change these expectations, 5 tests had really hard coded different expectations and I am expecting tests in PyScript to have issue too once this is brought in.

The TL;DR of this MR is that it might take a lot of effort to make this fundamental change around our code but it should be duable, given enough time to test all the things are still working and we don't break all our test coverage we had in place before.

/cc @ntoll

@WebReflection
Copy link
Contributor Author

WebReflection commented Jul 31, 2024

actually ... I've discovered a bug with async scripts and the document.currentScript accessor ... so here the thing, we cannot grant that at any point in time the currently executing asynchronous script points at the right currentScript within the Python code because async means other scripts might also run in the meantime.

However, I think this is one of those non resolvable cases as these are fighting against the current Web standard implementation of document.currentScript and we should never interfere (more than we do already) with that part of the specs.

I think it's safe to move forward with this MR as all tests are green and I am quite satisfied with the result.

@WebReflection WebReflection marked this pull request as ready for review July 31, 2024 15:44
@WebReflection WebReflection force-pushed the async-by-default branch 2 times, most recently from 2b2f2c1 to 3f7f65b Compare August 1, 2024 07:48
@WebReflection WebReflection force-pushed the async-by-default branch 2 times, most recently from 9c2cec6 to 3b9099e Compare August 5, 2024 09:14
@WebReflection WebReflection merged commit 32b920c into main Aug 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant