feat: add PWA offline dashboard support#1184
Conversation
|
@Kishalll is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
GSSoC Label Checklist 🏷️@Priyanshu-byte-coder — please apply the appropriate labels before merging: Difficulty (pick one):
Quality (optional):
Validation (required to score):
|
|
@Priyanshu-byte-coder completed both the prs assigned to me , ready to work on other issues |
|
CI is failing on this PR. Please fix the failing checks before this can be merged. |
|
@Priyanshu-byte-coder fixed ci issues and the pr is not clean |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Thanks for working on PWA offline support! However, there are significant issues with this implementation:
-
Committed pre-built service worker files —
public/sw.js,public/workbox-07cb5bfa.js,public/fallback-Gl9L3J2PvQllaLGPIoSXH.js, andpublic/worker-Gl9L3J2PvQllaLGPIoSXH.jsare build artifacts with content-hashed filenames. These should be gitignored and generated at build time — committing them means they'll go stale after the next build. -
next.config.mjschanges — 127+ line diff to a critical config file needs careful review. Please explain what each change does. -
worker/index.js— 233 lines of service worker logic. Is this the source for the build artifacts inpublic/? If so, the build step should generate the public files from this source.
Recommended approach:
- Add the generated service worker files to
.gitignore - Add a build step that generates them from source
- Keep
next.config.mjschanges minimal
Please rework the implementation to not commit build artifacts to the repo.
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Two issues:
- The PR commits built artifacts directly:
public/sw.js,public/workbox-07cb5bfa.js,public/fallback-*.js,public/worker-*.js. Generated files should not be committed to the repo. - The
next.config.mjschange conflicts with currentmain.
Please remove the built PWA artifacts from the commit and rebase onto current main. The next-pwa integration is otherwise a good addition.
|
@Priyanshu-byte-coder will do a pr fixing this shortly |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
This PR includes compiled PWA build artifacts (public/sw.js, public/workbox-*.js, etc.) that should not be committed to the repository — these are generated at build time. Please add these to .gitignore and remove them from the PR. Also rebase on main to resolve the existing conflicts.
|
@Priyanshu-byte-coder Updated PR based on the review comments and resolved conflicts.
|
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
PR still includes compiled PWA build artifacts: public/sw.js, public/workbox-*.js. These are generated at build time and must not be committed. Add them to .gitignore and remove them from the PR, then rebase on main.
aa75430 to
55842f3
Compare
|
@Priyanshu-byte-coder sorry for all the confusions earlier, i commited new changes along w old ones without force pushing that caused a lot of issue and i hv cleaned those noe, |
Also fixes missing Syne/JetBrains_Mono font imports in layout.tsx. Co-authored-by: Kishalll <kishal2007@gmail.com>
|
@Priyanshu-byte-coder the mergre conflicts needs to resolved before merge ryt ??to work properly, i was working on this now. |
Summary
Adds PWA offline support so the dashboard can load with cached data when the app is offline.
Closes #1050
Type of Change
Changes Made
next-pwaand configured runtime caching innext.config.mjs.data.
OfflineBannerthat showsOffline — showing cached data.loading.
POST /api/goals/sync.How to Test
npm run build.npm run start.http://localhost:3000/dashboardwith internet enabled./sw.jsis registered in browser DevTools under Application/Service Workers./dashboard.Screenshots (if UI change)
Checklist
npm run lintpasses locallynpm run type-check)Validation completed:
npm run buildpasses.