Skip to content

feat: pwa registration#762

Merged
kodiakhq[bot] merged 10 commits intokawalcovid19:mainfrom
kelilipan:experiment/pwa
Aug 25, 2021
Merged

feat: pwa registration#762
kodiakhq[bot] merged 10 commits intokawalcovid19:mainfrom
kelilipan:experiment/pwa

Conversation

@kelilipan
Copy link
Copy Markdown
Contributor

@kelilipan kelilipan commented Aug 18, 2021 •
edited
Loading

Closes #637

Related to #716 #719

Description

I am new with CSP stuff, but I think this might be a clue https://qubyte.codes/blog/content-security-policy-and-service-workers.

TLDR; SW is not using image-src policy but using connect-src policy instead. Since we don't have connect-src it fallback to default-src wich is only limited to self / origin of the domain itself. So, it refuses to "connect"

image

Mas @zainfathoni tried to add Cloudinary url to script-src (at #719 ) but ofc it doesn't work, we need to add to connect-src.

So I tried to add res.cloudinary.com to connect-src and deploy to my personal netlify account for preview (yes, I'm dumb. brb making a Draft PR for deploy preview). And it works (and it caches cloudinary img assets)

https://hardcore-noyce-3b9626.netlify.app/

image
image

Current Tasks

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 18, 2021 •
edited
Loading

✔️ Deploy Preview for wargabantuwarga ready!

🔨 Explore the source changes: bcd4831

🔍 Inspect the deploy log: https://app.netlify.com/sites/wargabantuwarga/deploys/6126495b95dcf800086ef499

😎 Browse the preview: https://deploy-preview-762--wargabantuwarga.netlify.app

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 18, 2021 •
edited
Loading

Codecov Report

Merging #762 (bcd4831) into main (2efaaf5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #762   +/-   ##
=======================================
  Coverage   84.82%   84.82%           
=======================================
  Files         132      132           
  Lines        1417     1417           
  Branches      455      455           
=======================================
  Hits         1202     1202           
  Misses        211      211           
  Partials        4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2efaaf5...bcd4831. Read the comment docs.

@kelilipan kelilipan mentioned this pull request Aug 18, 2021
Closed
@kelilipan kelilipan marked this pull request as ready for review August 18, 2021 07:41
@kelilipan
Copy link
Copy Markdown
Contributor Author

kelilipan commented Aug 18, 2021

PWA lighthouse report
image

@resir014 resir014 requested a review from a team August 18, 2021 08:57
mazipan
mazipan reviewed Aug 18, 2021
View reviewed changes
Comment thread netlify.toml
@mazipan
Copy link
Copy Markdown
Member

mazipan commented Aug 18, 2021 •
edited
Loading

Need help from @zainfathoni and others also to test the preview domain.
I believe the error can not be catched by Lighthouse test.
We need to check manually.

@ekafyi
Copy link
Copy Markdown
Contributor

ekafyi commented Aug 18, 2021 •
edited
Loading

I just checked https://deploy-preview-762--wargabantuwarga.netlify.app/ and got mixed result.

EDIT: Images were successfully cached as intended. LGTM.

This previous error was because i initially opened the page on large screen and the screen was reesized when opening devtools. The first two cached images were the large ones.

Most images were successfully fetched and cached by the SW as intended.

Two images (hero_banner and sembako-cta) were not fetched.

I initially thought it had something to do with images rendered via Next's Image component, but the telemedicine banner, which also uses Image component, was fetched correctly.

Also TIL the whole Content-Security stuff 🤯

script-src directive specifies valid sources for JavaScript

connect-src directive restricts the URLs which can be loaded using script interfaces

Link drop:

@resir014 resir014 requested a review from zainfathoni August 19, 2021 07:02
resir014
resir014 requested changes Aug 19, 2021
View reviewed changes
Comment thread netlify.toml
@kelilipan kelilipan requested a review from resir014 August 19, 2021 07:26
resir014
resir014 approved these changes Aug 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@resir014 resir014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@zainfathoni zainfathoni added the need-perf-check Trigger Lighthouse-CI check label Aug 25, 2021
zainfathoni
zainfathoni approved these changes Aug 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

Thanks for working on it, Mas @svspicious! 🙏

@mazipan mazipan added the automerge To be merged automatically once all the requirements are fulfilled label Aug 25, 2021
@kodiakhq kodiakhq Bot merged commit 8e69a8b into kawalcovid19:main Aug 25, 2021
@zainfathoni
Copy link
Copy Markdown
Member

zainfathoni commented Aug 25, 2021

@all-contributors please add @svspicious for code

@allcontributors allcontributors Bot mentioned this pull request Aug 25, 2021
Merged
@allcontributors
Copy link
Copy Markdown
Contributor

allcontributors Bot commented Aug 25, 2021

@zainfathoni

I've put up a pull request to add @svspicious! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@resir014 resir014 resir014 approved these changes

@zainfathoni zainfathoni zainfathoni approved these changes

@mazipan mazipan mazipan approved these changes

Assignees

No one assigned

Labels

automerge To be merged automatically once all the requirements are fulfilled need-perf-check Trigger Lighthouse-CI check

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SW activation and cache static files

5 participants

@kelilipan @mazipan @ekafyi @zainfathoni @resir014