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

Add no-cache header to figwheel server #464

Closed
wants to merge 1 commit into from
Closed

Add no-cache header to figwheel server #464

wants to merge 1 commit into from

Conversation

tonsky
Copy link
Contributor

@tonsky tonsky commented Jul 26, 2016

I had an issue when Edge used stale js files instead of checking for new ones. I think it's reasonable to add no-cache just to make sure. I've tested that locally and it solved my problem with Edge

@kommen
Copy link

kommen commented Dec 2, 2016

I think I just ran into the same issue with Chrome. Had some really strange issues until I disabled caches in the Chrome dev tools and then everything worked as expected.

@frickm
Copy link

frickm commented Apr 15, 2017

just had the same issue - it shows itself when you reload and get old stuff, but after making an arbitrary change to the source the latest js-files get pushed to the browser and everything looks correct.

@pesterhazy
Copy link
Contributor

I'm seeing the same symptoms in Chrome 59. If I make a change, figwheel injects it correctly. But after hitting reload, I see the old stale file before the change. To fix this, I need to Shift-click on the Reload button.

In the Network Tab I see "200 OK (from memory cache)" or "200 OK (from disk cache)". In fact it looks like chrome has changed recently and caches more aggressively these days.

I think adding no-cache headers as @tonsky proposes would be a great solution for this. @bhauman are you not seeing this issue in Chrome?

@danielcompton
Copy link
Contributor

I see this behaviour often in Chrome also as another datapoint.

@bhauman
Copy link
Owner

bhauman commented Jun 26, 2017

OK so I'm assuming that this is happening for several reasons:

Chrome Devtools no caching is enabled and Devtools is not open OR Devtools/Chrome has a hard time disabling caching for <script> tags that are injected by google closure in certain circumstances.

Also Figwheel more than likely isn't setting the Last-Modified or Etag headers correctly when it serves static assets.

We need to check to see if this is the case and then fix that first.

Why I don't want to enable no-cache for everything:

The ratio of files that change to files that don't change is very very small especially for medium and larger sized projects. Forcing "no-cache" on everything can create an unbearable load times for some projects because under optimizations none there are a large number of files loaded that don't change ever.

@pesterhazy
Copy link
Contributor

I just tried reproducing this in a fresh lein new figwheel project and couldn't. The "reload" button seems to work fine here. So I think you're right that this happens only in certain circumstances.

FWIW, the problems I've had were with compiled output, not static assets. And it does look like the Last-Modified header is set.

My take on this is that Chrome's behavior was changed recently to cache resources (especially scripts?) if no explicit caching headers were sent. Surprisingly, it may not even attempt to revalidate the files if they're in cache locally.

Quoting from https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.4:

Unless specifically constrained by a cache-control (section 14.9) directive, a caching system MAY always store a successful response (see section 13.8) as a cache entry, MAY return it without validation if it is fresh, and MAY return it after successful validation.

This Chrome blog post may be relevant:

To improve the stale content use case, Chrome now has a simplified reload behavior to only validate the main resource and continue with a regular page load. This new behavior maximizes the reuse of cached resources and results in lower latency, power consumption, and data usage.

Maybe part of the solution is to specify the "must-revalidate" header?

https://stackoverflow.com/questions/34761700/does-chrome-violate-the-standards-in-caching

@pesterhazy
Copy link
Contributor

... of course a foolproof variant would be to append a date/version number/hash to each js filename (query string?), but not sure if that's something you'd consider

@reitzensteinm
Copy link

This is a frequent source of bugs for me in Chrome. I definitely support appending a truncated file hash to name proposal; it's the only foolproof way to ensure you've got a clean version of the latest code, no matter what kind of badly implemented cache is sitting between the javascript engine and the files on disk.

@bhauman
Copy link
Owner

bhauman commented Jun 26, 2017

When I say "static assets" I mean the compiled js/html etc, as these don't change in the deployed application.

Appending a hash would require a compiler change and that is a tall order.

@bhauman
Copy link
Owner

bhauman commented Jun 26, 2017

So maybe we should set Cache-Control: must-revalidate, if the last-modified is being set correctly, and see if that alleviates this problem.

@tonsky
Copy link
Contributor Author

tonsky commented Jun 26, 2017

