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

fix: Firefox Hot reload #934

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: Firefox Hot reload #934

wants to merge 7 commits into from

Conversation

ajpinedam
Copy link

closes https://github.com/unoplatform/private/issues/511

This PR fixes Hot Reload on Firefox by replacing the ES6 module import on the service-worker.js file. This functionality is not supported on Firefox yet.

@ajpinedam ajpinedam requested a review from jeromelaban December 4, 2024 16:17
@ajpinedam ajpinedam self-assigned this Dec 4, 2024
@ajpinedam
Copy link
Author

@jeromelaban is there a way to use this fix locally?

Copy link

github-actions bot commented Dec 4, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

@ajpinedam ajpinedam marked this pull request as draft December 4, 2024 20:23
@ajpinedam
Copy link
Author

Marking as draft, did some local test and I believe there's more to do

@jeromelaban
Copy link
Member

@ajpinedam it's testable using the staging site. It shows this:

image

@ajpinedam
Copy link
Author

@ajpinedam it's testable using the staging site. It shows this:

image

Thanks. Yes that's the error I saw while testing using a demo local script.

We will need to add a build rule so that the unoConfig.ts also creates a non-module version of the file.

importScripts() does not support modules.

Copy link

github-actions bot commented Dec 5, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

@ajpinedam
Copy link
Author

To avoid making crazy changes that could break other things, I pushed a change to create a duplicate uno-config js using traditional JS instead of a module, removing the need to use import. This file is used solely on the service-worker.js when registering for the Service Worker.

@ajpinedam
Copy link
Author

It seems to be working. At least the Service Worker registration is happening. I will test other browsers and PWA in a moment.

##Firefox

image

Copy link

github-actions bot commented Dec 6, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

Copy link

github-actions bot commented Dec 6, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

@ajpinedam
Copy link
Author

ajpinedam commented Dec 6, 2024

Firefox was working, but Edge and Chrome stopped.

This new change separates the logic between module and classic registration using a fallback since the browser API does not provide a way to know if the module registration is available (w3c/ServiceWorker#1582).

So we will try to register with module, edge, and chrome will pass, and if that fails, we will fall back to register with classic; this is the case for Firefox. With this only Firefox will see a little performance hit since we will try register twice.

Firefox Edge
image image

@ajpinedam ajpinedam marked this pull request as ready for review December 6, 2024 06:00
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://white-river-0087b630f-934.eastus2.1.azurestaticapps.net

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.

2 participants