Financial-Times / n-service-worker

❌ DECOMMISSIONED Global service worker component for next.ft.com
5 stars 2 forks source link

fix appache manifest cache section #129

Closed bjfletcher closed 6 years ago

bjfletcher commented 7 years ago

the old code was generating this:

CACHE:
https://www.ft.com/__origami/service/build/v2/files/o-fonts-assets@1.3.2/MetricWeb-Regular.woff?,/__origami/service/build/v2/files/o-fonts-assets@1.3.2/MetricWeb-Semibold.woff?,/__origami/service/build/v2/files/o-fonts-assets@1.3.2/FinancierDisplayWeb-Regular.woff?
https://www.ft.com/__origami/service/image/v2/images/raw/ftlogo:brand-ft-masthead?source=o-header&tint=%2333302E,%2333302E&format=svg,/__origami/service/image/v2/images/raw/fticon-v1:hamburger?source=o-icons&tint=%2333302E,%2333302E&format=svg,/__origami/service/image/v2/images/raw/fticon-v1:search?source=o-icons&tint=%2333302E,%2333302E&format=svg,/__origami/service/image/v2/images/raw/ftlogo:brand-myft?source=o-header&tint=%2333302E,%2333302E&format=svg

when really we want:

CACHE:
https://www.ft.com/__origami/service/build/v2/files/o-fonts-assets@1.3.2/MetricWeb-Regular.woff
https://www.ft.com/__origami/service/build/v2/files/o-fonts-assets@1.3.2/MetricWeb-Semibold.woff
https://www.ft.com/__origami/service/build/v2/files/o-fonts-assets@1.3.2/FinancierDisplayWeb-Regular.woff
https://www.ft.com/__origami/service/image/v2/images/raw/ftlogo:brand-ft-masthead?source=o-header&tint=%2333302E,%2333302E&format=svg
https://www.ft.com/__origami/service/image/v2/images/raw/fticon-v1:hamburger?source=o-icons&tint=%2333302E,%2333302E&format=svg
https://www.ft.com/__origami/service/image/v2/images/raw/fticon-v1:search?source=o-icons&tint=%2333302E,%2333302E&format=svg
https://www.ft.com/__origami/service/image/v2/images/raw/ftlogo:brand-myft?source=o-header&tint=%2333302E,%2333302E&format=svg

which this PR will do

bjfletcher commented 7 years ago

I'd like clarification on why 3 out of 4 fonts are in appcache, with the 1 other font in sw, in https://github.com/Financial-Times/n-service-worker/blob/master/config/precache.js

Do you know why, @wheresrhys, @leggsimon or @tavvy?

wheresrhys commented 6 years ago

Good question. In addition all fonts should be at v1.3.2

Having dug through history, it was me https://github.com/Financial-Times/n-service-worker/pull/74

During the offline experiment in the autumn metric bold was added to the offline page. At the time it was only used on the offline pages, so I wanted to keep it clear from the appcache, which should only have an effect on iOS, which doesn't support sw & offline

Since then, metric bold is more widely used on the site. We agreed with design that it should only be used in article toppers for rich journalism. I'm not sure if they've stuck to that agreement though.

To be honest, I'm not convinced we should be precaching that font at all