Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 26.9k
PWA-ification, via SWPrecacheWebpackPlugin + manifest.json#1728
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
828938675088abbfed21528351052dc418b21d14bba681e91a9fb9061cdb4cd4e3ede486d590d0640a20cb40bd76aa8fe8444f0ca6a880f85ea92aef1b0f1d074fc7e804e858f9b303a502cbdf9ea16b7f8f3e2027424ceFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "short_name": "React App", | ||
| "name": "Create React App Sample", | ||
| "icons": [ | ||
| { | ||
| "src": "favicon.ico", | ||
| "sizes": "192x192", | ||
| "type": "image/png" | ||
| } | ||
| ], | ||
| "start_url": "./index.html", | ||
| "display": "standalone", | ||
| "theme_color": "#000000", | ||
| "background_color": "#ffffff" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| import React from 'react'; | ||
| import ReactDOM from 'react-dom'; | ||
| import App from './App'; | ||
| import registerServiceWorker from './service-worker-registration'; | ||
| import './index.css'; | ||
| ReactDOM.render(<App />, document.getElementById('root')); | ||
| registerServiceWorker(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| export default function register() { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we put this in If so, we need to make sure we run this code through babel so it's IE9+ compliant (minus module syntax). ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies if I'm misunderstanding something about the project organization, but doesn't There is the option of excluding registration, or explicitly switching to Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically On the other hand, I don’t think we really want the generated app to depend on | ||
| if (process.env.NODE_ENV === 'production' && 'serviceWorker' in navigator) { | ||
| window.addEventListener('load', () => { | ||
| const swUrl = `${process.env.PUBLIC_URL}/service-worker.js`; | ||
| navigator.serviceWorker.register(swUrl).then(registration => { | ||
| registration.onupdatefound = () => { | ||
| const installingWorker = registration.installing; | ||
| installingWorker.onstatechange = () => { | ||
| if (installingWorker.state === 'installed') { | ||
| if (navigator.serviceWorker.controller) { | ||
| // At this point, the old content will have been purged and | ||
| // the fresh content will have been added to the cache. | ||
| // It's the perfect time to display a "New content is | ||
| // available; please refresh." message in your web app. | ||
| console.log('New content is available; please refresh.'); | ||
| } else { | ||
| // At this point, everything has been precached. | ||
| // It's the perfect time to display a | ||
| // "Content is cached for offline use." message. | ||
| console.log('Content is cached for offline use.'); | ||
| } | ||
| } | ||
| }; | ||
| }; | ||
| }).catch(error => { | ||
| console.error('Error during service worker registration:', error); | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
| export function unregister() { | ||
| if ('serviceWorker' in navigator) { | ||
| navigator.serviceWorker.ready.then(registration => { | ||
| registration.unregister(); | ||
| }); | ||
| } | ||
| } | ||
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.
We should probably add documentation explaining to update these fields.
Are the
typeandsizesfields necessary? We want to avoid having to update multiple locations to get things working -- if we could automatically deduce this based onindex.htmlthat'd be even better (autogen this entire file at build instead of having it premade).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, my understanding is that
typeandsizesare required: https://developers.google.com/web/updates/2015/10/splashscreenWhile there is a good amount of overlap between metadata that's in
index.htmland what's in thismanifest.jsonfile, there are things that you couldn't trivially deduce—like thebackground_color, thesizesof the icon, or the distinction between theshort_nameandname.I'm a little hesitant to go down the route of requiring developers to partially populate
manifest.jsonand then infer the rest of the fields fromindex.html, as that feels a bit too magic to me.I'll definitely add some documentation explaining
manifest.jsonin general, though.