MystenLabs / walrus-sites

Walrus Sites: Decentralized Websites using Sui and Walrus.
https://docs.walrus.site/walrus-sites/intro.html
Apache License 2.0
10 stars 7 forks source link

Better filename for the Service Worker script #95

Closed kkomelin closed 2 weeks ago

kkomelin commented 2 weeks ago

Problem/Motivation

Walrus Sites routes requests from Web2 to Web3 using a Service Worker script, called sw.js, which is a very typical name for service worker scripts in many frontend frameworks. It means that if an app provides its own service worker script sw.js, it may cause some conflicts theoretically (I didn't test it).

Proposed Resolution

To avoid any conflicts or user confusion, I suggest renaming the Walrus Sites service worker from sw.js to something more specific, for example walrus-sites-sw.js.

An example is the Firebase Messaging Service Worker script which is normally called firebase-messaging-sw.js. Firebase Docs Reference

giac-mysten commented 2 weeks ago

Hey @kkomelin , thanks for raising the issue. Keep them coming :)

It means that if an app provides its own service worker script sw.js, it may cause some conflicts theoretically (I didn't test it).

This makes sense. In general, it may be hard to use service workers with Walrus Sites. The reason is that only one service worker can be installed per scope, and we install our service worker in the root scope /. Therefore, not to break the Walrus Site functionality, the site can only use more specific scopes.

Regardless, let me change the name. I guess someone may come up with a good way of using scoped SWs that do not break functionality.

kkomelin commented 2 weeks ago

No problem @giac-mysten I'm giving Walrus Sites deployment option to the users of my starter, so I'm kind of interested to make it work.

As for SW scopes, good to know. Yeh, well, it's a potential problem. Most people suggest merging multiple service workers into one like so https://mswjs.io/docs/recipes/merging-service-workers but if you don't have control over user scripts, you need to instruct users to do it and it complicates things a lot.

Anyway, I will let you know if find another solution. Thanks.

mlegner commented 2 weeks ago

@kkomelin It's really great to see you embracing Walrus! 🎉

Just want to emphasize to prevent any surprises and problems later: The current Walrus deployment is still a devnet, and we may want/have to wipe it repeatedly due to problems, breaking changes, etc., potentially even without warning. So please don't rely on it for any production deployments yet. 🙂

kkomelin commented 2 weeks ago

@mlegner Thanks for the note! It's good to know because I thought it's on testnet already. Anyway, I added a disclaimer to my documentation that Walrus is in active development, so I should be safe.

kkomelin commented 2 weeks ago

@giac-mysten @mlegner May I ask you guys to add the note about that Walrus Sites cannot be currently used for Progressive Web Apps to the limitations section of the Walrus documentation?

kkomelin commented 2 weeks ago

Thanks.

@giac-mysten @mlegner May I ask you guys to add the note about that Walrus Sites cannot be currently used for Progressive Web Apps to the limitations section of the Walrus documentation?

Okay, I will warn my users myself

giac-mysten commented 2 weeks ago

@kkomelin will add a note in the "known restrictions" section. Added an issue to the walrus-docs repo (#84)

kkomelin commented 2 weeks ago

Thanks @giac-mysten!