Support module service workers, and update for ES6

[domenic]

See https://github.com/whatwg/html/pull/608 which is now merged.

The essential work items are, I think:

• Add WorkerType type = "classic"; to RegistrationOptions
• Updated https://github.com/slightlyoff/ServiceWorker/pull/828 to include a type parameter (@mkruisselbrink)
• Update the registration process to more closely parallel run a worker and its callers e.g. the Worker constructor
  • I haven't looked into the details of this very much because I'm not too familiar with the existing registration algorithm. I guess it's run a service worker that needs the most updating?
  • It seems likely this will require modifications to HTML to allow "fetch a module script tree" to disallow cross-origin requests. Let me know exactly what they are and I can do that. My guess would be you need something like "Fetch a same-origin module script tree given url and settings object."
• Maybe add an example of using the module version, similar to https://html.spec.whatwg.org/#module-worker-example

This seems relatively high priority since as per https://github.com/whatwg/html/pull/608 module and module worker implementation is starting quite soon for multiple implementers, and I imagine they'd want to do service workers at the same time as other workers.

[jungkees]

Yes, I think the items you pointed would cover most of them. For SW, fetching a module script should be done in Update algorithm where the actual service worker script fetch occurs. I'll work on it and ask help where changes in the hooks are needed.

@mkruisselbrink I'll update the current text and try to merge the changes when the bikeshed conversion is completed. If you have a better plan, let me know.

[jungkees]

Working on this in worker-module branch: 9ddeabc4ff13c57078542f2c76c21e4e2c55ce16

Run Service Worker update is mostly done. Fetching scripts depending on the worker type is not done yet.

[slightlyoff]

I think I've got some concerns with the general design of @domenic's module workers proposal. I guess we can push the "type" thing through in a bunch of places as a one-off, but I'd like to have @mkruisselbrink's view on how this will work with header-based registration for foreign fetch.

[annevk]

Link: <sw.js>; rel=serviceworker; type=module?

[domenic]

It seems exactly the same as scope.

[mkruisselbrink]

That mostly seems okay, except that both the Link header and the <link> element already have a type attribute, which must be a mimetype. So we probably shouldn't call this type in the header/element. Not sure what to name this instead, but I'm sure we can come up with something sensible.

[domenic]

@mkruisselbrink for <script> we redefined type to not be a mimetype. Not sure if that's an option here.

[mkruisselbrink]

@domenic I don't see any technical reason why we couldn't do the same with type for link elements and headers. Just no idea how hard it would be to actually update rfc5988 to make that change for headers.

[mkruisselbrink]

Any reason module scripts aren't just a different mime-type from regular javascript though?

[domenic]

It doesn't really buy us anything, and requires everyone to change their server configurations and MIME type detection libraries (which impacts a lot more than just browsers). All downside, no upside.

[annevk]

For HTML we could definitely define what type means based on what the rel value says. I think the Web Linking RFC should allow the same, but I guess technically it doesn't. An alternative would be to use a module attribute or some such that you just leave empty.

[jungkees]

@domenic, there are two points that I'd like to discuss for the hooks.

It seems likely this will require modifications to HTML to allow "fetch a module script tree" to disallow cross-origin requests. Let me know exactly what they are and I can do that. My guess would be you need something like "Fetch a same-origin module script tree given url and settings object."

• SW's script fetch starts at Update step 5. A cross-origin script is not allowed and such URL should have been already filtered out (in Register step 2) before invoking fetch a classic worker script and fetch a module script tree. (For the later updates upon a successful installation, the Update uses the registration for the same origin where the installation is done against.) Besides, we allow loading cross-origin scripts via importScript(urls) called in SW. So if we expect the same behavior with worker module, fetch a module script tree seems fine as-is at least about the origin restriction.
• For both fetch a classic worker script and fetch a module script tree, SW wants:
• To set request's destination to "serviceworker", skip service worker flag, redirect mode to "manual", set a special header Service-Worker/script, and set request's cache mode to "reload" under certain condition (see Update step 10). => Do fetch a class worker script and fetch a module script tree taking the worker type ("worker"/"sharedworker"/"serviceworker") and do the above things for service workers make sense?
• To get a response (as well as script) as the result of the fetch to check the response's cache state, MIME type (for the classic script case too), and Service-Worker-Allowed header. => Is it possible to change fetch a classic worker script and fetch a module script tree returning/exposing a response? Or should I just do prose?

[domenic]

A cross-origin script is not allowed and such URL should have been already filtered out (in Register step 2) before invoking fetch a classic worker script and fetch a module script tree.

Hmm, does this properly take care of a same-origin request URL redirecting to a cross-origin response URL? I guess maybe that is why you are setting the redirect mode to manual?

