Open Bug 1360870 Opened 8 years ago Updated 12 days ago

Implement "module" service workers

Categories

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

enhancement

Tracking

()

People

(Reporter: bkelly, Unassigned, NeedInfo)

References

(Blocks 5 open bugs)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

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
Ben, feel free to comment if you think this should be prioritized and need resource to work on it.
Priority: -- → P3

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.

Blocks: 1775574
Severity: normal → S3

Depends on D156103

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.

Duplicate of this bug: 1775574
No longer duplicate of this bug: 1775574
Blocks: 1807919

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.

Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Duplicate of this bug: 1884257

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?

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.

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.

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.

thanks for taking this over!

(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.

Assignee: bugmail → john+bugzilla

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

Assignee: john+bugzilla → nobody
Status: ASSIGNED → NEW

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.

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.

No longer blocks: 1938663

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?

Flags: needinfo?(smaug)

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.

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.

Flags: needinfo?(smaug) → needinfo?(jstutte)

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:

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: