-
-
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
PWA-ification, via SWPrecacheWebpackPlugin + manifest.json #1728
Changes from all commits
8289386
75088ab
bfed215
2835105
2dc418b
21d14bb
a681e91
a9fb906
1cdb4cd
4e3ede4
86d590d
0640a20
cb40bd7
6aa8fe8
444f0ca
6a880f8
5ea92ae
f1b0f1d
074fc7e
804e858
f9b303a
502cbdf
9ea16b7
f8f3e20
27424ce
File filter
Filter by extension
Conversations
Jump to
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() { | ||
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). 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 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
type
andsizes
fields necessary? We want to avoid having to update multiple locations to get things working -- if we could automatically deduce this based onindex.html
that'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
type
andsizes
are required: https://developers.google.com/web/updates/2015/10/splashscreenWhile there is a good amount of overlap between metadata that's in
index.html
and what's in thismanifest.json
file, there are things that you couldn't trivially deduce—like thebackground_color
, thesizes
of the icon, or the distinction between theshort_name
andname
.I'm a little hesitant to go down the route of requiring developers to partially populate
manifest.json
and 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.json
in general, though.