For both fetch a classic worker script and fetch a module script tree, SW wants:

Thanks for outlining these. Hmm, this seems pretty tricky.

At first I thought it would be better for the SW spec to definefetch a classic service worker script and fetch a module service worker script tree by copying the algorithms from HTML and then inserting the appropriate steps and changes. But then I realized that the changes will also need to modify "fetch a single module script" for the response stuff, which makes that probably not a good idea.

How about the following changes to HTML:

• "fetch a classic worker script" replaces is shared with destination.
  • If destination is "serviceworker", then either HTML or Fetch sets the redirect mode and Service-Worker/script header. @annevk, any reason not to have Fetch do this?
• "fetch a classic worker script" and "fetch a module script tree" get an additional optional cache mode parameter. You can then set this under the appropriate conditions.
• We also add destination to "fetch a module script tree". The fact that it is currently not present seems like a bug (we currently always set the destination to "subresource" even for module workers. It looks like "subresource" doesn't even exist anymore?)
• We add the checks for cache state, MIME type, and Service-Worker-Allowed in one of two ways:
  • Either HTML's "fetch a classic worker script" and "fetch a single module script" check them if destination is "serviceworker", or
  • Fetch adds these checks itself. @annevk, any reason not to make these intrinsic properties of fetching a service worker?

An alternative design would be:

• We still do destination for both algorithms. But, we don't do any special processing, in either Fetch or HTML, based on destination.
• We add "set up the request" and "process the response" hooks to the two algorithms. Then the SW spec would do all its special stuff in these hooks.

Also, I assume that we don't want to apply any of these customizations to import-ed files from the service worker? Or... do those need different customizations, like importScripts??

[jungkees]

Hmm, does this properly take care of a same-origin request URL redirecting to a cross-origin response URL? I guess maybe that is why you are setting the redirect mode to manual?

SW doen't allow redirecting to any URL when it fetches its own script resource. That's the reason we set the redirect mode to "manual" and reject the update promise when the response's status is not an ok status. I don't see any problem using HTML's fetch worker scripts in that aspect.

Also, I assume that we don't want to apply any of these customizations to import-ed files from the service worker? Or... do those need different customizations, like importScripts??

I don't think we want to apply any of those customizations in fetching the imported resources. But with regard to the criteria for checking if the resource has changed and needs a re-installation, we need a discussion. With importScripts currenlty, it checks (byte-by-byte match) only the main script resource. I.e., if a service worker has ever been successfully installed, it never re-fetches the impored scripts from the network but just uses the cached resources until it detects the change in the main script. For this, we're using a service worker's "imported scripts updated flag" (set upon successful installation in Install step 14).

However, with the "fetch a module script tree", the updated imported scripts will be fetched from the network regardless the changes in the main script. IMO, that should apply to the service worker too. I think run the module script will take care of the rest. But I guess we still don't want to include the imported script resources in checking the changes for the re-installation decision. /cc @slightlyoff @jakearchibald @mkruisselbrink @annevk

How about the following changes to HTML: An alternative design would be:

I think I'm fine with either way, but will figure out further. @annevk's opinion will help here.

[mkruisselbrink]

I would expect the "fetch a module script tree" to be only done once, the first time the script is loaded. After that the module map could be persisted just like the current script resource map, and whenever the worker is ran in the future we'd need to somehow prepopulate the module map again from this cache before executing the script.

And yeah, I agree that we don't want to include anything other than the main script to decide if we should update/re-install.

[jungkees]

I would expect the "fetch a module script tree" to be only done once, the first time the script is loaded. After that the module map could be persisted just like the current script resource map, and whenever the worker is ran in the future we'd need to somehow prepopulate the module map again from this cache before executing the script.

Ah.. We'll invoke "fetch a module script tree" in Update and "run the module script" in Run Service Worker. So, I think that's already true.

[jungkees]

But the thing is when the "fetch a module script tree" is invoked in Update the imported scripts will be updated anyway. So if we don't cache the imported scripts as we do with importScripts the subsequent Run Service Worker has a chance to run the same main script with the updated imported scripts. We'd have to decide which behavior we want: caching the imported scripts vs deferring to the module script loading behavior.

[annevk]

1. The redirect mode should be "error" for fetching service workers, not "manual".
2. I prefer not doing tight coupling of type/destination with other features of request inside Fetch. It's less modular. We haven't even done that for mode "navigate" (redirect mode is set to "manual" by HTML for that).

[domenic]

@annevk OK, so not modifying Fetch. Do you have an opinion on the "set up the request" and "process the response" hooks vs. baking it into HTML?

[mkruisselbrink]

caching the imported scripts vs deferring to the module script loading behavior.

My personal opinion is that we should keep the caching behavior we have today. So store the module map the first time a worker is executed and in the future recreate the worker with the same module map. I don't see why we'd treat es6 modules any different from importScripts when it comes to updates, caching etc.

[jungkees]

I don't see why we'd treat es6 modules any different from importScripts when it comes to updates, caching etc.

Because they execute in a different way. With importScripts(urls), the network fetches for the imported scripts are always triggered when the main service worker script is run. So I guess the major concern was the scenario where the UA is offline and Run Service Worker fails while loading such imported scripts again from the network. Some related discussion was here: https://github.com/slightlyoff/ServiceWorker/issues/106.

But with the module script, loading imported scripts is separate from running them. For SW, we actually invoke them from two separate call sites: Update and Run Service Worker. That is, when Run Service Worker invokes "run the module script" algorithm, the most update-to-date imported scripts (by Update) in the module map can be used. So unless otherwise we had any other reason not to allow the updates to the imported scripts, relying on the ES6 module behavior seems more reasonable to me.

[annevk]

@domenic I have lost sight of the network requirements of service workers. I would be happy to include most of the logic in HTML if that's feasible since that makes it easier to tweak this going forward, but I'm also happy putting some in each.

As for request's destination, we recently changed its design and I forgot to file a downstream issue.

[domenic]

After thinking about this a bit more, I think I will go with the "An alternative design would be:" approach. I meant to work on it today but seem to have run out of time. I hope to have it ready for you sometime tomorrow, @jungkees.

[jungkees]

@domenic, sounds good. Let's try with that approach.

[domenic]

I worked on this today and got stuck. The problem is, what do we do in the following situation:

<script type="module" src="sw.js"></script>

<script>
navigator.serviceWorker.register("sw.js", { type: "module" });
</script>

Currently, all module loads go through the module map, which explicitly makes sure that we only fetch them once. Because of this, the special preprocessing steps on the request and postprocessing steps on the response do not happen. That seems pretty bad.

I wanted to check what the desired behavior here was for the above example. I think we need to re-fetch sw.js, with appropriate modifications to the request (request's destination to "serviceworker", skip service worker flag, redirect mode to "manual", set a special header Service-Worker/script, and set request's cache mode to "reload" under certain conditions). And check the response similarly (cache state, MIME type, and Service-Worker-Allowed header).

There is another related example:

<script>
navigator.serviceWorker.register("sw.js", { type: "module" });
</script>

<script type="module" src="sw.js"></script>

There are two possible behaviors here IMO:

• If sw.js is a valid module but not a valid service worker (e.g. because of the Service-Worker-Allowed header), it re-fetches the module, which runs fine and and ends up in the module map.
• If sw.js is a valid module but not a valid service worker, the failure to load it as a service worker means it will never re-fetch the module.

I think the first of these choices is better.

What this essentially means is that the initial fetch needs to be decoupled from the module map.

Since this is pretty tied in to HTML, I think I will try again but this time doing all the extra work in HTML. I will report back on how it goes. Maybe HTML will need to grow "to fetch a service worker module script tree".

[mkruisselbrink]

I'm not quite sure I understand this. Doesn't every window and/or worker have their own module map? If so why would navigator.serviceWorker.register every use the module map of the window from which the register call was made? I would expect this to behave the same as other workers do (at least if I'm reading the spec right) and the service worker script would have their own module map.

[domenic]

Oh, interesting! I guess it depends on where the service worker spec was planning to call "fetch a module script tree" from. And it does indeed make more sense to call it from inside the service worker; that parallels how normal workers ... work.

OK, thank you for clearing that up! That makes this much, much easier.

[annevk]

Yeah, the module map shouldn't cross continent/agent/vat/world boundaries.

[jungkees]

Working on this: https://github.com/slightlyoff/ServiceWorker/commit/245f98dadddb5bfce5a62ca65a5c1ac47eb91d75

I believe it's mostly done but would like to review it myself on Monday before requesting a review.

[jungkees]

I updated the patch: https://github.com/slightlyoff/ServiceWorker/commit/9111303ebc163b046d3f1ae6267a747ddfde2b64

@domenic @mkruisselbrink @annevk, please review it if I've done a right thing to integrate with the changes in HTML.

[domenic]

Looks good to me, with the only issue I spotted being that validate the response should not take care of errored/non-ok responses, but instead should handle the fetch-a-script algorithms asynchronously completing with null.

[jungkees]

Thanks for the review. I merged the changes having addressed the comments: 6d9500e99e764c12b117c63d10107e094d243662, dc2101edbfb89c635d9534ec85aa326999318d5a and b2085fb260628e970f0188b8585163e7f88a3360