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

Progressive Web App (suggestion) #1266

Open
superoo7 opened this Issue Oct 17, 2017 · 17 comments

Comments

Projects
None yet
2 participants
@superoo7
Copy link
Contributor

superoo7 commented Oct 17, 2017

I would like to suggest a few progressive web app approach of making this site.

  • Mobile Icon on iOS and Android (Add to Home Screen)

I sent a pull request on this #1265

  • enable service worker (it was disabled, I want to know why)

pull request on this #1265

  • push notification

  • Caching for faster load of the app (require eject of webpack)

  • Faster load speed and App Shell

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 17, 2017

I had some issues with the site not loading when the service worker was enabled. It started working again when I added the unregister call, not sure what the problem was

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Oct 17, 2017

I have hosted the web (https://superoo7-odota.surge.sh/), tested on chrome, no issue.

Google lighthouse report, before 64/100 and after 91/100

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 17, 2017

OK, submit a PR and we can test it on stage

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Oct 17, 2017

Alright I sent a pull request to re-enable service worker. Should I include package-lock.json?

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Oct 17, 2017

22553468_10209895288594094_445158864_o22563883_10209895288434090_931246752_o

Tested on Android with Chrome

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 17, 2017

Did package-lock change? This shouldn't require installing any dependencies?

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Oct 17, 2017

I think I build it then it updates, I did not include package-lock.json in my commit.

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 17, 2017

Does the service worker handle things properly if the file name changes? I think previously the issue was that the js bundle name changed (since it has a hash in the file name) and then the service worker was trying to reload the old bundle.

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Oct 17, 2017

I never publish or tested web that created with create-react-app with service worker. I normally build my own webpack from scratch and use this plugin for progressive web app because it caches automatically and create service workers. However, this requires to eject the project out from create-react-app.

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 31, 2017

It appears that the current version of the code won't load on the first visit after an update because it can't find the older version of the javascript bundle.

According to registerServiceWorker it only loads the latest data in the background during the failed page load, so the initial load won't work.

https://github.com/odota/ui/blob/master/src/registerServiceWorker.js#L6

I don't know if this is a deployment or configuration issues (we use netlify to serve the UI)

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Oct 31, 2017

The error is an "unexpected token <" error in the javascript console. It's possible this is due to the netlify configuration that causes it to serve index.html regardless of the path requested (this is to support client side routing).

I'm not sure if the service worker expects a 404 response in order to identify that a file no longer exists/has changed

@howardchung howardchung added this to the Backlog milestone Nov 16, 2017

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Nov 20, 2017

Sorry for the delay reply, I was busy in the past 3 weeks, now looking through the statement u said on the "unexpected token <", I don't really know what is it on the configuration on netlify, but another approach is to create our own service-worker.js where giving up the default service worker given by create-react-app, this approach will allow us to make our own cache strategy and more flexibility.

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Nov 20, 2017

If you know how to patch the worker to get it to work, we can try it out?

@howardchung

This comment has been minimized.

Copy link
Member

howardchung commented Nov 20, 2017

I tried making netlify serve 404 for not found errors of the JS bundle, but the problem still occurs, so that's probably not the issue.

If it's an issue with the service worker though, I'd imagine that people would have running into this since create-react-app is quite popular

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Nov 20, 2017

Not too sure whether is create-react-app issue or not, I think I will create a service-worker.js in the public folder, and then enable it in the src/index.js file. Will update you once is done.

In my opinion, create-react-app is for initial creating app, but when time goes, many configuration that you need required you to eject it and modified it.

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Nov 20, 2017

I ejected a project created with create-react-app and in the webpack.config.prod.js file:

screen shot 2017-11-20 at 12 48 50 pm

It uses SWPrecacheWebpackPlugin to create service-worker.js.
SWPrecache is the Preaccessor of WorkBox, it is stable and older tool, which does not have the latest service worker available tools. I myself use Workbox normally, never tried SWPrecache.

@superoo7

This comment has been minimized.

Copy link
Contributor Author

superoo7 commented Nov 29, 2017

I made a PR on fixing the error "unexpected token <"

#1344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.