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

fix(pwa): avoid caching large files #1672

Merged

Conversation

MrMurdock11
Copy link
Collaborator

@MrMurdock11 MrMurdock11 commented Apr 14, 2024

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (refactoring and improving code)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Description

I'm going to try to solve an issue related to caching files that are chunked for transfer (usually they have a 206 status) that we have discussed here GH-1651. To avoid this error, we should only cache files that have a response code of 200.

image

Copy link
Owner

@cotes2020 cotes2020 left a comment

Choose a reason for hiding this comment

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

  1. Please change the scop of the commit message to pwa:
    fix(cache): -> fix(pwa):.

  2. This change doesn‘t seem to work. It breaks PWA's ability to respond offline to cached resources (e.g. JS/CSS files of type cors). You can verify this with local testing. JEKYLL_ENV=production bundle exec jekyll b -l

    Visit http://localhost:4000/, open a few random posts, then disconnect from the network and visit the same history again:

    Screenshot 2024-04-14 at 21 26 05

@cotes2020 cotes2020 changed the title fix(cache): avoid caching large files fix(pwa): avoid caching large files Apr 14, 2024
assets/js/pwa/sw.js Outdated Show resolved Hide resolved
@cotes2020 cotes2020 added the help wanted Help us with this topic label Apr 16, 2024
@MrMurdock11
Copy link
Collaborator Author

@cotes2020 What do you think if I fix the 206 code to filter the request and finalize this PR, but create a new discussion to find a better solution?

@kungfux
Copy link
Collaborator

kungfux commented Apr 24, 2024

@cotes2020 What do you think if I fix the 206 code to filter the request and finalize this PR, but create a new discussion to find a better solution?

I didn't elaborate on this much, but I have seen the problem in action as it causes errors for newly developed features and so should be fixed before release, I think.

Is there anything else we should fix except 206? I mean maybe it's a good idea to fix what we have discovered so far only?

@cotes2020
Copy link
Owner

The comments I left above were mainly to get your opinions on whether there is a better solution.

Unless you have a better way, detecting whether the response status code is 206 is an acceptable solution.

If innerResponse’s status is 206, return a promise rejected with a TypeError.

Ref: https://w3c.github.io/ServiceWorker/#cache-put

@cotes2020 cotes2020 force-pushed the master branch 2 times, most recently from 2698778 to b77767f Compare April 29, 2024 00:19
@MrMurdock11
Copy link
Collaborator Author

@cotes2020 added the changes to your comment

assets/js/pwa/sw.js Outdated Show resolved Hide resolved
@cotes2020 cotes2020 removed the help wanted Help us with this topic label Apr 29, 2024
@cotes2020 cotes2020 merged commit 76d58fe into cotes2020:master Apr 29, 2024
5 checks passed
@MrMurdock11 MrMurdock11 deleted the hotfix/avoid-large-file-size-caching branch April 29, 2024 15:34
sgn07124 pushed a commit to sgn07124/sgn07124.github.io that referenced this pull request Apr 30, 2024
@cotes2020
Copy link
Owner

cotes2020 commented Apr 30, 2024

Sorry guys, I made a mistake while reviewing, returning directly after event.request.headers.has('range') is met would have the undesirable result that fontwesome styles could not be cached.

So, I'm going to rewrite this commit to fix this issue.


Update:

Everything works fine, it's my browser extension that interferes with the test results, the Fontawesome icons can be cached correctly. Sorry for the disruption.

github-actions bot pushed a commit that referenced this pull request May 11, 2024
## [7.0.0](v6.5.5...v7.0.0) (2024-05-11)

### ⚠ BREAKING CHANGES

