-
Notifications
You must be signed in to change notification settings - Fork 507
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 section why cache limitation is necessary with examples #70 #115
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! There's a lot of material to cover, and this tends to be tricky to explain, so I've added some suggestions about rephrasing or linking to other guidance.
README.md
Outdated
## What's not Included | ||
|
||
* You'll need to configure your web server to add HTTP headers to prevent caching of critical service worker files. | ||
If you don't do this, you might end up with browsers that never update the cached content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not (quite) as bad as that.
Can you update the "never" to say that there might be a delay of up to 24 hours? And link to https://stackoverflow.com/questions/38843970/service-worker-javascript-update-frequency-every-24-hours/38854905#38854905
README.md
Outdated
When you run the production build and deploy the application to a web server, the browser will call the service worker on the first visit and will download and cache all resources. | ||
When the browser accesses the site a second time, it will use the cached resources. | ||
To notice changes in your app it is crucial that the browser downloads the latest version `service-worker.js` from your server. | ||
The browser will only do this when the HTTP Header of the service worker indicates that a previous download has expired. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rephrase "HTTP Header" to "Cache-Control header associated with service-worker.js
"
README.md
Outdated
The browser will only do this when the HTTP Header of the service worker indicates that a previous download has expired. | ||
If a new service worker is available, it will download the new contents in the background. | ||
Once the download is complete, the default behavior is that the new content will be shown on the next visit of the site. | ||
You can change the template's `service-worker-prod.js` to display the message to the user to reload the page, or issue a `window.location.reload()` automatically to trigger an update depending on your application's needs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
service-worker-prod.js
=> service-worker-prod.js
README.md
Outdated
You can change the template's `service-worker-prod.js` to display the message to the user to reload the page, or issue a `window.location.reload()` automatically to trigger an update depending on your application's needs. | ||
|
||
Depending on your setup, you might want to have a short caching time for your service worker or no caching at all. | ||
At minimum, you should add caching headers for `service-worker.js` and `index.html`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rephrase "you should add caching headers" to make it clear that the headers you add should disable caching?
README.md
Outdated
|
||
Depending on your setup, you might want to have a short caching time for your service worker or no caching at all. | ||
At minimum, you should add caching headers for `service-worker.js` and `index.html`. | ||
You should also add caching headers for all files that are in the `static` folder of this template. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeffposnick - Hello Jeff, thank you for the review. I've added the new content. I've also moved some of the content I added to README.md to a file docs/prevent_caching.md. Please review again. |
Thanks! |
This tries to document the necessary web server setup regarding caching. This should close #70