1360870 - Implement "module" service workers

Status: NEW

(Core :: DOM: Service Workers, enhancement, P3)

Description

[Ben Kelly [:bkelly, not reviewing]]

The spec has support for loading service workers built on modules instead of classic importScripts().  See things that reference:

https://w3c.github.io/ServiceWorker/#dfn-type

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Ben, feel free to comment if you think this should be prioritized and need resource to work on it.

Comment 3

[alancutter]

It might be helpful to error and link to this bug when users attempt to navigator.serviceWorker.register(url, {type: 'module'}) to make it more clear that the implementation of this is not complete. This is Chrome's current behaviour.

Comment 4

[Yulia Startsev [:yulia]]

Attachment: WIP: Bug 1360870 - wip service workers

Depends on D156103

Comment 5

[Yulia Startsev [:yulia]]

I'll update the in progress patch, but here is the current state:

the patch largely works, however we are breaking certain constraints: Specifically,

• we need to disable top level await for service workers via a flag that is passed to spidermonkey
• we need to disable dynamic import once it lands.

Otherwise we are passing a good chunk of the tests here.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

It seems like this shouldn't be too hard to do, going to take the bug although this will not be immediate and it may end up making sense to fix bug 1808685 as part of our tech debt cleanups first. (Note that the landed patch was a disabling of a prior attempt to do that, not a fix.)

(In reply to Yulia Startsev [:yulia] | Back in Oct. from comment #5)

I'll update the in progress patch, but here is the current state:

the patch largely works, however we are breaking certain constraints: Specifically,

• we need to disable top level await for service workers via a flag that is passed to spidermonkey

Here's the CompileOptions flag to set.

• we need to disable dynamic import once it lands.

Yulia already implemented the check to bail out in this case, although it's currently after we do the work to create the dynamic loader, which is weird, but the initial landing state was that way per the diff for that section.

Comment 9

[John Renner]

Hey I'm interested in pushing this forward. Is it necessary to fix 1808685, or can we replicate Yulia's change on top of current HEAD and it should be mostly there?

Comment 10

[John Renner]

Attachment: WIP: Bug 1360870 - Prototype implementation

Comment 11

[John Renner]

Took a crack at implementation and was able to run it against a local app (with module serviceworkers and import statements) I've been working on!

I'm assuredly missing some implementations of equality checks and things like that, so it would be helpful to get someone who knows the surrounding code to take a look.

I'd love to pull in the related WPT tests for service worker modules, but I'm not familiar with how to do that.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Thanks for trying your hand! I think the main things that are missing are disabling top-level await and the ServiceWorkerRegistrar persistence which requires a schema version change to the file. As a heads-up, we have a few in-flight priority patch stacks right now that are likely going to bit-rot some of the plumbing in the patch over the next 1-2 weeks.

Comment 13

[John Renner]

Awesome. Could we get away with sticking the new field at the end of the serialization and default to ModuleType::Classic if we hit the terminator early? (or would that constitute a version change anyway).

No worries about the bitrot. I won't rest until every -Info -Data -Registrar -Parent and -Child has a WorkerType aType in its constructor.

Comment 14

[Yulia Startsev [:yulia]]

thanks for taking this over!

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

(In reply to John Renner from comment #13)

Awesome. Could we get away with sticking the new field at the end of the serialization and default to ModuleType::Classic if we hit the terminator early? (or would that constitute a version change anyway).

That would constitute a version change anyway, because the logic that makes sure the registration terminator is where we expect it would discard the registrations anyways on (unsupported[1]) downgrade.

A nice part of bumping the rev is that we can also then potentially write "module" or "classic" adjacent to the cache UUID that makes the file format a little bit more human readable than just stashing it as a new last line.

1: So although there is a defensive build id ratchet on profiles now that yells at users if they try and downgrade, we do try and "handle" downgrades so that people doing bisection or having to roll-back at least won't get into super-weird states.

Comment 16

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 17

[Peter van Hardenberg]

Hey there -- I just wanted to voice some support for this issue. I've been working on building a cross-platform service worker for a library called Automerge which works across the Blink family of browsers, Safari, and Firefox. Unfortunately, Firefox (in service workers) is the odd platform out right now and my users are complaining. I'd really really like not to open the can of worms that it would entail to develop and test yet another version of the library that works cross platform that is only for service workers based on importScripts, particularly since there seems to be some energy around landing this feature in Firefox.

I'm happy to test builds, and provide validation (technical or emotional) if it helps get this patch landed.

Comment 18

[Erich Gubler [:ErichDonGubler]]

WebGPU CTS uses module workers for coverage of service workers. Also, most service worker demos I'm aware of for WebGPU use module scripts. This makes deployment of service workers for WebGPU applications in Firefox require additional work, and hurts our story for Firefox as a developer's primary developer browser.

Comment 19

[Bob Hood [:bhood]]

The WebGPU project and our internal ML team has blockers that are (now) depending upon this. Additionally, we have people at huggingface.io who are also wanting to have this available--a vast majority of the service worker dependencies they have are with module workers. Can we get this issue resuscitated and its priority elevated?

Comment 20

[John Renner]

Apologies for not coming back to this (despite an admittedly my bold declaration above). I continue to not have time amongst the many responsiblities of life. The change shouldn't be too bad for someone with knowledge of the code since most of the underlying support in the runtime is there, it's mostly just threading the flag.

Comment 21

[Peter van Hardenberg]

Just a note that over here at Ink & Switch we're building local-first projects that rely heavily on Service Workers and don't support Firefox because of this missing feature. We have had a few sad folks as a result and I'd love to have Firefox support.

Comment 22

[Joshua Lochner]

Hi all! I'd also like to voice support for this issue. I'm the lead maintainer of Transformers.js, a JavaScript library for running ML models in the browser, and many of our users (and those testing our demos) have mentioned that they often run into browser issues when using Firefox. One of which, is this one: module support in service workers. So, it would be great to have this working!

We've released a bunch of demos that can help with testing; hopefully they will be useful:

• Online demos
• Source code
• Posts on reddit (really great user feedback, including comments)