sveltejs / sapper-template

Starter template for Sapper apps
703 stars 214 forks source link

Service worker would fail if accessing static assets with query string #274

Closed ehrencrona closed 4 years ago

ehrencrona commented 4 years ago

Fixes sveltejs/sapper#1587. The service worker would assume that is the request path matches an asset, then the asset is in the cache. It would then use CacheStorage.match to retrieve the request, but that would fail if the query string was different.

I don't think there's any reasonably simple way of writing a test for this unfortunately.

benmccann commented 4 years ago

Thanks for tracking this down. I think if a query string is specified we should not return the asset from the cache. E.g. some people use query strings for cache busting and this would break that. Perhaps we can just change the line above to skip the cache instead?

ehrencrona commented 4 years ago

I don't find it logical to just not cache URLs with a query string. I could see it being logical to cache the content of each URL separately if we want to support manual cache-busting. The disadvantage in both cases is that pre-loading and offline won't work, and they are kind of the point of having a service worker in the first place. I see the risk of developers not noticing that – for whatever reason – there's a query parameter in a request and that then makes the app not work offline.

Any idea why this behavior seems to have changed since 0.27 and how it used to behave? The service worker code is unchanged since February.

ehrencrona commented 4 years ago

superseded by #277