Code4PuertoRico / suministrospr

Suministros Puerto Rico - Entérate, Ayuda, Informa
https://suministrospr.com
Apache License 2.0
42 stars 17 forks source link

Service worker #146

Closed carlosa54 closed 4 years ago

carlosa54 commented 4 years ago

@gcollazo @Xiomara7 Hola, vi el issue #40 , me puse a jugar con el service worker y luego vi que ya había un PR para lo mismo. Subo este just in case. Está WIP faltando la logica de invalidar/delete caches.

Lo que hace hasta ahora es cache el root y offline html on install y luego añade al cache las páginas que el usuario va viendo, de esta forma si se van offline van a poder ver las páginas que ya habían visto y si la página no está en el cache les sale el offline page.

gcollazo commented 4 years ago

Preguntas: ¿Que estamos caching con el SW? ¿Qué pasa con el SW cuando publiquemos una versión nueva del site? ¿Como se hace un cache bust?

carlosa54 commented 4 years ago

Preguntas: ¿Que estamos caching con el SW? ¿Qué pasa con el SW cuando publiquemos una versión nueva del site? ¿Como se hace un cache bust?

¿Que estamos caching con el SW?

Cuando ocurre el 'install' event del service worker se está caching el root y el /offline page. Luego en el 'fetch' event se hace cache de las paginas que el usuario va navegando dentro del app (Sectores, busqueda, etc). El cache es dinamico y solo guarda las paginas que el user haya visitado mientras el SW está en función

Acá un ejemplo toggling el offline mode:

offline-worker

¿Qué pasa con el SW cuando publiquemos una versión nueva del site?

Actualmente el PR no tiene logica para manejar versiones pero se le puede añadir version specific logic para manejar los caches. Se tendría que pensar como update la version en cada publicación nueva del site.

¿Como se hace un cache bust?

Pensando en el version specific cache de la pregunta anterior y que el contenido del site puede estar en constante cambio (Ediciones a sectores o a la descripción) podemos hacer que el SW siempre escriba al cache en cada fetch event, así mantenemos la ultima version de la pagina que el usuario visitó. Este cache se puede guardar por versions cache-{version} y podemos delete los caches anteriores si cambia la version.

gcollazo commented 4 years ago

@carlosa54 Sin duda necesitamos una idea para invalidar el cache. El site no funciona si devolvemos data vieja para siempre.

Si entiendo bien lo que dices lo que recomiendas hacer es:

En este escenario creo que sería crucial indicar la fecha de actualización de cada página e indicar que estamos mostrando una versión cached y que la data puede haber cambiado pero es necesario tener internet para mostrar las actualizaciones.

¿Suena bien?

carlosa54 commented 4 years ago

@gcollazo

@carlosa54 Sin duda necesitamos una idea para invalidar el cache. El site no funciona si devolvemos data vieja para siempre.

Si entiendo bien lo que dices lo que recomiendas hacer es:

  • Siempre que estemos online, devolvemos la pagina del servidor y guardamos en cache
  • Siempre que estemos offline devolvemos la última página del cache si la tenemos
  • Si estamos offline y no tenemos el page devolvemos una página de error.

Correcto

En este escenario creo que sería crucial indicar la fecha de actualización de cada página e indicar que estamos mostrando una versión cached y que la data puede haber cambiado pero es necesario tener internet para mostrar las actualizaciones.

¿Suena bien?

Hice un refactor al service worker para usar la lógica del online first approach y versioning.

Cuando haya un cambio en el version del service worker se borra el old cache y se hace cache al nuevo version. (Queda pendiente como se le da update al version cada vez que se publique una version nueva del site)

El online first approach siempre devuelve la pagina del servidor y guarda en cache, si el user está offline se trata de devolver la pagina que esté cached y si no hay nada devuelve la pagina indicando que está offline.

Añadí un offline header para indicar al user que está offline. Aquí se añade el copy indicando al user que está viendo una pagina cached y que la data pudo haber cambiado. Intenté buscar la fecha en que se hizo el cache de la pagina utilizando el Date en los headers del cached response pero me devuelve la fecha en que se hizo el request al cache (la fecha actual).

Acá un ejemplo de los cambios:

offline

froi commented 4 years ago

@gcollazo @carlosa54 Any updates on this PR?

I've marked it as stale, please remove the label if it's being worked on or when merging.

carlosa54 commented 4 years ago

@froi @gcollazo I think the PR is mostly done in terms of the offline support logic, only things left would be some copy text changes and a way to update the service worker's version on each new deploy.

https://github.com/Code4PuertoRico/suministrospr/blob/290ac908ae3e8309131ee92c29c7f8698d28902f/templates/common/service-worker.js#L1

Don't know what would be the best way to update that variable on each deploy. The version must be updated so that the service worker deletes old caches.

jpadilla commented 4 years ago

I've enabled dyno metadata on the Heroku app. We could use either HEROKU_RELEASE_VERSION or HEROKU_SLUG_COMMIT environment variables to signal a new deploy.

froi commented 4 years ago

@carlosa54 take a look at @jpadilla's comment

carlosa54 commented 4 years ago

Thanks @jpadilla @froi,

Updated the PR to use HEROKU_RELEASE_VERSION env variable.

If you guys have suggestions in the design and copy text of these files let me know. https://github.com/Code4PuertoRico/suministrospr/blob/4b2837e0b815dbe0dbd3431a1ada262fe12415b8/templates/common/offline.html#L4-L6 https://github.com/Code4PuertoRico/suministrospr/blob/4b2837e0b815dbe0dbd3431a1ada262fe12415b8/templates/common/offline_header.html#L1-L5

froi commented 4 years ago

Only feedback I would have is to integrate the localization work that was merged.

carlosa54 commented 4 years ago

Updated the offline message and added localization.

en:

en

es:

es
froi commented 4 years ago

@carlosa54 Gracias por todo este esfuerzo!

¿Qué me recomiendas para probarlo?

@gcollazo @jpadilla thoughts? Can we move forward with this?

carlosa54 commented 4 years ago

Añadí una condición en el service worker para solo activarlo en producción ya que puede haber conflicto si se tienen otras apps que corran en el mismo puerto en localhost.

gcollazo commented 4 years ago

LGTM 🚀