My vote goes for hash in filename (if possible) or for completely disabling cache. Browser has to re-validate files anyways, it doesn’t have all the necessary information to cache intellegently on its own. Maybe I don’t understand something, but is going for file, reading its content to calculate its ETag and sending back Not modified faster than fetching the file anew? Given that it all happens locally? We have 11kloc project with 85 CLJS files and I always keep my cache disabled, haven’t noticed any problems. Fighting another non-issue caused by stale cache doesn’t simply worth it for me. If it can be done fast and right (hashes for filename, force to reload), I’m in for it. If it can’t be done, I’m for “right” approach (disabling cache) over “fast but sometimes wrong”

@bhauman
Copy link
Owner

bhauman commented Jun 26, 2017

I'll take an updated PR for Cache-Control: must-revalidate if that sounds good. Thanks for chasing down those resources @pesterhazy.

@bhauman
Copy link
Owner

bhauman commented Jun 26, 2017

Oh yeah, I'm not thinking about must-revalidate correctly.

@pesterhazy
Copy link
Contributor

Digging in some more, it looks like Cache-control: no-cache might actually be what we're looking for. Despite the name what this does is to force server-side revalidation. If a cached copy exists, the User Agent sends a network request to validate the resource (using the If-Modified-Since header). If the resource is still valid, the server will respond "304 Not Modified" and the UA will use the locally cached version.

This is different from Cache-control: no-store, which actually prevents local caching. Confusing names for sure. [This link] explains it well (section "mutable content, always server-revalidated"). Chrome 58's behavior seems to be to assume that, unless otherwise specified, non-entry resources are immutable, which is not correct for Figwheel.

So if that's true, Cache-control: no-cache, must-revalidate should be safe. (Not sure about the "Expires" header mentioned in the PR.)

danielcompton added a commit to danielcompton/lein-figwheel that referenced this pull request Jul 24, 2017
Cache-Control: no-cache tells the browser that it may cache the
response, but it must always revalidate the cached response with the
server, before using it. This is equivalent to setting

Cache-Control: max-age=0, must-revalidate

max-age=0 means that the response is always stale, and must-revalidate
means that the client must always revalidate stale resources.

Expires: -1 is not needed, all modern browsers support Cache-Control.
IE8 (and below?) treat 'Cache-Control: no-cache' as 'no-store', meaning
that they will always revalidate the cache. That seems a small price to
pay for the simpler reasoning about caching by only using Cache-Control.

https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching
has good information on the current state of the art for HTTP caching.
https://jakearchibald.com/2016/caching-best-practices/#pattern-2-mutable-content-always-server-revalidated
has a good explanation of exactly the caching pattern that we are
looking to employ.

See also https://stackoverflow.com/a/19938619/826486 for more discussion
on Cache-Control: no-cache.

Based on bhauman#464, but fixes merge conflicts, and removes Expires: -1.

Fixes bhauman#546.
@danielcompton
Copy link
Contributor

danielcompton commented Jul 24, 2017

For those following along, I've made a fresh PR in #586 which sets Cache-Control headers to force revalidation always.

bhauman pushed a commit that referenced this pull request Jul 25, 2017
* Add no-cache header to figwheel server

* Set Cache-Control: no-cache on responses

Cache-Control: no-cache tells the browser that it may cache the
response, but it must always revalidate the cached response with the
server, before using it. This is equivalent to setting

Cache-Control: max-age=0, must-revalidate

max-age=0 means that the response is always stale, and must-revalidate
means that the client must always revalidate stale resources.

Expires: -1 is not needed, all modern browsers support Cache-Control.
IE8 (and below?) treat 'Cache-Control: no-cache' as 'no-store', meaning
that they will always revalidate the cache. That seems a small price to
pay for the simpler reasoning about caching by only using Cache-Control.

https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching
has good information on the current state of the art for HTTP caching.
https://jakearchibald.com/2016/caching-best-practices/#pattern-2-mutable-content-always-server-revalidated
has a good explanation of exactly the caching pattern that we are
looking to employ.

See also https://stackoverflow.com/a/19938619/826486 for more discussion
on Cache-Control: no-cache.

Based on #464, but fixes merge conflicts, and removes Expires: -1.

Fixes #546.

* Return 304 when files haven't been modified

Figwheel will generate the response for the static file, and then the
wrap-not-modified middleware checks the response to see if the request
headers show that the client already has this version of the file. If
so, it will return a 304, rather than sending the whole file again.
@pesterhazy
Copy link
Contributor

This PR can be closed now that #586 is merged I think

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

Successfully merging this pull request may close these issues.

7 participants