feat: implement PWA support#305
Open
Papia-tech wants to merge 10 commits into
Open
Conversation
Added PwaRegister component to enable PWA features.
|
@Papia-tech is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughThis PR implements Progressive Web App (PWA) support by adding a service worker for offline caching, a client-side registration component, PWA manifest configuration, and iOS Safari meta tags to enable home-screen installation and offline fallback pages. ChangesPWA Support
Sequence DiagramsequenceDiagram
participant Browser
participant PwaRegister
participant ServiceWorker as Service Worker
participant Cache as Cache Storage
participant Network
Browser->>PwaRegister: window load event
PwaRegister->>ServiceWorker: navigator.serviceWorker.register('/sw.js')
ServiceWorker->>Cache: install: cache '/' and '/offline.html'
ServiceWorker->>Cache: activate: remove old caches<br/>and claim clients
Browser->>ServiceWorker: fetch request
alt Network available
ServiceWorker->>Cache: match cached response
Cache-->>ServiceWorker: return cached asset
else Network failed
ServiceWorker->>Cache: fetch '/offline.html'
Cache-->>ServiceWorker: offline page
end
ServiceWorker-->>Browser: response
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/layout.tsx (1)
15-19: ⚡ Quick winConsider adding apple-touch-icon for iOS home screen installation.
While the
appleWebAppmetadata covers the basic iOS requirements, iOS devices also look forapple-touch-iconmeta tags to display the app icon on the home screen. Without this, iOS will use a screenshot of the page instead.🍎 Add apple-touch-icon to metadata
Next.js metadata API supports icons. Add this to the metadata object:
export const metadata: Metadata = { title: "LinkID", description: "Your professional identity, simplified.", + icons: { + apple: [ + { url: "/web-app-manifest-192x192.png", sizes: "192x192" }, + ], + }, appleWebApp: { capable: true, statusBarStyle: "default", title: "LinkID", }, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/layout.tsx` around lines 15 - 19, The metadata currently defines appleWebApp but lacks an apple-touch-icon; update the metadata object (the same one containing appleWebApp in app/layout.tsx) to include an icons entry with an "apple-touch-icon" entry pointing to your iOS icon (and optional sizes), e.g. add icons: { "apple-touch-icon": "/path/to/icon.png" } alongside any existing icon definitions so iOS home-screen installations use the proper icon; ensure the path and sizes match your asset filenames and export the metadata object unchanged otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/manifest.json`:
- Around line 35-37: The manifest.json has duplicate keys "theme_color",
"background_color", and "display" which must be removed to restore valid JSON;
open manifest.json, locate the duplicate entries for "theme_color",
"background_color", and "display" (the second occurrences shown in the diff) and
delete those duplicate lines so each key is declared only once (keeping the
original definitions at their first occurrence).
In `@app/PwaRegister.tsx`:
- Around line 6-20: The useEffect adds a "load" listener with
window.addEventListener("load", ...) but never removes it; update the effect in
PwaRegister.tsx to store the handler you pass to window.addEventListener("load",
handler) and return a cleanup function that calls
window.removeEventListener("load", handler) so the listener is removed when the
component unmounts or effects re-run (preserve the existing logic around
navigator.serviceWorker.register inside the handler).
In `@app/sw.js`:
- Around line 34-46: The fetch handler currently uses a cache-first flow
(caches.match then fetch) and doesn't perform runtime stale-while-revalidate or
cache new static assets; modify the self.addEventListener('fetch', ...) handler
so it: (1) detects static asset requests (e.g., by checking request.destination
or URL extensions for 'script', 'style', 'font'), (2) attempts
caches.match(event.request) and immediately returns that cachedResponse if
present while simultaneously performing fetch(event.request) in the background,
and when the fetch succeeds clone the response and put it into a runtime cache
(e.g., 'runtime-cache') to update stored assets, (3) for cache misses await
fetch, cache the fetched response then return it, and (4) keep the offline
fallback to caches.match('/offline.html') only if both cache and network fail;
use event.respondWith and event.waitUntil to coordinate the background update
and cache.put operations.
- Around line 1-5: The service worker caches '/offline.html' via ASSETS_TO_CACHE
(CACHE_NAME) causing install failures because offline.html is missing and
registration in PwaRegister.tsx points to "/sw.js" which isn’t served from root;
add a real offline fallback file (create public/offline.html) and make the
worker reachable at /sw.js (move or copy app/sw.js to public/sw.js or add a
Next.js route that serves app/sw.js at '/sw.js') so
cache.addAll(ASSETS_TO_CACHE) succeeds and PwaRegister.tsx can register the
worker.
---
Nitpick comments:
In `@app/layout.tsx`:
- Around line 15-19: The metadata currently defines appleWebApp but lacks an
apple-touch-icon; update the metadata object (the same one containing
appleWebApp in app/layout.tsx) to include an icons entry with an
"apple-touch-icon" entry pointing to your iOS icon (and optional sizes), e.g.
add icons: { "apple-touch-icon": "/path/to/icon.png" } alongside any existing
icon definitions so iOS home-screen installations use the proper icon; ensure
the path and sizes match your asset filenames and export the metadata object
unchanged otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb1752c2-3b21-4742-ba3a-4afab61356c1
📒 Files selected for processing (4)
app/PwaRegister.tsxapp/layout.tsxapp/manifest.jsonapp/sw.js
Comment on lines
35
to
37
| "theme_color": "#6366f1", | ||
| "background_color": "#ffffff", | ||
| "display": "standalone" |
Contributor
There was a problem hiding this comment.
Remove duplicate keys to fix JSON validity.
The manifest contains duplicate declarations of theme_color, background_color, and display at lines 35-37, which were already defined at lines 7, 6, and 5 respectively. Duplicate keys violate the JSON specification and can lead to unpredictable parsing behavior across different browsers.
🔧 Remove duplicate keys
}
],
- "theme_color": "`#6366f1`",
- "background_color": "`#ffffff`",
- "display": "standalone"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "theme_color": "#6366f1", | |
| "background_color": "#ffffff", | |
| "display": "standalone" | |
| } | |
| ], | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/manifest.json` around lines 35 - 37, The manifest.json has duplicate keys
"theme_color", "background_color", and "display" which must be removed to
restore valid JSON; open manifest.json, locate the duplicate entries for
"theme_color", "background_color", and "display" (the second occurrences shown
in the diff) and delete those duplicate lines so each key is declared only once
(keeping the original definitions at their first occurrence).
Comment on lines
+6
to
+20
| useEffect(() => { | ||
| // Force registration on local dev server | ||
| if ("serviceWorker" in navigator) { | ||
| window.addEventListener("load", () => { | ||
| navigator.serviceWorker | ||
| .register("/sw.js") | ||
| .then((registration) => { | ||
| console.log("Service Worker registered: ", registration.scope); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Service Worker registration failed: ", error); | ||
| }); | ||
| }); | ||
| } | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Clean up the event listener to prevent memory leaks.
The load event listener is added inside useEffect but never removed. In React Strict Mode (development), effects run twice, causing multiple listeners to accumulate. This creates a memory leak and could cause the service worker to register multiple times.
🧹 Return cleanup function from useEffect
export default function PwaRegister() {
useEffect(() => {
- // Force registration on local dev server
if ("serviceWorker" in navigator) {
- window.addEventListener("load", () => {
+ const handleLoad = () => {
navigator.serviceWorker
.register("/sw.js")
.then((registration) => {
console.log("Service Worker registered: ", registration.scope);
})
.catch((error) => {
console.error("Service Worker registration failed: ", error);
});
- });
+ };
+
+ window.addEventListener("load", handleLoad);
+
+ return () => {
+ window.removeEventListener("load", handleLoad);
+ };
}
}, []);
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| // Force registration on local dev server | |
| if ("serviceWorker" in navigator) { | |
| window.addEventListener("load", () => { | |
| navigator.serviceWorker | |
| .register("/sw.js") | |
| .then((registration) => { | |
| console.log("Service Worker registered: ", registration.scope); | |
| }) | |
| .catch((error) => { | |
| console.error("Service Worker registration failed: ", error); | |
| }); | |
| }); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| if ("serviceWorker" in navigator) { | |
| const handleLoad = () => { | |
| navigator.serviceWorker | |
| .register("/sw.js") | |
| .then((registration) => { | |
| console.log("Service Worker registered: ", registration.scope); | |
| }) | |
| .catch((error) => { | |
| console.error("Service Worker registration failed: ", error); | |
| }); | |
| }; | |
| window.addEventListener("load", handleLoad); | |
| return () => { | |
| window.removeEventListener("load", handleLoad); | |
| }; | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/PwaRegister.tsx` around lines 6 - 20, The useEffect adds a "load"
listener with window.addEventListener("load", ...) but never removes it; update
the effect in PwaRegister.tsx to store the handler you pass to
window.addEventListener("load", handler) and return a cleanup function that
calls window.removeEventListener("load", handler) so the listener is removed
when the component unmounts or effects re-run (preserve the existing logic
around navigator.serviceWorker.register inside the handler).
Comment on lines
+1
to
+5
| const CACHE_NAME = 'linkid-cache-v1'; | ||
| const ASSETS_TO_CACHE = [ | ||
| '/', | ||
| '/offline.html', // Create a simple offline page if you want a fallback | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for offline.html in public directories
fd -t f "offline.html" public app
# Check current location of sw.js
fd -t f "sw.js"
# Verify Next.js public directory structure
tree -L 2 public/ 2>/dev/null || ls -la public/ 2>/dev/nullRepository: vishnukothakapu/linkid
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where sw.js is referenced/registered
rg -n --hidden --no-ignore-vcs "serviceWorker\.register|/sw\.js|sw\.js" . || true
# Find where offline.html is referenced
rg -n --hidden --no-ignore-vcs "offline\.html" . || true
# Look for any build/copy steps for sw.js/offline.html
rg -n --hidden --no-ignore-vcs "copy.*sw\.js|sw\.js.*public|public.*sw\.js|offline\.html.*public|public.*offline\.html|next-pwa|service worker" package.json .github . || trueRepository: vishnukothakapu/linkid
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Limit to repo code/config (exclude node_modules and build artifacts)
RG_BASE=(--hidden --no-ignore-vcs --glob '!node_modules/**' --glob '!.next/**' --glob '!.eslintcache/**')
echo "== offline.html references =="
rg -n "${RG_BASE[@]}" "offline\.html" . || true
echo "== service worker registration / sw.js references (app/ + config only) =="
rg -n "${RG_BASE[@]}" "serviceWorker\.register|/sw\.js|sw\.js" app .github package.json next.config.* . || true
echo "== PWA/next-pwa / copy plugins config =="
rg -n "${RG_BASE[@]}" "next-pwa|workbox|service worker|sw\.js|offline\.html|copy-plugin|copy\\s*\\(" package.json next.config.* .github . || true
echo "== Candidate files: PwaRegister and layout =="
ls -ლა app | sed -n '1,200p' || true
ls -la .github | sed -n '1,200p' || trueRepository: vishnukothakapu/linkid
Length of output: 3128
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm offline.html existence anywhere in-repo (excluding node_modules and build dirs)
echo "== offline.html files (repo-local, excluding node_modules/.next) =="
fd -t f "offline.html" . --exclude node_modules --exclude .next --exclude .eslintcache || true
echo
echo "== sw.js files (repo-local, excluding node_modules/.next) =="
fd -t f "sw.js" . --exclude node_modules --exclude .next --exclude .eslintcache || true
echo
echo "== app/PwaRegister.tsx =="
sed -n '1,160p' app/PwaRegister.tsx
echo
echo "== app/layout.tsx (relevant section) =="
sed -n '1,120p' app/layout.tsx
echo
echo "== app/sw.js (install/fetch relevant section) =="
# show enough around install and caching logic
sed -n '1,120p' app/sw.js
sed -n '120,260p' app/sw.js
echo
echo "== package.json (scripts/deps relevant to PWA) =="
cat package.json
echo
echo "== next.config.* (if present) =="
ls -la next.config.* 2>/dev/null || true
for f in next.config.*; do [ -f "$f" ] && sed -n '1,200p' "$f"; doneRepository: vishnukothakapu/linkid
Length of output: 6101
Fix missing offline.html and ensure service worker is served at /sw.js
app/sw.jscaches'/offline.html'during theinstallevent viacache.addAll(ASSETS_TO_CACHE), but there is nooffline.htmlanywhere in the repo, so the service worker install can fail (and the fetch fallbackcaches.match('/offline.html')can never work).app/PwaRegister.tsxregisters the worker at"/sw.js", but the onlysw.jsin the repo isapp/sw.js(nothing inpublic/). With no copy/build step configured,/sw.jswon’t be served, so registration will fail.
Add public/offline.html and ensure sw.js is available at /sw.js (e.g., move/copy it to public/sw.js, or add a Next route that serves it).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/sw.js` around lines 1 - 5, The service worker caches '/offline.html' via
ASSETS_TO_CACHE (CACHE_NAME) causing install failures because offline.html is
missing and registration in PwaRegister.tsx points to "/sw.js" which isn’t
served from root; add a real offline fallback file (create public/offline.html)
and make the worker reachable at /sw.js (move or copy app/sw.js to public/sw.js
or add a Next.js route that serves app/sw.js at '/sw.js') so
cache.addAll(ASSETS_TO_CACHE) succeeds and PwaRegister.tsx can register the
worker.
Comment on lines
+34
to
+46
| self.addEventListener('fetch', (event) => { | ||
| event.respondWith( | ||
| caches.match(event.request).then((cachedResponse) => { | ||
| if (cachedResponse) { | ||
| return cachedResponse; | ||
| } | ||
| return fetch(event.request).catch(() => { | ||
| // Fallback when network fails | ||
| return caches.match('/offline.html'); | ||
| }); | ||
| }) | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
Implementation doesn't match stated objectives or caching strategy.
The PR objectives specify "runtime caching of static assets (JS, CSS, fonts)" and the PR summary claims a "stale-while-revalidate caching strategy," but the current implementation:
- Uses cache-first, not stale-while-revalidate: Lines 36-39 return cached responses immediately without revalidation.
- No runtime caching: The service worker only serves pre-cached assets and doesn't cache new requests. JS, CSS, and font files fetched after installation won't be cached.
- Limited offline support: Only
/and/offline.htmlare cached, so navigating to any other page while offline will fail.
For true runtime caching with stale-while-revalidate, the fetch handler should:
- Return the cached response while simultaneously fetching fresh content
- Update the cache with the fresh response for next time
- Cache new assets (JS, CSS, fonts) as they're requested
♻️ Example stale-while-revalidate implementation
// Fetch Assets
self.addEventListener('fetch', (event) => {
+ // Skip non-GET requests
+ if (event.request.method !== 'GET') {
+ return;
+ }
+
event.respondWith(
- caches.match(event.request).then((cachedResponse) => {
- if (cachedResponse) {
- return cachedResponse;
- }
- return fetch(event.request).catch(() => {
- // Fallback when network fails
- return caches.match('/offline.html');
- });
- })
+ caches.open(CACHE_NAME).then((cache) => {
+ return cache.match(event.request).then((cachedResponse) => {
+ const fetchPromise = fetch(event.request)
+ .then((networkResponse) => {
+ // Cache successful responses for static assets
+ if (networkResponse.ok &&
+ (event.request.url.endsWith('.js') ||
+ event.request.url.endsWith('.css') ||
+ event.request.url.match(/\.(woff2?|ttf|eot)$/))) {
+ cache.put(event.request, networkResponse.clone());
+ }
+ return networkResponse;
+ })
+ .catch(() => {
+ // Network failed - return offline page for navigation requests
+ if (event.request.mode === 'navigate') {
+ return cache.match('/offline.html');
+ }
+ throw new Error('Network request failed');
+ });
+
+ // Stale-while-revalidate: return cache immediately, update in background
+ return cachedResponse || fetchPromise;
+ });
+ })
);
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/sw.js` around lines 34 - 46, The fetch handler currently uses a
cache-first flow (caches.match then fetch) and doesn't perform runtime
stale-while-revalidate or cache new static assets; modify the
self.addEventListener('fetch', ...) handler so it: (1) detects static asset
requests (e.g., by checking request.destination or URL extensions for 'script',
'style', 'font'), (2) attempts caches.match(event.request) and immediately
returns that cachedResponse if present while simultaneously performing
fetch(event.request) in the background, and when the fetch succeeds clone the
response and put it into a runtime cache (e.g., 'runtime-cache') to update
stored assets, (3) for cache misses await fetch, cache the fetched response then
return it, and (4) keep the offline fallback to caches.match('/offline.html')
only if both cache and network fail; use event.respondWith and event.waitUntil
to coordinate the background update and cache.put operations.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Anushreebasics
requested changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR transforms the GitNest web application into a Progressive Web App (PWA). This change is necessary to provide users with offline access, "Add to Home Screen" functionality on mobile and desktop, and improved performance through asset caching.
Changes Made
Related Issue
Type of Change
Screenshots / Video (if applicable)
LinkLid.mp4
Summary by CodeRabbit