Byte by byte match does not capture developer friendly behaviour
nikhilm opened this issue · 11 comments
The spec currently only updates a SW if "response is a byte-for-byte match with the script of newestWorker, then: "
But if my SW is:
importScripts("actualLogic.js");
I'd like the browser to consider changes to actualLogic.js as an update. The spec should include importScripts() in the update condition.
Note that this adds side effects. Detecting importScripts() will require running the worker, so any mutation code inside the SW that is not in an event handler, but in the global scope, will run, even if the worker later gets thrown away before being installed.
Conversely, I've seen devs use this behaviour deliberately. Eg:
importScripts("actualloglc-v1.js");The reason they've done this is to "keep the update check really small". I'm not a fan of this as etags are a better fit, but it is what devs are doing.
In the case of:
importScripts("//other-origin/third-party.js");…I'm not sure I want the third party to decide when a new version of my SW should be created.
@nikhilm in your example, what's the benefit of the import vs just including the script?
Detecting importScripts() will require running the worker
Implementations currently cache all the imported scripts on first run. We need to spec that. But that gives us a list of scripts to check if we wanted to do that.
We could give access to a private Cache object that represents the SW scripts itself and any imported scripts. We had this in the initial design, but I just removed it from the spec because it was too hand-wavey. Developers could update stuff as needed.
@nikhilm in your example, what's the benefit of the import vs just including the script?
Well this was meant as a pathological example. The intention is if I've some shared code in my SW and in the page or somewhere else, and I make a change to it, should the SW itself be updated?
The reason they've done this is to "keep the update check really small". I'm not a fan of this as etags are a better fit, but it is what devs are doing.
So is the solution to disable caching completely when using devtools or something so that devs can change imported scripts and see the changes?
Contra @jakearchibald, I think the pattern of a single importScripts() call with a versioned URL is actually quite good. It removes the requirement for figuring out etags (which is seriously onerous for most developers).
I REALLY dislike the notion that we're going to spider importSrcripts() when checking for freshness. I'm not sure why it improves the situation for anyone.
Solving this in devtools for developers feels superior in every way.
I like the simplicity of only checking the SW script for modifications.
Also, if we need to change in the future, we can relatively easily add checking of dependent scripts without breaking back compat. Going the other way is harder.
Perhaps we can revisit this when es6 modules enter the picture.
At Pushpad we would really need to have a way for our customers to import our script in their service worker and have it automatically updated when it changes. I think that our use case of importScripts is a quite common scenario that needs a solution (I've seen that most of the other BaaS make the same use of importScripts).
I think that the best solution is to check for updates (e.g. byte to byte comparison) in all the importedScripts.
Implementations currently cache all the imported scripts on first run.
Just to clarify, is this already specifyed? Not sure if this is the script resource map record.
Just to clarify, is this already specifyed? Not sure if this is the script resource map record.
Yes, its monkey patched here:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#importscripts