-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Updates to reflect service worker registration being opt-in. #3924
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
(Re: the CLA, it has been previously addressed outside of the normal submission process.) |
@@ -74,8 +74,8 @@ You can find the most recent version of this guide [here](https://github.com/fac | |||
- [Getting Started with Storybook](#getting-started-with-storybook) | |||
- [Getting Started with Styleguidist](#getting-started-with-styleguidist) | |||
- [Publishing Components to npm](#publishing-components-to-npm) | |||
- [Making a Progressive Web App](#making-a-progressive-web-app) | |||
- [Opting Out of Caching](#opting-out-of-caching) | |||
- [Making a Progressive Web App](#making-Making a Progressive Web Appa-progressive-web-app) |
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.
Oops
|
||
>Note: In the current version of `create-react-app`, opting-in is required for | ||
all developers who want offline-first behavior, even if you've previously | ||
deployed a version of your web app which used a service worker. If you redeploy |
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.
I don't think this is the right place to say redeploy because the opt-in only affect the people who generate a new project with CRA. The people that are upgrading to v2 from existing v1 generated project would still have the old registration code. (if they don't also update the file manually, it is)
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.
Ah, so the local src/index.js
is left alone when someone updates create-react-app
from v1 to v2? I've got to be honest that I'm not familiar with the upgrade process 😄
If that's the case, I'll update the text. Thanks!
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.
Yes, all files in src
are left alone when upgrading. "Upgrading" is really just updating a dependency in package.json
. The template code in src
is only used when creating a new project.
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.
usually, there would be a guide in the release like https://github.com/facebook/create-react-app/releases/tag/v1.0.8 to migrate the files in src, which I think is the more appropriate place for it.
@jeffposnick I think it'll be more appropriate if I'd close #3870 and you'd handle the situation in one PR (revert of #3419 and updating the docs) if that's ok with you. I was about to resolve conflicts in my PR but this would just create new ones in this one. |
@piotr-cz—I can handle that all in a single PR, sure. |
@jeffposnick Thanks, I've closed #3870 |
I've moved @piotr-cz's changes from #3870 (reinstating I've also added a config option to disable |
Hey guys - any update on this PR? |
Hey everyone, any estimate when this will be merged? |
…#3924) * Updates to reflect service worker registration being opt-in. * Fixed an anchor link. * Updates to SWPrecacheWebpackPlugin config, and corresponding docs.
R: @gaearon @Timer etc.
Fixes #3864
There are some changes to the docs needed if/when
navigateFallback
is re-enabled in the service worker configuration, but those are tracked in #3870 and I'm guessing that @piotr-cz will tackle them?