feat: add manifest.json and registered a service worker#237
feat: add manifest.json and registered a service worker#237jedryczkatymon wants to merge 3 commits intodevfrom
Conversation
|
The preview deployment for frontend-testownik-dev is ready. 🟢 Open Preview | Open Build Logs | Open Application Logs Last updated at: 2026-04-07 12:32:35 CET |
There was a problem hiding this comment.
Pull request overview
Adds baseline PWA assets (Web App Manifest + Service Worker) and wires them into the Next.js App Router layout so the app can be installed/packaged on mobile.
Changes:
- Add
public/manifest.jsonand reference it viametadata.manifestinsrc/app/layout.tsx. - Add a client-side
ServiceWorkerRegistercomponent and mount it in the root layout to register/sw.js. - Add
public/sw.jsand include it intsconfig.json.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Includes public/sw.js in TS project inputs. |
| src/components/service-worker-register.tsx | Registers the service worker on the client. |
| src/app/layout.tsx | Adds manifest metadata and mounts SW registration component. |
| public/sw.js | Implements fetch interception + caching strategy. |
| public/manifest.json | Adds PWA manifest (currently placeholder content). |
| if ("serviceWorker" in navigator) { | ||
| window.addEventListener("load", () => { | ||
| navigator.serviceWorker.register("/sw.js").then( | ||
| (registration) => { | ||
| console.log( | ||
| "Service Worker registration successful with scope: ", | ||
| registration.scope, | ||
| ); | ||
| }, | ||
| (err) => { | ||
| console.error("Service Worker registration failed: ", err); | ||
| }, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
useEffect adds a load event listener but never removes it, and if the component hydrates after the load event has already fired the callback won’t run (so the service worker may never register). Register immediately when document.readyState === "complete" (or just call register() directly in the effect) and return a cleanup that removes the listener to avoid duplicate registrations in dev/StrictMode.
| if ("serviceWorker" in navigator) { | |
| window.addEventListener("load", () => { | |
| navigator.serviceWorker.register("/sw.js").then( | |
| (registration) => { | |
| console.log( | |
| "Service Worker registration successful with scope: ", | |
| registration.scope, | |
| ); | |
| }, | |
| (err) => { | |
| console.error("Service Worker registration failed: ", err); | |
| }, | |
| ); | |
| }); | |
| } | |
| if (!("serviceWorker" in navigator)) { | |
| return; | |
| } | |
| const registerServiceWorker = () => { | |
| navigator.serviceWorker.register("/sw.js").then( | |
| (registration) => { | |
| console.log( | |
| "Service Worker registration successful with scope: ", | |
| registration.scope, | |
| ); | |
| }, | |
| (err) => { | |
| console.error("Service Worker registration failed: ", err); | |
| }, | |
| ); | |
| }; | |
| if (document.readyState === "complete") { | |
| registerServiceWorker(); | |
| return; | |
| } | |
| window.addEventListener("load", registerServiceWorker); | |
| return () => { | |
| window.removeEventListener("load", registerServiceWorker); | |
| }; |
| console.log( | ||
| "Service Worker registration successful with scope: ", | ||
| registration.scope, | ||
| ); |
There was a problem hiding this comment.
Avoid console.log in app runtime (it’ll ship to production and can leak implementation details). Consider removing it, gating behind process.env.NODE_ENV !== "production", or using the project’s existing error/reporting mechanism if you need telemetry.
| console.log( | |
| "Service Worker registration successful with scope: ", | |
| registration.scope, | |
| ); | |
| if (process.env.NODE_ENV !== "production") { | |
| console.log( | |
| "Service Worker registration successful with scope: ", | |
| registration.scope, | |
| ); | |
| } |
| .catch((_) => { | ||
| /* eat any errors */ | ||
| }), |
There was a problem hiding this comment.
respondWith(...catch(() => { /* eat */ })) can resolve to undefined, which is not a valid Response and can cause “Failed to respond to fetch” errors. Return a concrete fallback Response (or rethrow) from the catch path so respondWith always receives a Response/Promise<Response>.
| .catch((_) => { | |
| /* eat any errors */ | |
| }), | |
| .catch((_) => Response.error()), |
| self.addEventListener("fetch", (event) => { | ||
| // Skip some of cross-origin requests, like those for Google Analytics. | ||
| if (HOSTNAME_WHITELIST.indexOf(new URL(event.request.url).hostname) > -1) { | ||
| // Stale-while-revalidate | ||
| // similar to HTTP's stale-while-revalidate: https://www.mnot.net/blog/2007/12/12/stale | ||
| // Upgrade from Jake's to Surma's: https://gist.github.com/surma/eb441223daaedf880801ad80006389f1 | ||
| const cached = caches.match(event.request); | ||
| const fixedUrl = getFixedUrl(event.request); | ||
| const fetched = fetch(fixedUrl, { cache: "no-store" }); | ||
| const fetchedCopy = fetched.then((resp) => resp.clone()); |
There was a problem hiding this comment.
This fetch handler modifies all same-origin URLs by appending a cache-bust query param and then caches responses under the original request. In a Next.js app this can break navigations and route handlers (and can serve stale HTML that references removed hashed chunks). Restrict SW logic to safe requests (e.g., GET only and static assets like /_next/static, icons, fonts), and avoid rewriting URLs for navigations/API routes.
| "name": "The name of your application", | ||
| "short_name": "This name will show in your Windows taskbar, in the start menu, and Android homescreen", | ||
| "start_url": "The URL that should be loaded when your application is opened", | ||
| "display": "standalone", | ||
| "description": "A description for your application", | ||
| "lang": " The default language of your application", |
There was a problem hiding this comment.
Several required manifest fields are placeholder sentences (with spaces) rather than valid values (name, short_name, start_url, description, lang). Browsers/installers may reject the manifest or produce a broken install. Replace these with real app values (e.g., name: "Testownik Solvro", short_name: "Testownik", start_url: "/", lang: "pl").
| "name": "The name of your application", | |
| "short_name": "This name will show in your Windows taskbar, in the start menu, and Android homescreen", | |
| "start_url": "The URL that should be loaded when your application is opened", | |
| "display": "standalone", | |
| "description": "A description for your application", | |
| "lang": " The default language of your application", | |
| "name": "Testownik Solvro", | |
| "short_name": "Testownik", | |
| "start_url": "/", | |
| "display": "standalone", | |
| "description": "Platforma do nauki i rozwiązywania quizów dla studentów PWr.", | |
| "lang": "pl", |
| @@ -0,0 +1,29 @@ | |||
| { | |||
| "display_override": ["window-controls-overlay"], | |||
| "protocol_handlers": [{ "protocol": "mailto", "url": "/newEmail?to=%s" }], | |||
There was a problem hiding this comment.
protocol_handlers points to /newEmail?to=%s, but there’s no matching route in the codebase, so invoking the handler will 404. Either implement the route or remove protocol_handlers until it’s supported.
| "protocol_handlers": [{ "protocol": "mailto", "url": "/newEmail?to=%s" }], |
|
Hej, przejrzysz review od Copilota i zrobisz resolve już poprawionych rzeczy ? |
nie mogę spakować apki na mobile zanim nie dodam tych rzeczy
(odnośnie
service-worker-register.tsx-> wiem, że useEffecta nie używamy, ale to tylko PoC 😄 )