When should "imported scripts updated flag" be unset?
jakearchibald opened this issue · 19 comments
importScripts has two behaviours. One where it fetches from the network and adds to its "script resource map", and another where it fetches only from the "script resource map".
This allows importScripts during initial execution to implicitly populate the cache while the service worker is not-active, then later work without the network by using this cache.
The spec flips the switch once install has completed, but @mattto and @wanderview say implementations flip the switch after the first synchronous run of the service worker script.
Should the spec match implementations?
I quite like the currently specced behaviour, as it means I can do something like this:
addEventListener('install', event => {
importScripts('heavy-script.js');
});
addEventListener('fetch', event => {
const url = new URL(event.request.url);
if (url.pathname == '/whatever/') {
importScripts('heavy-script.js');
event.respondWith(heavyThing());
}
});I can cache some heavy scripts, but avoid the cost of executing them on each service worker startup. I can take that hit only when I need it.
Does it make sense to do this? We should think about this in a world with import().
Note, we've had a gecko bug to implement this for over a year, but I was never sure if it was really desired:
Our thought flow over the discussion has been:
- No sync API should be allowed in SW, and
importScripts()is one. So, let's allow it only during install events: #730 - (1) prevents even the cached imported scripts from being executed in the later events. So, let's remove this restriction: #642 (comment)
(2) sort of assumed that a specific service worker version and its (cached) imported script resources should map 1:1. So, it currently sets the flag upon a successful install.
@mattto and @wanderview say implementations flip the switch after the first synchronous run of the service worker script.
This is a question about until when we should allow fetching the scripts from the network: either until the first SW run during an Update or until the successful install of a SW version.
In addition to that, #839 (comment) should be discussed in this thread as well - whether we should allow the imported scripts to run in async tasks or not. @jakearchibald's example above suggests a usage where cached imported scripts can be used in async tasks while offline.
Yeah, I think dynamic importing is useful, and probably more likely once import() lands.
Personally I'm inclined to keep importScripts() as a sync script eval thing only right now. Reasons:
- Async importScripts() is a complication for an already complicated piece of code.
- Its unclear how much devs will feel they need to risk janking startup of the SW which is off the main thread. I personally have not seen people clamoring for this yet.
- Browsers can minimize the impact by storing pre-compiled js for offlined scripts. So the real impact is mainly felt at update/install time.
- Nested dedicated workers would allow developers to explicitly move work (including eval of heavy scripts) out of the service worker into a separate thread. I think this is something we want for other use cases and it could potentially solve this use case as well.
- Its easier to allow install event importScripts() later if we get a lot of feedback from devs that its needed.
Its easier to allow install event importScripts() later if we get a lot of feedback from devs that its needed.
Hmm, although it's really difficult to feature-detect this.
@wanderview how do you feel about import()? Would you want that to enable just-in-time importing?
Well, if we want to support import(), yes we would need to add async loading. But import() will require additional work regardless.
I guess it comes down to what people expect to actually implement. If we keep the spec text that allows importScripts() until the end of install, is chrome going to actually implement it? Neither chrome nor firefox has implemented it in the last year. AFAIK no one has complained.
On reflection, I'm leaning toward allowing it only until the initial eval. AFAICT, the purpose of the byte-for-byte check is to examine whether it needs to continue to Install or not. If the spec changes to allow keeps allowing it until the end of install, what do we do with the byte-for-byte check for? When it comes to import(), I think the dynamically imported scripts should not be considered as the service worker script resources determining a version. WDYT?
EDIT: This reasoning is under the decision of #839 (comment).
@mattto pointed in #839 (comment) that my previous comment is not true. I agree that the byte-by-byte check for Install is orthogonal to the discussion of the timing of setting the flag.
I think dynamic imports are useful, but given that:
- browsers don't implement it
- it's sync
- no one's asking for it
…I'm happy for us to align the spec with browsers for importScripts. However, I think dynamic importing is going to become popular with import(), and we should support it there.
We could achieve this by allowing import() to go via the fetch event. Maybe that's the less magic solution?
We could achieve this by allowing import() to go via the fetch event. Maybe that's the less magic solution?
I don't think that would work if import() is called before the install event. Or if the script needs code from the import() in order to handle fetch events.
A general problem will be how to handle the race between import() async modules loading and life cycle events. I guess the most straightforward way to handle that would be for script to use import() in an install event so it can use waitUntil() to guarantee the load completes.
Still, I feel like that can be handled at a later time. I'd like to see import() ship on window and regular workers first.
I don't think that would work if import() is called before the install event.
Yeah, we'd have to come up with some rule there, like it goes to the network before activate.
A general problem will be how to handle the race between import() async modules loading and life cycle events. I guess the most straightforward way to handle that would be for script to use import() in an install event so it can use waitUntil() to guarantee the load completes.
Basically what the spec says for importScripts now?
Basically what the spec says for importScripts now?
Kind of. But in this future world someone could call import() at script eval time and it could finish loading after the install event completes. The "complete install" steps basically will need to have a check for any outstanding async import loads and fail install in those cases. Since we can't just throw from import().
Really I don't like this, though, because it could work fine in a fast local dev environment and then fail later in a slower production environment. Seems like a footgun we should try to avoid if we can.
Edit: Maybe we could make install completion wait until all outstanding import() loads are complete.
With #1021 (comment) and #1021 (comment), I'm also happy for us to align the spec with the implementations.
One thing I'd like to check with the current implementations is whether they don't even allow to get a script from the cache when importScripts() is called from within an async task.
With a simple test code:
self.oninstall = e => {
importScripts('importedscript1.js');
};
self.onfetch = e => {
importScripts('importedscript2.js');
};Both importedscript1.js and importedscript2.js execute (without a network error) in both Firefox and Chrome without importScripts() to those urls in the top-level script. I'm confused. This is not expected to be working according to what @wanderview and @mattto mentioned? Am I misunderstanding?
Very interesting @jungkees!
Here's an expanded test, and live demo.
Results:
| Test | Spec | Firefox | Chrome |
|---|---|---|---|
| Root import (first run) | Download, execute & cache | As spec'd | As spec'd |
| Root import (later runs) | Fetch from cache & execute | As spec'd | As spec'd |
| Install event import | Download, execute & cache | Downloads, executes, but does not cache | As spec'd |
| Uncached fetch event import | Throw error | Fetch from network | Fetch from network |
So I withdraw "I'm happy for us to align the spec with browsers for importScripts", since the browsers aren't particularly aligned. Also, allowing synchronous network fetches in functional events (which both browsers do) seems like a really bad idea.
Aligning to the spec seems like a better idea.
Maybe we could make install completion wait until all outstanding import() loads are complete
We don't do this for things like cache.addAll so why should we do it here? Devs should do:
import whatever from "./thing-i-always-need.js";
addEventListener('install', event => {
event.waitUntil(
import('./thing-i-sometimes-need.js')
);
});We don't do this for things like cache.addAll so why should we do it here?
Mainly because I wasn't aware the syntax made it easy like this. Works for me.