Skip to content

Added polyscript/service-worker as workaround for missing sabayon #2334

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 4 commits into
base: main
Choose a base branch
from

Conversation

WebReflection
Copy link
Contributor

@WebReflection WebReflection commented Apr 30, 2025

Description

see this live PyScript .com example


This MR tries to address @JoshuaLowe1002 comment #2333 (comment) but in a different way, because once implemented I've realized that polyfill gotta polyfill so, as opt-in, it cannot be imported or executed too late or nothing would actually work.

How to test this

First of all, the expected ServiceWorker code must be this one because the previous code used in sabayon would not work with its polyfill variant (it's outdated, I should actually deprecate everything I've done to date and just keep the polyfill as it's much better).

Secondly, there must be this <script> tag before pyscript on each page:

<!-- current polyfill helper -->
<script type="module" src="https://cdn.jsdelivr.net/npm/@pyscript/core@0.6.51/dist/service-worker.js"></script>
<!-- current PyScript branch published on npm -->
<script type="module" src="https://cdn.jsdelivr.net/npm/@pyscript/core@0.6.51/dist/core.js"></script>

That's it: if the browser needs a service-worker fallback it will bootstrap sabayon before any other code using/needing global Worker and Atomics get a chance to trap those classes.

In every other case (non Safari browsers) nothing actually happens.

Changes

  • updated polyscript to its latest after testing and providing the proposed solution
  • exported a new utility/helper as @pyscript/core/service-worker that enables back what got lost via latest updates, due bloated and slower code around it for all browsers, not just Safari
  • tested everything works as expected

Checklist

  • I have checked make build works locally.
  • I have created / updated documentation for this change (if applicable).

@WebReflection
Copy link
Contributor Author

Copy link
Member

@ntoll ntoll left a comment

Choose a reason for hiding this comment

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

LGTM - needs @JoshuaLowe1002 to confirm it works with Edublocks.

@WebReflection
Copy link
Contributor Author

we just double checked and after latest amend I think we're good to go, right @JoshuaLowe1002 ?

Copy link
Member

@JoshuaLowe1002 JoshuaLowe1002 left a comment

Choose a reason for hiding this comment

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

Yep! Approved. Thanks @WebReflection

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.

3 participants