Skip to content
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

Byte by byte match does not capture developer friendly behaviour #639

Closed
nikhilm opened this issue Feb 25, 2015 · 11 comments
Closed

Byte by byte match does not capture developer friendly behaviour #639

nikhilm opened this issue Feb 25, 2015 · 11 comments

Comments

@nikhilm
Copy link
Contributor

nikhilm commented Feb 25, 2015

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.

@nikhilm
Copy link
Contributor Author

nikhilm commented Feb 25, 2015

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.

@jakearchibald
Copy link
Contributor

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?

@jakearchibald
Copy link
Contributor

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.

@jakearchibald
Copy link
Contributor

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
Copy link
Contributor Author

nikhilm commented Feb 26, 2015

@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?

@nikhilm
Copy link
Contributor Author

nikhilm commented Feb 26, 2015

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?

@slightlyoff
Copy link
Contributor

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.

@wanderview
Copy link
Member

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.

@nikhilm nikhilm closed this as completed Feb 27, 2015
@collimarco
Copy link

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.

@delapuente
Copy link

delapuente commented Jul 15, 2016

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.

@wanderview
Copy link
Member

wanderview commented Jul 15, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants