ilan-schemoul / meteor-service-worker

An universal service worker for meteor apps
MIT License
137 stars 26 forks source link

SVG's not cached #7

Closed ramezrafla closed 7 years ago

ramezrafla commented 7 years ago

But images (such as png and jpg) are .. any guidance would be appreciated as we can PR this repo if you want.

ramezrafla commented 7 years ago

Upon further checking, we are not sending down cache-control header, investigating ...

ramezrafla commented 7 years ago

It was indeed a cache header issue - sorry for the false alert. Closing.

ilan-schemoul commented 7 years ago

Hello, no problem :

errare humanum est

ramezrafla commented 7 years ago

Thanks! Any idea how we can integrate with Reload to clear cache upon migration? That's what's needed to close the loop (we also need to document cache settings in the README)

ilan-schemoul commented 7 years ago

Why do you need to clear cache ? What loop ? The Readme.md may be improved indeed, too much pointless/unclear informations IMO and SW is not necessarily well documented so yeah it may be improved. When you're talking of "cache settings", are you talking of headers ?

ilan-schemoul commented 7 years ago

If CSS/JS is updated at any moment the hash of the URL included in the HTML will change so the SW deletes the old version to replace by the new JS/CSS.

ramezrafla commented 7 years ago

Yes, I am talking about headers so people know what to do with their nginx / apache setup.

In terms of clearing the cache, imagine this: we set cache expiry at 30d. Then we update the app twice over a period < 30d. You would have data saved in the cache that is no longer used (2 outdated and 1 in use), waiting for cache expiry. Ideally, there is a mechanism to clear the memory, especially for large apps.

Case # 2, say we changed the content of an image, but keep the same name (may be needed in dynamic templates), we need a mechanism to clear the cache for that image to download the new one.

ilan-schemoul commented 7 years ago

Case 1

Ideally, there is a mechanism to clear the memory, especially for large apps.

It's a bit unclear, how could one implements a mechanism to clear outdated assets. The SW catches requests but don't know what's been removed or not, you would like a parameter that one can set to X days so the SW checks for files stocked since more than X days and remove them from Cache ? That could be an idea to explore for sure.

Case 2 : an image has changed but not the name. Two possibility :

First idea

You adopt in your workflow a tool that add an hash (file.png?hash=XXXX) and in counterparts the SW improves by adopting JS/CSS system to other assets so it updates files with a different hash than previously.

Pros :

Pros :

Globally I think it's a better idea when it comes to caching to use hash, so the SW is more precise and take really when needed the information needed. So in this case we're really in an optimized, close from native, approach.

Feel free to make PR for the Readme.md, or for the SW.js if you feel like there's a missing part. In anyway PR always open interesting discussions about how to implement ideas, to have multiples ideas on a subject.

ramezrafla commented 7 years ago

You adopt in your workflow a tool that add an hash (file.png?hash=XXXX) and in counterparts the SW improves by adopting JS/CSS system to other assets.

Right, this is the standard way, and probably least-risky.

Here is an idea for you. We no longer use 'MSW V0.3' as the cache name, but a string that is unique for each build. When migration happens, the previous build is cleaned up via cache.delete() and a new cache is started. I haven't figured out yet how to pass a variable to sw.js from the main thread. Any ideas?

ilan-schemoul commented 7 years ago

I still do not get why you could want to delete every single files when you app migrates as each time a JS/CSS file changes it's deleted and that if you want to change images but keeping the same name you can use the hash system we're agree to implement.

The only way I know to communicate from the main thread to the SW is http://craig-russell.co.uk/2016/01/29/service-worker-messaging.html#.WHt-lRvhAvg It's a rather not concise API so one may want to use the package Swivel but I'm still unsure about how to include a package to a SW as we cannot use Meteor's workflow to compile SW for now