-
Notifications
You must be signed in to change notification settings - Fork 312
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
Support module service workers, and update for ES6 #831
Comments
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. |
Working on this in worker-module branch: 9ddeabc Run Service Worker update is mostly done. Fetching scripts depending on the worker type is not done yet. |
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. |
|
It seems exactly the same as scope. |
That mostly seems okay, except that both the Link header and the |
@mkruisselbrink for |
@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. |
Any reason module scripts aren't just a different mime-type from regular javascript though? |
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. |
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. |
@domenic, there are two points that I'd like to discuss for the hooks.
|
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?
Thanks for outlining these. Hmm, this seems pretty tricky. At first I thought it would be better for the SW spec to define How about the following changes to HTML:
An alternative design would be:
Also, I assume that we don't want to apply any of these customizations to |
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.
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
I think I'm fine with either way, but will figure out further. @annevk's opinion will help here. |
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. |
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. |
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 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? |
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. |
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: #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. |
@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. |
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. |
@domenic, sounds good. Let's try with that approach. |
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 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:
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". |
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. |
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. |
This allows more complicated callers, like service workers, to customize script fetching as necessary. See [1], and in particular [2]. [1]: w3c/ServiceWorker#831 [2]: w3c/ServiceWorker#831 (comment)
This allows more complicated callers, like service workers, to customize script fetching as necessary. See [1], and in particular [2]. [1]: w3c/ServiceWorker#831 [2]: w3c/ServiceWorker#831 (comment)
Yeah, the module map shouldn't cross continent/agent/vat/world boundaries. |
Working on this: 245f98d I believe it's mostly done but would like to review it myself on Monday before requesting a review. |
This allows more complicated callers, like service workers, to customize script fetching as necessary. See [1], and in particular [2]. [1]: w3c/ServiceWorker#831 [2]: w3c/ServiceWorker#831 (comment)
This allows more complicated callers, like service workers, to customize script fetching as necessary. See [1], and in particular [2]. [1]: w3c/ServiceWorker#831 [2]: w3c/ServiceWorker#831 (comment)
I updated the patch: 9111303 @domenic @mkruisselbrink @annevk, please review it if I've done a right thing to integrate with the changes in HTML. |
This allows more complicated callers, like service workers, to customize script fetching as necessary. See [1], and in particular [2]. [1]: w3c/ServiceWorker#831 [2]: w3c/ServiceWorker#831 (comment)
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. |
See whatwg/html#608 which is now merged.
The essential work items are, I think:
WorkerType type = "classic";
to RegistrationOptionstype
parameter (@mkruisselbrink)This seems relatively high priority since as per whatwg/html#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.
The text was updated successfully, but these errors were encountered: