-
Notifications
You must be signed in to change notification settings - Fork 32
fix: avoid caching personalized pages #434
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| const CACHE_VERSION = "v3"; | ||
| const CACHE_VERSION = "v4"; | ||
| const STATIC_CACHE = `examcooker-static-${CACHE_VERSION}`; | ||
| const PAGE_CACHE = `examcooker-pages-${CACHE_VERSION}`; | ||
| const RUNTIME_CACHE = `examcooker-runtime-${CACHE_VERSION}`; | ||
|
|
@@ -17,6 +17,7 @@ const KNOWN_CACHES = new Set([STATIC_CACHE, PAGE_CACHE, RUNTIME_CACHE]); | |
| const STATIC_PATH_PREFIXES = ["/_next/static/", "/icons/", "/assets/", "/vendor/"]; | ||
| const STATIC_PATH_EXACT = new Set(["/manifest.webmanifest", "/offline.html", "/sw.js"]); | ||
| const FONT_HOSTS = new Set(["fonts.googleapis.com", "fonts.gstatic.com"]); | ||
| const NO_CACHE_PATHS = new Set(["/", "/auth"]); | ||
| const NO_CACHE_PATH_PREFIXES = [ | ||
| "/api/", | ||
| "/auth/", | ||
|
|
@@ -42,7 +43,17 @@ function isStaticAsset(url) { | |
| } | ||
|
|
||
| function isUncacheable(url) { | ||
| return NO_CACHE_PATH_PREFIXES.some((prefix) => url.pathname.startsWith(prefix)); | ||
| return ( | ||
| NO_CACHE_PATHS.has(url.pathname) || | ||
| NO_CACHE_PATH_PREFIXES.some((prefix) => url.pathname.startsWith(prefix)) | ||
| ); | ||
| } | ||
|
|
||
| function isCacheableResponse(response) { | ||
| if (!response || !response.ok || response.type === "opaque") return false; | ||
|
|
||
| const cacheControl = response.headers.get("cache-control") || ""; | ||
| return !/(^|,\s*)(no-store|no-cache|private)(\s|,|=|$)/i.test(cacheControl); | ||
| } | ||
|
Comment on lines
+52
to
57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Per RFC 9111, |
||
|
|
||
| function isHtmlAccept(request) { | ||
|
|
@@ -103,8 +114,11 @@ self.addEventListener("message", (event) => { | |
| for (const route of event.data.routes) { | ||
| if (typeof route !== "string" || !route.startsWith("/")) continue; | ||
| try { | ||
| const url = new URL(route, self.location.origin); | ||
| if (isUncacheable(url)) continue; | ||
|
|
||
| const response = await fetch(route, { credentials: "same-origin" }); | ||
| if (response && response.ok) { | ||
| if (isCacheableResponse(response)) { | ||
| await cache.put(route, response.clone()); | ||
| } | ||
| } catch { | ||
|
|
@@ -125,7 +139,7 @@ async function staleWhileRevalidate(event, cacheName) { | |
|
|
||
| const networkFetch = (preloadResponse ? Promise.resolve(preloadResponse) : fetch(event.request)) | ||
| .then((response) => { | ||
| if (response && response.ok && response.type !== "opaque") { | ||
| if (isCacheableResponse(response)) { | ||
| cache.put(event.request, response.clone()).catch(() => undefined); | ||
| } | ||
| return response; | ||
|
|
@@ -163,7 +177,7 @@ async function cacheFirst(event) { | |
| event.waitUntil( | ||
| fetch(event.request) | ||
| .then((response) => { | ||
| if (response && response.ok && response.type !== "opaque") { | ||
| if (isCacheableResponse(response)) { | ||
| return cache.put(event.request, response.clone()); | ||
| } | ||
| return undefined; | ||
|
|
@@ -174,7 +188,7 @@ async function cacheFirst(event) { | |
| } | ||
| try { | ||
| const response = await fetch(event.request); | ||
| if (response && response.ok && response.type !== "opaque") { | ||
| if (isCacheableResponse(response)) { | ||
| cache.put(event.request, response.clone()).catch(() => undefined); | ||
| } | ||
| return response; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message
fix: avoid caching personalized pagesfollows conventional-commit form but is thin — it doesn't mention that/authexact-match was missing from the no-cache set, thatCache-Control: private/no-store/no-cacheis now inspected before every cache write, or that the version bump was needed to evict stale entries. A reader scanninggit logwon't understand the root cause or the scope of the fix without opening the PR. Something likefix(sw): add /auth+/ exact-no-cache paths, honour Cache-Control private/no-store, bump cache to v4captures all three changes in one line.Context Used: Encourage people to write better commit messages (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!