-
Notifications
You must be signed in to change notification settings - Fork 87
feat: added lazy and suspense with skeleton on various folders #111
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?
Conversation
|
@Shivam107 is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Shivam107 brooooo 🙇 nice work. just checking it. |
|
@Shivam107 nice work shivam! just left some reviews. 🙏 |
c2d3fd3 to
39febd9
Compare
|
Deleted the unnecesaary backend files and prisma is now 5.22.0 |
|
@Shivam107 Please avoid updating the dependencies file as it might break things. If an update is necessary, check with the maintainer first. |
changed that few days ago |
|
hi @Shivam107, ive checked it locally and i don't know why the position of the skeleton is not the same as the respective component. attaching a video of this event. am i missing something in this? Screen.Recording.2025-10-24.at.9.28.41.PM.mov |
Removed unnecessary columns from the Account table.
its a merge conflict issue , since this PR was raised a week ago. |
d68514b to
6dc8a40
Compare
WalkthroughReplaces many static component imports with React.lazy + Suspense and adds a new Skeleton UI component; extracts pricing, terms, and privacy content into separate modules; converts several pages (landing, dashboard, login, checkout, pricing, legal) to lazy-load subcomponents and adjusts dashboard layout/scroll behavior and a projects useEffect. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Page
participant Suspense
participant Skeleton
participant LazyComp as React.lazy Component
User->>Browser: Navigate to route
Browser->>Page: Render server/client shell
Page->>Suspense: Mount Suspense boundary for lazy part
Suspense->>Skeleton: No module yet — render fallback
Skeleton->>Browser: Show placeholder UI
par Async load
Suspense->>LazyComp: Dynamic import (React.lazy)
LazyComp->>LazyComp: Initialize module
end
LazyComp->>Suspense: Component ready
Suspense->>Browser: Replace Skeleton with actual UI
Browser->>User: Full content displayed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus during review:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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.
Actionable comments posted: 14
🧹 Nitpick comments (7)
apps/web/src/app/(main)/legal/privacy/page.tsx (1)
1-7: Reconsider the "use client" directive for SEO.Legal pages like privacy policies benefit from server-side rendering for SEO. The current implementation makes this entire route a client component, which delays content rendering until JavaScript loads.
Consider either:
- Removing lazy loading for this static content page (it's a legal page, not performance-critical to lazy load)
- Using Next.js's dynamic imports with
ssr: trueinstead of React.lazyFor SEO-critical static content, immediate server-side rendering is typically preferred over lazy loading.
Example alternative using Next.js dynamic import:
-"use client"; - -import React, { Suspense, lazy } from "react"; - +import dynamic from "next/dynamic"; import { Skeleton } from "@/components/ui/skeleton"; -const PrivacyPolicyPage = lazy(() => import("./PrivacyPolicyPage")); +const PrivacyPolicyPage = dynamic(() => import("./PrivacyPolicyPage"), { + loading: () => ( + <div className="py-20 text-center text-lg text-neutral-400"> + <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> + <Skeleton className="h-6 w-40" /> + <Skeleton className="h-4 w-72" /> + <Skeleton className="h-4 w-64" /> + <Skeleton className="h-4 w-56" /> + </div> + </div> + ), + ssr: true +}); export default function PrivacyOfServicePage() { - return ( - <Suspense - fallback={ - <div className="py-20 text-center text-lg text-neutral-400"> - <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> - <Skeleton className="h-6 w-40" /> - <Skeleton className="h-4 w-72" /> - <Skeleton className="h-4 w-64" /> - <Skeleton className="h-4 w-56" /> - </div> - </div> - } - > - <PrivacyPolicyPage /> - </Suspense> - ); + return <PrivacyPolicyPage />; }apps/web/src/app/checkout/page.tsx (1)
19-29: Skeleton-position mismatch (UX).The centered, max-w-md fallback won’t match CheckoutConfirmation’s layout, causing the offset shown in the video. Mirror the same outer wrappers and widths as the target component in the loading UI to avoid layout shift. I can help craft a layout‑specific skeleton if you share the component’s container classes.
apps/web/src/components/ui/skeleton.tsx (1)
7-9: Make Skeleton presentational by default (a11y).Mark as decorative to reduce verbosity for AT; announce loading at the wrapper instead.
-export function Skeleton({ className, ...props }: SkeletonProps) { - return <div className={cn("animate-pulse rounded-md bg-white/10", className)} {...props} />; -} +export function Skeleton({ className, ...props }: SkeletonProps) { + return ( + <div + aria-hidden="true" + className={cn("animate-pulse rounded-md bg-white/10", className)} + {...props} + /> + ); +}Optional: wrap page-level fallbacks with role="status" aria-busy="true" aria-live="polite".
apps/web/src/app/(main)/legal/terms/TermsContent.tsx (1)
12-14: Effective date maintenance (nit).Consider centralizing the effective date (config/env) to avoid stale dates in legal copy.
apps/web/src/app/(main)/dashboard/page.tsx (1)
10-23: Skeleton fallback alignment with DashboardContainer.Match the container classes (height, borders, margins) used by DashboardContainer to prevent the centered max-w-md skeleton from appearing offset. Consider a block-level skeleton mirroring h-[80vh]/rounded/border.
apps/web/src/app/(main)/(landing)/layout.tsx (1)
12-25: Make the Navbar fallback match its real height/width.A single full-width bar better mirrors the Navbar and avoids visible jump.
- <div className="py-20 text-center text-lg text-neutral-400"> - <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> - <Skeleton className="h-6 w-40" /> - <Skeleton className="h-4 w-72" /> - <Skeleton className="h-4 w-64" /> - <Skeleton className="h-4 w-56" /> - </div> - </div> + <div className="w-full"> + <Skeleton className="h-14 w-full rounded-none" /> + </div>apps/web/src/app/(main)/(landing)/page.tsx (1)
21-163: Optional: Extract repeated skeleton fallback to reduce duplication.The same skeleton structure is repeated 10 times across all Suspense boundaries. Consider extracting it to a reusable component or constant for maintainability.
+const GenericSectionSkeleton = () => ( + <div className="py-20 text-center text-lg text-neutral-400"> + <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> + <Skeleton className="h-6 w-40" /> + <Skeleton className="h-4 w-72" /> + <Skeleton className="h-4 w-64" /> + <Skeleton className="h-4 w-56" /> + </div> + </div> +); <Suspense - fallback={ - <div className="py-20 text-center text-lg text-neutral-400"> - <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> - <Skeleton className="h-6 w-40" /> - <Skeleton className="h-4 w-72" /> - <Skeleton className="h-4 w-64" /> - <Skeleton className="h-4 w-56" /> - </div> - </div> - } + fallback={<GenericSectionSkeleton />} >Note: This doesn't address the critical skeleton mismatch issue; it only reduces code duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/web/src/app/(main)/(landing)/layout.tsx(1 hunks)apps/web/src/app/(main)/(landing)/page.tsx(1 hunks)apps/web/src/app/(main)/(landing)/pricing/PricingContent.tsx(1 hunks)apps/web/src/app/(main)/(landing)/pricing/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/home/page.tsx(2 hunks)apps/web/src/app/(main)/dashboard/layout.tsx(1 hunks)apps/web/src/app/(main)/dashboard/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/projects/page.tsx(2 hunks)apps/web/src/app/(main)/legal/privacy/PrivacyPolicyPage.tsx(1 hunks)apps/web/src/app/(main)/legal/privacy/page.tsx(1 hunks)apps/web/src/app/(main)/legal/terms/TermsContent.tsx(1 hunks)apps/web/src/app/(main)/legal/terms/page.tsx(1 hunks)apps/web/src/app/(main)/login/page.tsx(1 hunks)apps/web/src/app/checkout/page.tsx(2 hunks)apps/web/src/components/faq/FaqSection.tsx(1 hunks)apps/web/src/components/ui/skeleton.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
apps/web/src/app/(main)/dashboard/page.tsx (2)
apps/web/src/components/dashboard/DashboardContainer.tsx (1)
DashboardContainer(11-42)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/dashboard/projects/page.tsx (2)
apps/web/src/app/(main)/dashboard/page.tsx (1)
Dashboard(7-26)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/dashboard/home/page.tsx (2)
apps/web/src/app/(main)/dashboard/page.tsx (1)
Dashboard(7-26)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/(landing)/layout.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/dashboard/layout.tsx (5)
apps/web/src/store/useFilterStore.ts (1)
useFilterStore(9-14)apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(8-11)apps/web/src/components/dashboard/DashboardHeader.tsx (1)
DashboardHeader(10-45)apps/web/src/components/ui/FiltersContainer.tsx (1)
FiltersContainer(21-144)apps/web/src/components/dashboard/Sidebar.tsx (1)
Sidebar(27-113)
apps/web/src/app/(main)/(landing)/pricing/PricingContent.tsx (1)
apps/web/src/components/ui/shine-borders.tsx (1)
ShineBorder(30-61)
apps/web/src/app/(main)/login/page.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/checkout/page.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/legal/terms/page.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/(landing)/pricing/page.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/components/ui/skeleton.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/(main)/legal/privacy/page.tsx (1)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/(landing)/page.tsx (3)
apps/web/src/components/landing-sections/how-it-works.tsx (1)
HowItWorks(33-66)apps/web/src/components/faq/FaqSection.tsx (1)
FaqSection(10-62)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
🔇 Additional comments (7)
apps/web/src/components/faq/FaqSection.tsx (1)
10-10: Export change verified—all imports correctly updated to default import with lazy loading.Verification confirms that the FaqSection export change from named to default export has been properly implemented:
- ✓ No broken named imports found in the codebase
- ✓ Single usage at
apps/web/src/app/(main)/(landing)/page.tsx:13correctly useslazy(() => import(...))- ✓ Component wrapped in
<Suspense>with appropriate Skeleton fallback UI- ✓ React imports (
Suspense,lazy) properly declaredThe lazy loading pattern is complete and no breaking changes exist.
apps/web/src/app/(main)/legal/privacy/PrivacyPolicyPage.tsx (3)
1-1: LGTM: ESLint disable is appropriate.The
react/no-unescaped-entitiesdisable is justified for this text-heavy legal document containing natural language with apostrophes and quotes.
10-12: Verify the hardcoded effective date.The effective date is set to January 8, 2024, which is nearly 2 years before this PR. Please confirm whether this is the correct effective date for your privacy policy or if it should be updated to reflect when this policy actually takes effect.
418-425: LGTM: External links follow security best practices.External links properly use
target="_blank"withrel="noopener noreferrer", which prevents security vulnerabilities like reverse tabnabbing.apps/web/src/app/(main)/dashboard/layout.tsx (1)
2-29: LGTM: Layout improvements for scrollable dashboard content.The overflow handling changes (overflow-hidden on the container, overflow-y-auto on sidebar and main content) properly enable vertical scrolling for dashboard sections. The self-closing tag syntax and import organization are clean improvements.
apps/web/src/app/(main)/(landing)/pricing/page.tsx (1)
8-8: LGTM: Clean lazy loading structure.The import path for PricingContent is appropriately located within the same feature directory, making the component organization clear.
apps/web/src/app/(main)/(landing)/pricing/PricingContent.tsx (1)
13-109: LGTM: Well-structured data-driven pricing content.The data structures for
opensoxFeatures,whySub,freePlanCard, andpremiumPlanCardare well-organized and make the component maintainable. The handling of both string and array descriptions (lines 154-164) is appropriately flexible.
|
@Shivam107 thanks for the changes, shivam! reviewing the dashboard pr and once it's merged. we'll take this one. |
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.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/web/src/app/(main)/dashboard/projects/page.tsx (2)
10-10: Critical architectural issue remains unresolved: cross-route page import.As flagged in the previous review, importing
../pagefromdashboard/projects/page.tsx(a Client Component) violates Next.js boundaries becausedashboard/page.tsxis a Server Component. This creates route coupling and architectural issues.The recommended solution is to extract a shared
DashboardScreenclient component that both pages can import, eliminating the problematic cross-route dependency.
23-38: Extract duplicated skeleton fallback to a reusable component.The skeleton fallback markup (lines 24-34) is duplicated identically in
dashboard/home/page.tsx(lines 29-39). Extract this to a reusableDashboardSkeletoncomponent to eliminate duplication and ensure consistency.Create
apps/web/src/components/dashboard/DashboardSkeleton.tsx:export function DashboardSkeleton() { return ( <div className="py-20 text-center text-lg text-neutral-400"> <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> <Skeleton className="h-6 w-40" /> <Skeleton className="h-4 w-72" /> <Skeleton className="h-4 w-64" /> <Skeleton className="h-4 w-56" /> </div> </div> ); }Then update both files:
- <Suspense - fallback={ - <div className="py-20 text-center text-lg text-neutral-400"> - <div className="mx-auto flex w-full max-w-md flex-col items-center gap-4 px-6"> - <Skeleton className="h-6 w-40" /> - <Skeleton className="h-4 w-72" /> - <Skeleton className="h-4 w-64" /> - <Skeleton className="h-4 w-56" /> - </div> - </div> - } - > + <Suspense fallback={<DashboardSkeleton />}> <Dashboard /> </Suspense>Note: The skeleton mismatch issue (flagged in previous reviews and confirmed by the maintainer's video) should also be addressed by updating
DashboardSkeletonto match the actual Dashboard layout.apps/web/src/app/(main)/dashboard/home/page.tsx (2)
11-11: Same critical cross-route import issue as projects/page.tsx.This file has the same architectural violation: importing
../pagefrom another route page. See the detailed comment ondashboard/projects/page.tsxline 10 for the full explanation and recommended solution.
28-43: Skeleton fallback duplicated from projects/page.tsx.This skeleton markup is identical to the one in
dashboard/projects/page.tsx(lines 24-34). Refer to the comment on that file for the recommended refactoring to extract a reusableDashboardSkeletoncomponent.
🧹 Nitpick comments (1)
apps/web/src/app/(main)/dashboard/projects/page.tsx (1)
17-21: Remove or clarify outdated comment.Line 18's comment "Change to true to always render the container" is confusing since the value is already
true. If this was a development note, it should be removed. If it's meant to document behavior, rephrase it as "Always render the container."Apply this diff:
useEffect(() => { - setRenderProjects(true); // Change to true to always render the container + setRenderProjects(true); setProjectTitle("Projects"); setData([]); // Clear any existing projects }, [setRenderProjects, setProjectTitle, setData]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/web/src/app/(main)/dashboard/home/page.tsx(2 hunks)apps/web/src/app/(main)/dashboard/projects/page.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/app/(main)/dashboard/home/page.tsx (2)
apps/web/src/app/(main)/dashboard/page.tsx (1)
Dashboard(7-26)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
apps/web/src/app/(main)/dashboard/projects/page.tsx (2)
apps/web/src/app/(main)/dashboard/page.tsx (1)
Dashboard(7-26)apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(7-9)
feat: added lazy and suspense with skeleton on the pricing , landing)home , projects , dashboard , privacy , terms , login , checkout folder. #40
**_
_**
demoLazySuspense.mp4
landing page : Added reusable wait helper, imported the new Skeleton component, and replaced all text fallbacks with skeleton placeholders for every lazily loaded section (navbar, hero, content sections, CTA, footer).
landing layout : Mirrored the skeleton fallback for the shared navbar wrapper.
dashboard shell : Converted major dashboard pieces to lazy loading with delayed imports and consistent skeleton fallbacks (header, filters, sidebar, page content).
auth & checkout pages : Wrapped page-level components in Suspense with skeleton placeholders.
pricing page : Replaced Vue fallback with the new skeleton loader.
legal entry points : Switched to lazy loading PrivacyPolicyPage / TermsContent with skeleton fallbacks.
Added new shared Components & Content -
Skeleton UI (components/ui/skeleton.tsx): Introduced reusable skeleton div component using cn.
Pricing content (landing/pricing/PricingContent.tsx): Added full pricing page markup (cards, testimonials, etc.) split out from the route.
Legal content (main/legal/privacy/PrivacyPolicyPage.tsx and main/legal/terms/TermsContent.tsx): Added dedicated components containing the full privacy policy and terms of service text.
Summary by CodeRabbit
New Features
Improvements
Refactor