* optimize the resource hints (#1717)
* rename media-url file and related parameters (#1651)
* rename comment setting parameter (#1563)
* **analytics:** add post pageviews for GoatCounter (#1543)

### Features

* add cloudflare web analytics ([#1723](#1723)) ([c17fba4](c17fba4))
* add support for embed video files ([#1558](#1558)) ([9592146](9592146))
* add support for giscus strict title matching ([#1614](#1614)) ([700fd5b](700fd5b))
* **analytics:** add post pageviews for GoatCounter ([#1543](#1543)) ([b641b3f](b641b3f))
* **analytics:** add Umami and Matomo tracking codes ([#1658](#1658)) ([61bdca2](61bdca2))
* change site verification settings ([#1561](#1561)) ([e436387](e436387))
* **deps:** move `MathJax` configuration to a separate file ([#1670](#1670)) ([44f552c](44f552c))
* display theme version in footer ([#1611](#1611)) ([8349314](8349314))
* **i18n:** allow `page.lang` to override `site.lang` ([#1586](#1586)) ([547b95c](547b95c))
* make post description customizable ([#1602](#1602)) ([f865336](f865336))
* **media:** support audio and video tag with multi sources ([#1618](#1618)) ([23be416](23be416))

### Bug Fixes

* make TOC title and entries visible at the same time ([#1711](#1711)) ([e0950fc](e0950fc))
* mode toggle not outlined when receiving keyboard focus ([#1690](#1690)) ([cd37f63](cd37f63))
* prevent footnote back arrow from becoming an emoji ([#1716](#1716)) ([8608147](8608147))
* **pwa:** skip range requests in service worker ([#1672](#1672)) ([76d58fe](76d58fe))
* search result prompt is empty ([#1583](#1583)) ([8a2afae](8a2afae))
* use `https` for Weibo sharing URL ([#1612](#1612)) ([8e5fbb7](8e5fbb7))

### Improvements

* improve <hr> visibility in dark mode ([#1565](#1565)) ([4ddd5c4](4ddd5c4))
* lean bootstrap javascript ([#1734](#1734)) ([ddb48ed](ddb48ed))
* rename comment setting parameter ([#1563](#1563)) ([f8390d4](f8390d4))
* replace jQuery with Vanilla JS ([#1681](#1681)) ([fe7afa3](fe7afa3))
* simplify mode toggle script ([#1692](#1692)) ([d4a6d64](d4a6d64))
* tree shaking Bootstrap CSS ([#1736](#1736)) ([363a3d9](363a3d9))

### Changes

* optimize the resource hints ([#1717](#1717)) ([dcb0add](dcb0add))
* rename media-url file and related parameters ([#1651](#1651)) ([9f8aeaa](9f8aeaa))
Copy link

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kungfux kungfux mentioned this pull request May 13, 2024
1 task
kimbob13 pushed a commit to kimbob13/kimbob13.github.io that referenced this pull request May 25, 2024
kimbob13 pushed a commit to kimbob13/kimbob13.github.io that referenced this pull request May 25, 2024
## [7.0.0](cotes2020/jekyll-theme-chirpy@v6.5.5...v7.0.0) (2024-05-11)

### ⚠ BREAKING CHANGES

* optimize the resource hints (cotes2020#1717)
* rename media-url file and related parameters (cotes2020#1651)
* rename comment setting parameter (cotes2020#1563)
* **analytics:** add post pageviews for GoatCounter (cotes2020#1543)

### Features

* add cloudflare web analytics ([cotes2020#1723](cotes2020#1723)) ([c17fba4](cotes2020@c17fba4))
* add support for embed video files ([cotes2020#1558](cotes2020#1558)) ([9592146](cotes2020@9592146))
* add support for giscus strict title matching ([cotes2020#1614](cotes2020#1614)) ([700fd5b](cotes2020@700fd5b))
* **analytics:** add post pageviews for GoatCounter ([cotes2020#1543](cotes2020#1543)) ([b641b3f](cotes2020@b641b3f))
* **analytics:** add Umami and Matomo tracking codes ([cotes2020#1658](cotes2020#1658)) ([61bdca2](cotes2020@61bdca2))
* change site verification settings ([cotes2020#1561](cotes2020#1561)) ([e436387](cotes2020@e436387))
* **deps:** move `MathJax` configuration to a separate file ([cotes2020#1670](cotes2020#1670)) ([44f552c](cotes2020@44f552c))
* display theme version in footer ([cotes2020#1611](cotes2020#1611)) ([8349314](cotes2020@8349314))
* **i18n:** allow `page.lang` to override `site.lang` ([cotes2020#1586](cotes2020#1586)) ([547b95c](cotes2020@547b95c))
* make post description customizable ([cotes2020#1602](cotes2020#1602)) ([f865336](cotes2020@f865336))
* **media:** support audio and video tag with multi sources ([cotes2020#1618](cotes2020#1618)) ([23be416](cotes2020@23be416))

### Bug Fixes

* make TOC title and entries visible at the same time ([cotes2020#1711](cotes2020#1711)) ([e0950fc](cotes2020@e0950fc))
* mode toggle not outlined when receiving keyboard focus ([cotes2020#1690](cotes2020#1690)) ([cd37f63](cotes2020@cd37f63))
* prevent footnote back arrow from becoming an emoji ([cotes2020#1716](cotes2020#1716)) ([8608147](cotes2020@8608147))
* **pwa:** skip range requests in service worker ([cotes2020#1672](cotes2020#1672)) ([76d58fe](cotes2020@76d58fe))
* search result prompt is empty ([cotes2020#1583](cotes2020#1583)) ([8a2afae](cotes2020@8a2afae))
* use `https` for Weibo sharing URL ([cotes2020#1612](cotes2020#1612)) ([8e5fbb7](cotes2020@8e5fbb7))

### Improvements

* improve <hr> visibility in dark mode ([cotes2020#1565](cotes2020#1565)) ([4ddd5c4](cotes2020@4ddd5c4))
* lean bootstrap javascript ([cotes2020#1734](cotes2020#1734)) ([ddb48ed](cotes2020@ddb48ed))
* rename comment setting parameter ([cotes2020#1563](cotes2020#1563)) ([f8390d4](cotes2020@f8390d4))
* replace jQuery with Vanilla JS ([cotes2020#1681](cotes2020#1681)) ([fe7afa3](cotes2020@fe7afa3))
* simplify mode toggle script ([cotes2020#1692](cotes2020#1692)) ([d4a6d64](cotes2020@d4a6d64))
* tree shaking Bootstrap CSS ([cotes2020#1736](cotes2020#1736)) ([363a3d9](cotes2020@363a3d9))

### Changes

* optimize the resource hints ([cotes2020#1717](cotes2020#1717)) ([dcb0add](cotes2020@dcb0add))
* rename media-url file and related parameters ([cotes2020#1651](cotes2020#1651)) ([9f8aeaa](cotes2020@9f8aeaa))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants