Skip to content

Add skeleton screens and route-level loading states across all pages (including Ideas section)#594

Open
shreyamittal239 wants to merge 2 commits intoAOSSIE-Org:mainfrom
shreyamittal239:improved-UX-issue-277
Open

Add skeleton screens and route-level loading states across all pages (including Ideas section)#594
shreyamittal239 wants to merge 2 commits intoAOSSIE-Org:mainfrom
shreyamittal239:improved-UX-issue-277

Conversation

@shreyamittal239
Copy link

@shreyamittal239 shreyamittal239 commented Feb 22, 2026

Addressed Issues:

Fixes #277

Recordings:

  • Added skeleton loading states across all pages
  • Route-level loading spinner during navigation
  • Ideas section now shows loading placeholders during data fetch
  • Dark mode compatibility verified
Screen.Recording.2026-02-23.025129.mp4

Additional Notes:

This PR introduces consistent skeleton screens and route-level loading states across the entire application to improve perceived performance and eliminate layout shifts.

Key Changes:

  • Implemented reusable Skeleton component system (rectangular, circular, text variants)
  • Added SkeletonText, SkeletonCard, SkeletonGrid
  • Added SuspenseWrapper for loading states
  • Added route-level loading indicator in _app.jsx
  • Implemented page-level skeleton placeholders for:
    • Home
    • Projects
    • About
    • Apply
    • Ideas section (including dynamic routes)

Ideas Section Refactor:

  • Removed getStaticProps and getStaticPaths
  • Converted to client-side data fetching using useState + useEffect
  • Added IdeasGridSkeleton and IdeaContentSkeleton
  • Proper error handling added

If maintaining static generation for SEO is preferred, I’m happy to adjust the implementation accordingly.

All changes were tested locally (including slow network simulation and dark mode).

Summary by CodeRabbit

  • New Features

    • Added skeleton loading screens and loading indicators across the app for improved user feedback during data fetching.
    • Implemented loading states for route navigation with visual indicators.
    • Ideas pages now load dynamically with skeleton placeholders while content fetches.
  • Bug Fixes

    • Fixed an SVG icon path to display correctly.

@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

Introduces a comprehensive skeleton loading system with new Skeleton components and updates pages to use client-side data fetching with loading states. Adds SuspenseWrapper for suspense boundaries, implements route-change loading indicators in the app wrapper, and migrates ideas pages from static generation to dynamic client-side data fetching. Includes a minor SVG icon update.

Changes

Cohort / File(s) Summary
Skeleton Components
src/components/Skeletons/Skeleton.jsx, src/components/Skeletons/SkeletonText.jsx, src/components/Skeletons/SkeletonCard.jsx, src/components/Skeletons/SkeletonGrid.jsx, src/components/Skeletons/index.js
New reusable skeleton loading components: base Skeleton with variant support (rectangular, circular, text), SkeletonText for text lines, SkeletonCard for card placeholders, and SkeletonGrid for responsive grid layouts. Index file centralizes exports.
Suspense & Loading Infrastructure
src/components/SuspenseWrapper.jsx, src/pages/_app.jsx
Adds SuspenseWrapper component with default fallback UI and PageLoadingSkeleton. Updates _app.jsx to show loading spinner during route changes via router event listeners.
Ideas Pages Migration
src/pages/ideas/index.jsx, src/pages/ideas/2024/index.jsx, src/pages/ideas/[year]/[slug].jsx
Migrates all ideas pages from static generation (getStaticProps/getStaticPaths) to client-side data fetching with useState/useEffect. Adds internal skeleton components for loading states during data fetch.
Icon Update
src/pages/projects.jsx
Updates SVG path data for icon in LinkIcon component.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant Router as Next.js Router
    participant Page as Ideas Page
    participant API as Data API
    participant UI as UI (Skeletons/Content)

    User->>Router: Navigate to /ideas
    Router->>Page: Render component
    Page->>Page: Initialize state (articles=[], isLoading=true)
    Page->>UI: Render SkeletonGrid
    UI-->>User: Show skeleton placeholders
    
    Page->>API: useEffect: fetch ideas data
    API-->>Page: Return articles data
    Page->>Page: Update state (articles, isLoading=false)
    Page->>UI: Re-render with fetched data
    UI-->>User: Display article cards
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Skeletons dance while data takes flight,
Loading states shimmer, smooth as the night,
No more waiting, the pages now breathe,
Client-side magic tucked in the sleeves,
Routes shine bright with their pulse and their gleam!

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR aims to address issue #277 (update Twitter-X logo), but the changeset focuses on skeleton screens and loading states instead, with only a minor SVG path fix in projects.jsx. Either update the PR to fully address the Twitter-X logo replacement from issue #277, or relink the PR to a more appropriate issue focused on loading states and skeleton screens.
Out of Scope Changes check ⚠️ Warning The PR contains extensive out-of-scope changes: refactoring Ideas section to client-side fetching, removing static generation, and adding route-level loading spinners go beyond the linked issue #277's requirement of replacing a Twitter logo. Clarify the PR's primary objective: either focus on the Twitter-X logo update from #277, or create a separate issue for the skeleton/loading state feature and update the PR link accordingly.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding skeleton screens and route-level loading states across pages, which aligns with the majority of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@shreyamittal239
Copy link
Author

Hi @Muneerali199

I’ve implemented consistent skeleton loading states across all pages, including the Ideas section (converted to client-side fetching to enable loading placeholders).

The project builds successfully and has been tested locally (including slow network simulation).

Please let me know if you'd prefer retaining SSG for the Ideas section — I’m happy to adjust the implementation if needed.

Looking forward to your feedback. Thanks!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/pages/ideas/[year]/[slug].jsx (2)

64-72: ⚠️ Potential issue | 🟡 Minor

router.isFallback check is dead code after removing getStaticPaths.

router.isFallback is only true during SSG fallback rendering, which requires getStaticPaths with fallback: true or 'blocking'. Since getStaticPaths was removed in this PR, this branch can never be reached and should be deleted.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx around lines 64 - 72, Remove the now-dead
SSG fallback branch: delete the router.isFallback conditional and its JSX return
(the Container block that renders "Loading...") since getStaticPaths was removed
and this branch can never be reached; update any imports or surrounding code in
the [year]/[slug].jsx component if they become unused after removing the
router.isFallback block.

195-206: ⚠️ Potential issue | 🟠 Major

The inline prop on the code component was removed in react-markdown v9.0.0 (not v8.0.0), and it does not exist in v10.1.0. As a result, inline is always undefined and every code snippet—including single-backtick inline code—renders using block-level styles instead of inline styles.

The modern approach is to override pre for code blocks and code for inline code:

Recommended fix
-code: ({ node, inline, ...props }) =>
-  inline ? (
-    <code
-      className="bg-zinc-100 dark:bg-zinc-800 text-green-600 dark:text-yellow-400 px-2 py-1 rounded text-sm font-mono"
-      {...props}
-    />
-  ) : (
-    <code
-      className="block bg-zinc-100 dark:bg-zinc-800 text-zinc-800 dark:text-zinc-200 p-4 rounded-lg overflow-x-auto text-sm font-mono my-4"
-      {...props}
-    />
-  ),
+pre: ({ node, ...props }) => (
+  <pre className="bg-zinc-100 dark:bg-zinc-800 p-4 rounded-lg overflow-x-auto my-4" {...props} />
+),
+code: ({ node, ...props }) => (
+  <code
+    className="bg-zinc-100 dark:bg-zinc-800 text-green-600 dark:text-yellow-400 px-2 py-1 rounded text-sm font-mono"
+    {...props}
+  />
+),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx around lines 195 - 206, The current custom
renderer uses the removed inline prop on the code component so every snippet
renders as a block; replace that logic by providing both a pre renderer for
block-level code and a code renderer for inline code inside the ReactMarkdown
components prop: implement components.pre to render the block wrapper (using the
block classes you already have: "block bg-zinc-100 ... font-mono my-4" and
forward children/props), and implement components.code to render inline code
(using the inline classes "bg-zinc-100 ... text-green-600 ... font-mono" and
forward className/children/props); remove reliance on the inline argument and
ensure className and children are forwarded correctly in both pre and code
renderers so fenced code blocks get the pre styling and single-backtick inline
code gets the inline styling.
src/pages/_app.jsx (1)

37-62: ⚠️ Potential issue | 🟠 Major

Double-loading pattern creates jarring UX — current page unmounts immediately on navigation.

When a route change begins, loading flips to true and the current <Component> unmounts right away, showing only <PageLoader>. Once the route transition completes, the incoming page mounts with its own isLoading = true state and then runs the 500ms timer. The user therefore sees:

current page disappears → full-page spinner (route transition) → per-page skeleton (500ms) → real content

That's three sequential states every time a link is clicked. A more user-friendly approach is to keep the current page visible and use a non-blocking top-bar progress indicator (e.g. nprogress) while the route is loading, and remove the artificial per-page timers for pages that have no real async data dependency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/_app.jsx` around lines 37 - 62, The current useEffect toggles
loading and unmounts the current page by switching from rendering <Component> to
<PageLoader>; instead, keep <Component previousPathname={previousPathname}
{...pageProps} /> mounted and render the route loader as a non-blocking
indicator (e.g., integrate nprogress or a top-bar progress component) so
routeChangeStart triggers nprogress.start() and
routeChangeComplete/routeChangeError call nprogress.done(); update the JSX to
always render <Component /> and conditionally render PageLoader only as an
overlay/progress indicator (not full-page replacement), and remove any
artificial per-page 500ms timers in page-level components that rely on isLoading
so pages with no async data don’t double-load.
🧹 Nitpick comments (10)
src/components/Skeletons/Skeleton.jsx (1)

4-12: Move variantStyles map outside the component.

It's a static constant recreated on every render. Hoisting it to module scope avoids unnecessary object allocations.

♻️ Proposed refactor
+const variantStyles = {
+  rectangular: 'rounded-md',
+  circular: 'rounded-full',
+  text: 'rounded',
+}
+
 const Skeleton = forwardRef(function Skeleton(
   { className, variant = 'rectangular', width, height, ...props },
   ref
 ) {
-  const variantStyles = {
-    rectangular: 'rounded-md',
-    circular: 'rounded-full',
-    text: 'rounded',
-  }
-
   return (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Skeletons/Skeleton.jsx` around lines 4 - 12, The variantStyles
object is recreated on every render inside the forwardRef Skeleton component;
hoist it to module scope as a top-level constant (e.g., VARIANT_STYLES) and have
the Skeleton function reference that constant instead of declaring variantStyles
inline so you avoid repeated allocations; update any usages in the Skeleton
component to read from the new top-level VARIANT_STYLES.
src/pages/apply.jsx (1)

11-31: items array values are defined but never read in TimelineSkeleton.

The map callback uses (_, index) — the items entries (with their title/description fields) are ignored. Replace the array with a plain Array.from({ length: 5 }).

♻️ Proposed refactor
 function TimelineSkeleton() {
-  const items = [
-    { title: 'Join us on Discord', description: '' },
-    { title: 'Start Contributing', description: '' },
-    { title: 'Write a Draft Application', description: '' },
-    { title: 'Discuss with Mentors', description: '' },
-    { title: 'Submit Final Application', description: '' },
-  ];
-
   return (
     <ol className="relative border-l-2 border-gray-200 dark:border-gray-700">
-      {items.map((_, index) => (
+      {Array.from({ length: 5 }).map((_, index) => (
         <li key={index} className="mb-10 ml-6">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/apply.jsx` around lines 11 - 31, TimelineSkeleton defines a
constant items = [...] but the map callback ignores each entry (uses (_,
index)), so the array contents are unused; fix by either using the defined items
inside the map (e.g., map((item, index) => ...) and render
item.title/item.description) or replace the items array with a simple
Array.from({ length: 5 }) and keep map((_, index) => ...); update the map
callback in TimelineSkeleton accordingly so the code no longer defines unused
values.
src/components/SuspenseWrapper.jsx (1)

14-18: Prefer nullish coalescing over || for the fallback check.

fallback || defaultFallback will discard valid falsy fallbacks (e.g., 0, false, ""). Use ?? to only substitute when the caller passes null or undefined.

♻️ Proposed fix
-    <Suspense fallback={fallback || defaultFallback}>
+    <Suspense fallback={fallback ?? defaultFallback}>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SuspenseWrapper.jsx` around lines 14 - 18, The Suspense
fallback expression currently uses fallback || defaultFallback which will
incorrectly override valid falsy values; update the JSX in the SuspenseWrapper
component to use the nullish coalescing operator (fallback ?? defaultFallback)
so only null or undefined fallbacks are replaced—adjust the expression passed to
the Suspense fallback prop (referencing fallback, defaultFallback, and
Suspense).
src/pages/_app.jsx (1)

1-10: Remove unused Suspense and Skeleton imports.

Neither Suspense (Line 3) nor Skeleton (Line 10) are referenced anywhere in this file.

♻️ Proposed fix
-import { useEffect, useRef, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
 import { useRouter } from 'next/router'
-import { Suspense } from 'react'
 
 ...
 
-import { Skeleton } from '@/components/Skeletons'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/_app.jsx` around lines 1 - 10, Remove the unused imports to clean
up the module: delete the unused React symbol Suspense from the named imports
and remove the unused Skeleton import from '@/components/Skeletons' so only used
symbols (useEffect, useRef, useState, useRouter, Header, Footer) remain; update
the import statements at the top of the module to drop Suspense and the Skeleton
import line.
src/pages/about.jsx (2)

174-188: Loading branch duplicates styles.container as an inline object.

The outer <div> at Line 176 re-declares the same style values already captured in the styles.container object (Lines 102-109). Use styles.container for consistency.

♻️ Proposed fix
-    <div style={{ fontFamily: 'Arial, sans-serif', padding: '20px', textAlign: 'center', maxWidth: '1000px', margin: '0 auto' }}>
+    <div style={styles.container}>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/about.jsx` around lines 174 - 188, The loading branch duplicates
inline container styles; replace the inline style object on the outer <div> in
the isLoading return with the existing styles.container constant to avoid
duplication and ensure consistency (update the JSX that renders AboutSkeleton,
Banner and Container.Outer to use styles.container instead of the inline style).

50-55: Artificial 500ms delay on a fully-static page degrades UX.

The About page has no async data: isDarkMode is a synchronous DOM read and all content is hardcoded. The timer adds 500ms of unnecessary skeleton display on every visit without any benefit.

The skeleton pattern makes sense only for pages with genuine async dependencies (e.g. ideas/index.jsx fetching from GitHub). For pages that are entirely static, drop the timer and the isLoading state entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/about.jsx` around lines 50 - 55, Remove the artificial 500ms
loading delay and the now-unnecessary isLoading state: delete the useEffect
timer block that calls setIsLoading, remove the isLoading state variable and any
conditional rendering that shows the skeleton when isLoading is true (in
src/pages/about.jsx), and render the About page content directly (still
respecting isDarkMode where used). Ensure any references to setIsLoading or
isLoading are cleaned up so there are no unused state variables or dead branches
left.
src/pages/index.jsx (1)

92-98: setIsLoading(true) at the top of the effect is redundant.

isLoading is already initialized to true, and the effect runs only once on mount, so this call is a no-op.

♻️ Proposed fix
   useEffect(() => {
-    setIsLoading(true)
     const timer = setTimeout(() => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/index.jsx` around lines 92 - 98, The initial setIsLoading(true)
inside the useEffect is redundant because isLoading is initialized to true and
the effect runs only on mount; remove the top-level setIsLoading(true) call in
the useEffect that contains setRandomProjects and setIsLoading(false), leaving
the timeout logic (setRandomProjects(projects.sort(() => 0.5 -
Math.random()).slice(0, 3)) and setIsLoading(false)) intact and keep the cleanup
return () => clearTimeout(timer) so the effect still runs once and clears the
timer properly.
src/pages/ideas/[year]/[slug].jsx (1)

11-11: getCurrentYear is imported but never used in this file.

♻️ Proposed fix
-import { fetchIdeaContent, getCurrentYear } from '@/helper/fetchGitHubIdeas';
+import { fetchIdeaContent } from '@/helper/fetchGitHubIdeas';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx at line 11, Remove the unused
getCurrentYear import from the import statement (leave fetchIdeaContent) or, if
you intended to use it, actually call getCurrentYear where the current year is
needed (e.g., for defaulting or validating the [year] route param); update the
import to only import fetchIdeaContent if you choose removal and ensure there
are no other references to getCurrentYear in this component.
src/pages/projects.jsx (2)

15-16: Consolidate two imports from the same module into one statement.

♻️ Proposed fix
-import { Skeleton } from '@/components/Skeletons';
-import { SkeletonText } from '@/components/Skeletons';
+import { Skeleton, SkeletonText } from '@/components/Skeletons';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/projects.jsx` around lines 15 - 16, Consolidate the two separate
imports into a single import statement by combining Skeleton and SkeletonText:
replace the two lines importing Skeleton and SkeletonText from
'@/components/Skeletons' with one import that imports both symbols (Skeleton,
SkeletonText) from the same module to remove the duplicate import.

185-192: Artificial 500ms delay for synchronous data.

Same pattern as about.jsx: all project content comes from a synchronous local import, so the 500ms timer only delays the page render with no real async work behind it. Consider removing the timer entirely and rendering content immediately, keeping the skeleton pattern only for pages with genuine async dependencies.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/projects.jsx` around lines 185 - 192, The 500ms artificial delay in
Projects page is unnecessary because data is synchronous; remove the useEffect
timer and the isLoading state (useState/setIsLoading) or set isLoading to false
immediately so the component renders content without delay; update render logic
that checks isLoading to show content directly (or keep the skeleton only for
true async pages) and delete the timer cleanup code referencing
timer/clearTimeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/Skeletons/SkeletonGrid.jsx`:
- Around line 6-10: The wrapper <span> around each <SkeletonCard /> produces
invalid HTML because SkeletonCard renders a <div>; remove the <span> and apply
the key directly to the <SkeletonCard /> inside the Array.from map in
SkeletonGrid.jsx (i.e., change the mapping from <span key={index}><SkeletonCard
/></span> to <SkeletonCard key={index} /> or wrap in a fragment if needed).

In `@src/components/SuspenseWrapper.jsx`:
- Around line 21-31: The outer wrapper in PageLoadingSkeleton currently uses the
animate-pulse class which duplicates the animation already applied inside each
Skeleton, causing compounded opacity; remove animate-pulse from the outer
wrapper div (leave other classes like spacing/background intact) so only the
Skeleton components control the pulse animation and restore the intended 50%
keyframe opacity.

In `@src/pages/apply.jsx`:
- Around line 35-42: Remove the artificial 500ms delay by deleting the isLoading
state, its setter setIsLoading, and the useEffect that creates the timer
(setTimeout/clearTimeout); then remove the conditional render that checks
isLoading and render the page content directly (replace the isLoading ? skeleton
branch with the actual content) so the apply page displays immediately without
synthetic latency.

In `@src/pages/ideas/2024/index.jsx`:
- Around line 127-130: Add a new error state (e.g., const [error, setError] =
useState(null)) and update the data-fetching catch block in the getAllIdeas flow
to call setError(error) (or setError(error.message)) before the finally that
calls setIsLoading(false); ensure setArticles is only set on success. Then
update the JSX where the articles grid renders to conditionally show a
user-facing error message when error is truthy (and hide the empty grid), while
preserving the existing loading indicator controlled by setIsLoading.
- Around line 120-135: Calling getAllIdeas() inside the client-side useEffect is
invalid because getAllIdeas (and fast-glob/path usage) relies on Node.js
filesystem APIs and will break in the browser; revert the data load back to
getStaticProps to keep the MDX ideas fetched at build time, or move the
server-only logic into a new API route (e.g., /api/ideas) that calls getAllIdeas
server-side and have the useEffect fetch that endpoint; also add an error state
alongside setIsLoading and setArticles (update state setters referenced in the
component such as setArticles and setIsLoading) so failures are surfaced to the
user instead of silently logging with console.error.

In `@src/pages/ideas/index.jsx`:
- Around line 119-136: Replace the unused state for currentYear with a stable
value (use const currentYear = getCurrentYear() or useMemo) so you can remove
currentYear from the useEffect dependency list; add an error state (e.g., const
[error, setError] = useState(null)) and in fetchData's catch call
setError(error.message || String(error)) and optionally setArticles([]) as
appropriate; keep setIsLoading(false) in finally; update the component render to
show a visible error branch (e.g., when error is truthy render a centered red
message) so users see failures from fetchIdeasFromGitHub instead of an empty
Grid.

In `@src/pages/index.jsx`:
- Around line 94-96: The current timer callback uses projects.sort(...), which
mutates the imported module array; change it to sort a shallow copy instead
(e.g., use [...projects] or projects.slice() before .sort) inside the timer
callback that calls setRandomProjects and setIsLoading so the module-level
projects array is not mutated across consumers or HMR reloads.

---

Outside diff comments:
In `@src/pages/_app.jsx`:
- Around line 37-62: The current useEffect toggles loading and unmounts the
current page by switching from rendering <Component> to <PageLoader>; instead,
keep <Component previousPathname={previousPathname} {...pageProps} /> mounted
and render the route loader as a non-blocking indicator (e.g., integrate
nprogress or a top-bar progress component) so routeChangeStart triggers
nprogress.start() and routeChangeComplete/routeChangeError call
nprogress.done(); update the JSX to always render <Component /> and
conditionally render PageLoader only as an overlay/progress indicator (not
full-page replacement), and remove any artificial per-page 500ms timers in
page-level components that rely on isLoading so pages with no async data don’t
double-load.

In `@src/pages/ideas/`[year]/[slug].jsx:
- Around line 64-72: Remove the now-dead SSG fallback branch: delete the
router.isFallback conditional and its JSX return (the Container block that
renders "Loading...") since getStaticPaths was removed and this branch can never
be reached; update any imports or surrounding code in the [year]/[slug].jsx
component if they become unused after removing the router.isFallback block.
- Around line 195-206: The current custom renderer uses the removed inline prop
on the code component so every snippet renders as a block; replace that logic by
providing both a pre renderer for block-level code and a code renderer for
inline code inside the ReactMarkdown components prop: implement components.pre
to render the block wrapper (using the block classes you already have: "block
bg-zinc-100 ... font-mono my-4" and forward children/props), and implement
components.code to render inline code (using the inline classes "bg-zinc-100 ...
text-green-600 ... font-mono" and forward className/children/props); remove
reliance on the inline argument and ensure className and children are forwarded
correctly in both pre and code renderers so fenced code blocks get the pre
styling and single-backtick inline code gets the inline styling.

---

Nitpick comments:
In `@src/components/Skeletons/Skeleton.jsx`:
- Around line 4-12: The variantStyles object is recreated on every render inside
the forwardRef Skeleton component; hoist it to module scope as a top-level
constant (e.g., VARIANT_STYLES) and have the Skeleton function reference that
constant instead of declaring variantStyles inline so you avoid repeated
allocations; update any usages in the Skeleton component to read from the new
top-level VARIANT_STYLES.

In `@src/components/SuspenseWrapper.jsx`:
- Around line 14-18: The Suspense fallback expression currently uses fallback ||
defaultFallback which will incorrectly override valid falsy values; update the
JSX in the SuspenseWrapper component to use the nullish coalescing operator
(fallback ?? defaultFallback) so only null or undefined fallbacks are
replaced—adjust the expression passed to the Suspense fallback prop (referencing
fallback, defaultFallback, and Suspense).

In `@src/pages/_app.jsx`:
- Around line 1-10: Remove the unused imports to clean up the module: delete the
unused React symbol Suspense from the named imports and remove the unused
Skeleton import from '@/components/Skeletons' so only used symbols (useEffect,
useRef, useState, useRouter, Header, Footer) remain; update the import
statements at the top of the module to drop Suspense and the Skeleton import
line.

In `@src/pages/about.jsx`:
- Around line 174-188: The loading branch duplicates inline container styles;
replace the inline style object on the outer <div> in the isLoading return with
the existing styles.container constant to avoid duplication and ensure
consistency (update the JSX that renders AboutSkeleton, Banner and
Container.Outer to use styles.container instead of the inline style).
- Around line 50-55: Remove the artificial 500ms loading delay and the
now-unnecessary isLoading state: delete the useEffect timer block that calls
setIsLoading, remove the isLoading state variable and any conditional rendering
that shows the skeleton when isLoading is true (in src/pages/about.jsx), and
render the About page content directly (still respecting isDarkMode where used).
Ensure any references to setIsLoading or isLoading are cleaned up so there are
no unused state variables or dead branches left.

In `@src/pages/apply.jsx`:
- Around line 11-31: TimelineSkeleton defines a constant items = [...] but the
map callback ignores each entry (uses (_, index)), so the array contents are
unused; fix by either using the defined items inside the map (e.g., map((item,
index) => ...) and render item.title/item.description) or replace the items
array with a simple Array.from({ length: 5 }) and keep map((_, index) => ...);
update the map callback in TimelineSkeleton accordingly so the code no longer
defines unused values.

In `@src/pages/ideas/`[year]/[slug].jsx:
- Line 11: Remove the unused getCurrentYear import from the import statement
(leave fetchIdeaContent) or, if you intended to use it, actually call
getCurrentYear where the current year is needed (e.g., for defaulting or
validating the [year] route param); update the import to only import
fetchIdeaContent if you choose removal and ensure there are no other references
to getCurrentYear in this component.

In `@src/pages/index.jsx`:
- Around line 92-98: The initial setIsLoading(true) inside the useEffect is
redundant because isLoading is initialized to true and the effect runs only on
mount; remove the top-level setIsLoading(true) call in the useEffect that
contains setRandomProjects and setIsLoading(false), leaving the timeout logic
(setRandomProjects(projects.sort(() => 0.5 - Math.random()).slice(0, 3)) and
setIsLoading(false)) intact and keep the cleanup return () =>
clearTimeout(timer) so the effect still runs once and clears the timer properly.

In `@src/pages/projects.jsx`:
- Around line 15-16: Consolidate the two separate imports into a single import
statement by combining Skeleton and SkeletonText: replace the two lines
importing Skeleton and SkeletonText from '@/components/Skeletons' with one
import that imports both symbols (Skeleton, SkeletonText) from the same module
to remove the duplicate import.
- Around line 185-192: The 500ms artificial delay in Projects page is
unnecessary because data is synchronous; remove the useEffect timer and the
isLoading state (useState/setIsLoading) or set isLoading to false immediately so
the component renders content without delay; update render logic that checks
isLoading to show content directly (or keep the skeleton only for true async
pages) and delete the timer cleanup code referencing timer/clearTimeout.

Comment on lines +6 to +10
{Array.from({ length: count }).map((_, index) => (
<span key={index}>
<SkeletonCard />
</span>
))}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

<span> wrapping a block <div> is invalid HTML.

SkeletonCard renders a <div>, so wrapping it in a <span> produces an invalid block-in-inline nesting. The wrapper is also unnecessary since key can be applied directly to <SkeletonCard />.

🐛 Proposed fix
-      {Array.from({ length: count }).map((_, index) => (
-        <span key={index}>
-          <SkeletonCard />
-        </span>
-      ))}
+      {Array.from({ length: count }).map((_, index) => (
+        <SkeletonCard key={index} />
+      ))}
📝 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
{Array.from({ length: count }).map((_, index) => (
<span key={index}>
<SkeletonCard />
</span>
))}
{Array.from({ length: count }).map((_, index) => (
<SkeletonCard key={index} />
))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Skeletons/SkeletonGrid.jsx` around lines 6 - 10, The wrapper
<span> around each <SkeletonCard /> produces invalid HTML because SkeletonCard
renders a <div>; remove the <span> and apply the key directly to the
<SkeletonCard /> inside the Array.from map in SkeletonGrid.jsx (i.e., change the
mapping from <span key={index}><SkeletonCard /></span> to <SkeletonCard
key={index} /> or wrap in a fragment if needed).

Comment on lines +21 to +31
export function PageLoadingSkeleton() {
return (
<div className="animate-pulse">
<div className="h-16 bg-gray-200 dark:bg-gray-700 mb-8" />
<div className="space-y-4">
<Skeleton height="3rem" width="60%" className="mx-auto" />
<Skeleton height="1.5rem" width="80%" className="mx-auto" />
<Skeleton height="1.5rem" width="70%" className="mx-auto" />
</div>
</div>
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Redundant animate-pulse on outer wrapper causes compounded opacity animation.

Skeleton already applies animate-pulse internally. The wrapper div at Line 23 also applies it, so at the 50% keyframe the child Skeleton elements render at ~25% opacity (0.5 × 0.5) instead of the intended 50%. Remove animate-pulse from the outer container.

🐛 Proposed fix
 export function PageLoadingSkeleton() {
   return (
-    <div className="animate-pulse">
-      <div className="h-16 bg-gray-200 dark:bg-gray-700 mb-8" />
+    <div>
+      <div className="animate-pulse h-16 bg-gray-200 dark:bg-gray-700 mb-8" />
       <div className="space-y-4">
📝 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
export function PageLoadingSkeleton() {
return (
<div className="animate-pulse">
<div className="h-16 bg-gray-200 dark:bg-gray-700 mb-8" />
<div className="space-y-4">
<Skeleton height="3rem" width="60%" className="mx-auto" />
<Skeleton height="1.5rem" width="80%" className="mx-auto" />
<Skeleton height="1.5rem" width="70%" className="mx-auto" />
</div>
</div>
)
export function PageLoadingSkeleton() {
return (
<div>
<div className="animate-pulse h-16 bg-gray-200 dark:bg-gray-700 mb-8" />
<div className="space-y-4">
<Skeleton height="3rem" width="60%" className="mx-auto" />
<Skeleton height="1.5rem" width="80%" className="mx-auto" />
<Skeleton height="1.5rem" width="70%" className="mx-auto" />
</div>
</div>
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SuspenseWrapper.jsx` around lines 21 - 31, The outer wrapper
in PageLoadingSkeleton currently uses the animate-pulse class which duplicates
the animation already applied inside each Skeleton, causing compounded opacity;
remove animate-pulse from the outer wrapper div (leave other classes like
spacing/background intact) so only the Skeleton components control the pulse
animation and restore the intended 50% keyframe opacity.

Comment on lines +120 to +135
useEffect(() => {
const fetchData = async () => {
setIsLoading(true);
try {
const ideas = await getAllIdeas();
const mappedIdeas = ideas.map(({ component, ...meta }) => meta);
setArticles(mappedIdeas);
} catch (error) {
console.error('Error fetching ideas:', error);
} finally {
setIsLoading(false);
}
};

fetchData();
}, []);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and examine getAllIdeas function
fd -e js -e ts getAllIdeas --exec cat {}

Repository: AOSSIE-Org/website

Length of output: 2237


🏁 Script executed:

#!/bin/bash
# Also check the full file to understand context
cat -n src/pages/ideas/2024/index.jsx | head -150

Repository: AOSSIE-Org/website

Length of output: 6605


🏁 Script executed:

#!/bin/bash
# Search for getAllIdeas import and usage
rg -B2 -A2 "getAllIdeas" src/pages/ideas/2024/index.jsx

Repository: AOSSIE-Org/website

Length of output: 518


getAllIdeas() is a Node.js-only function and will break at runtime when called client-side.

fast-glob traverses the filesystem using Node.js APIs, and getAllIdeas also calls path.join(process.cwd(), ...), both of which are Node.js-only. When moved from getStaticProps into useEffect, this function will execute in the browser, where process.cwd() and filesystem I/O are not available. The module cannot be tree-shaken out of the client bundle when imported directly into a component.

Additionally, the error is silently swallowed (line 128: console.error only) with no error state. On failure, setArticles is never called, leaving an empty list displayed to the user with no indication of what went wrong.

Options (in preference order):

  1. Revert to getStaticProps — the ideas list is static MDX content; there is no need to fetch it at runtime.
  2. Create an API route (/api/ideas) that calls getAllIdeas server-side and fetch from it in useEffect.

If keeping useEffect, also add an error state to communicate failures to the user.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/2024/index.jsx` around lines 120 - 135, Calling getAllIdeas()
inside the client-side useEffect is invalid because getAllIdeas (and
fast-glob/path usage) relies on Node.js filesystem APIs and will break in the
browser; revert the data load back to getStaticProps to keep the MDX ideas
fetched at build time, or move the server-only logic into a new API route (e.g.,
/api/ideas) that calls getAllIdeas server-side and have the useEffect fetch that
endpoint; also add an error state alongside setIsLoading and setArticles (update
state setters referenced in the component such as setArticles and setIsLoading)
so failures are surfaced to the user instead of silently logging with
console.error.

Comment on lines +127 to +130
} catch (error) {
console.error('Error fetching ideas:', error);
} finally {
setIsLoading(false);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing error state leaves users staring at an empty grid on fetch failure.

When getAllIdeas() throws, setArticles is never called (stays []), isLoading flips to false via finally, and the page silently renders an empty grid. Add an error state and display a user-facing message.

🐛 Proposed fix
 export default function Ideas() {
     const [articles, setArticles] = useState([]);
     const [isLoading, setIsLoading] = useState(true);
+    const [error, setError] = useState(null);

     useEffect(() => {
         const fetchData = async () => {
             setIsLoading(true);
+            setError(null);
             try {
                 const ideas = await getAllIdeas();
                 const mappedIdeas = ideas.map(({ component, ...meta }) => meta);
                 setArticles(mappedIdeas);
             } catch (error) {
                 console.error('Error fetching ideas:', error);
+                setError('Failed to load ideas. Please try again later.');
             } finally {
                 setIsLoading(false);
             }
         };
         fetchData();
     }, []);

Then render the error where the grid appears:

-                        {articles.map((article) => (
+                        {error ? (
+                            <p className="text-red-500 font-mono">{error}</p>
+                        ) : articles.map((article) => (
                             <Article key={article.slug} article={article} />
-                        ))}
+                        ))}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/2024/index.jsx` around lines 127 - 130, Add a new error state
(e.g., const [error, setError] = useState(null)) and update the data-fetching
catch block in the getAllIdeas flow to call setError(error) (or
setError(error.message)) before the finally that calls setIsLoading(false);
ensure setArticles is only set on success. Then update the JSX where the
articles grid renders to conditionally show a user-facing error message when
error is truthy (and hide the empty grid), while preserving the existing loading
indicator controlled by setIsLoading.

Comment on lines +119 to +136
const [currentYear, setCurrentYear] = useState(getCurrentYear());
const [isLoading, setIsLoading] = useState(true);

useEffect(() => {
const fetchData = async () => {
setIsLoading(true);
try {
const ideas = await fetchIdeasFromGitHub(currentYear);
setArticles(ideas);
} catch (error) {
console.error('Error fetching ideas:', error);
} finally {
setIsLoading(false);
}
};

fetchData();
}, [currentYear]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

setCurrentYear is never called, and fetch errors produce an empty grid with no user feedback.

Two issues here:

  1. Dead setter: currentYear is initialized once from getCurrentYear() and setCurrentYear is never invoked. The year never changes, so useState is unnecessary — a plain const currentYear = getCurrentYear() (or useMemo) is sufficient, and dropping the state variable removes currentYear from the useEffect dependency array.

  2. Silent failure: When fetchIdeasFromGitHub rejects (API rate-limit, network error, etc.), the catch block only logs the error and leaves articles as []. The rendered output is an empty <Grid> with no error message, which gives users no indication that something went wrong.

🐛 Proposed fix
-  const [currentYear, setCurrentYear] = useState(getCurrentYear());
+  const currentYear = getCurrentYear();
   const [isLoading, setIsLoading] = useState(true);
+  const [error, setError] = useState(null);
 
   useEffect(() => {
     const fetchData = async () => {
       setIsLoading(true);
+      setError(null);
       try {
         const ideas = await fetchIdeasFromGitHub(currentYear);
         setArticles(ideas);
       } catch (error) {
         console.error('Error fetching ideas:', error);
+        setError('Failed to load ideas. Please try again later.');
       } finally {
         setIsLoading(false);
       }
     };
 
     fetchData();
-  }, [currentYear]);
+  }, []);

Then add an error branch to the render:

{error && (
  <p className="text-center text-red-600 dark:text-red-400">{error}</p>
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/index.jsx` around lines 119 - 136, Replace the unused state
for currentYear with a stable value (use const currentYear = getCurrentYear() or
useMemo) so you can remove currentYear from the useEffect dependency list; add
an error state (e.g., const [error, setError] = useState(null)) and in
fetchData's catch call setError(error.message || String(error)) and optionally
setArticles([]) as appropriate; keep setIsLoading(false) in finally; update the
component render to show a visible error branch (e.g., when error is truthy
render a centered red message) so users see failures from fetchIdeasFromGitHub
instead of an empty Grid.

Comment on lines 94 to 96
const timer = setTimeout(() => {
setRandomProjects(projects.sort(() => 0.5 - Math.random()).slice(0, 3))
setIsLoading(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

projects.sort() mutates the shared imported module array in place.

Array.prototype.sort mutates the original array. Since projects is a module-level import, this permanently reorders it for every consumer in the same module graph (other pages, future effects, HMR reloads, etc.).

🐛 Proposed fix — sort a shallow copy
-      setRandomProjects(projects.sort(() => 0.5 - Math.random()).slice(0, 3))
+      setRandomProjects([...projects].sort(() => 0.5 - Math.random()).slice(0, 3))
📝 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
const timer = setTimeout(() => {
setRandomProjects(projects.sort(() => 0.5 - Math.random()).slice(0, 3))
setIsLoading(false)
const timer = setTimeout(() => {
setRandomProjects([...projects].sort(() => 0.5 - Math.random()).slice(0, 3))
setIsLoading(false)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/index.jsx` around lines 94 - 96, The current timer callback uses
projects.sort(...), which mutates the imported module array; change it to sort a
shallow copy instead (e.g., use [...projects] or projects.slice() before .sort)
inside the timer callback that calls setRandomProjects and setIsLoading so the
module-level projects array is not mutated across consumers or HMR reloads.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/pages/ideas/[year]/[slug].jsx (2)

64-72: ⚠️ Potential issue | 🟡 Minor

router.isFallback is dead code — getStaticPaths was removed in this PR.

router.isFallback is only ever true during on-demand ISR generation when getStaticPaths exports fallback: true. Since getStaticPaths was deleted as part of this migration, this branch can never execute and can be safely removed.

🐛 Proposed fix
-  if (router.isFallback) {
-    return (
-      <Container className="mt-16 mb-20">
-        <div className="text-center">
-          <p className="text-lg text-zinc-600 dark:text-zinc-400">Loading...</p>
-        </div>
-      </Container>
-    );
-  }
-
   if (isLoading) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx around lines 64 - 72, The
router.isFallback branch in the component is dead code because getStaticPaths
was removed; remove the entire conditional block that checks router.isFallback
(and the accompanying Loading... JSX) and any now-unused references to
router/useRouter in this file (e.g., the useRouter() call and its import) so the
component no longer contains an unreachable fallback rendering path.

190-201: ⚠️ Potential issue | 🟠 Major

inline prop was removed in react-markdown v9 — inline code is always rendered as a block.

The code component no longer accepts inline as a prop. Prior to 9.0.0, react-markdown exposed the inline prop to code components; this functionality was removed in 9.0.0. With react-markdown v10.1.0, inline will always be undefined, so the ternary at line 191 always falls into the block-code branch, meaning backtick-quoted inline code (e.g., `foo`) will be rendered as a block element.

The idiomatic v9+/v10 fix is to check className for a language-* pattern to distinguish fenced code from inline:

🐛 Proposed fix
-code: ({ node, inline, ...props }) =>
-  inline ? (
-    <code
-      className="bg-zinc-100 dark:bg-zinc-800 text-green-600 dark:text-yellow-400 px-2 py-1 rounded text-sm font-mono"
-      {...props}
-    />
-  ) : (
-    <code
-      className="block bg-zinc-100 dark:bg-zinc-800 text-zinc-800 dark:text-zinc-200 p-4 rounded-lg overflow-x-auto text-sm font-mono my-4"
-      {...props}
-    />
-  ),
+code: ({ node, className, children, ...props }) => {
+  const isBlock = /language-(\w+)/.exec(className || '');
+  return isBlock ? (
+    <code
+      className={`block bg-zinc-100 dark:bg-zinc-800 text-zinc-800 dark:text-zinc-200 p-4 rounded-lg overflow-x-auto text-sm font-mono my-4 ${className}`}
+      {...props}
+    >
+      {children}
+    </code>
+  ) : (
+    <code
+      className="bg-zinc-100 dark:bg-zinc-800 text-green-600 dark:text-yellow-400 px-2 py-1 rounded text-sm font-mono"
+      {...props}
+    >
+      {children}
+    </code>
+  );
+},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx around lines 190 - 201, The code renderer
passed to react-markdown still checks the removed "inline" prop; update the code
renderer (the code: ({ node, inline, ...props }) => ...) to instead destructure
className (e.g. code: ({ node, className, ...props }) => { const isCodeBlock =
className && /language-/.test(className); return isCodeBlock ? /* block code
markup with fenced styling */ : /* inline code markup with inline styling */ }).
Replace the ternary that uses inline with a regex test on className
(/language-/) to distinguish fenced code blocks from inline backtick code and
apply the corresponding className/markup.
🧹 Nitpick comments (2)
src/pages/ideas/[year]/[slug].jsx (1)

48-59: Component-level catch is unreachable — fetchIdeaContent swallows all errors internally.

fetchIdeaContent (in src/helper/fetchGitHubIdeas.js) catches every error and returns null, so the catch block here never executes. setIdea(null) is also a no-op since null is the initial state. Consider either removing the redundant try/catch or having fetchIdeaContent re-throw so the component can surface fetch failures to the user.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/`[year]/[slug].jsx around lines 48 - 59, The component-level
try/catch in fetchData is ineffective because fetchIdeaContent swallows errors
and returns null; either remove the redundant try/catch in fetchData (inside the
component where fetchData sets setIsLoading, setIdea) or change fetchIdeaContent
(in src/helper/fetchGitHubIdeas.js) to re-throw after logging so the catch in
fetchData can run — specifically update fetchIdeaContent to throw the caught
error (or return a result/throw pattern) and keep fetchData's try/catch to
setIsLoading(false) and setIdea(null)/show an error state, or remove the
try/catch in fetchData and rely on fetchIdeaContent's null return and handle
that case after awaiting the call.
src/pages/ideas/2024/index.jsx (1)

78-114: ArticleSkeleton and IdeasGridSkeleton are duplicated verbatim in src/pages/ideas/index.jsx.

Consider extracting both into src/components/Skeletons/ (alongside the existing SkeletonCard/SkeletonGrid) so they can be shared.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/ideas/2024/index.jsx` around lines 78 - 114, The ArticleSkeleton
and IdeasGridSkeleton components are duplicated; extract both functions into a
shared component module (e.g., a new Skeletons component alongside the existing
SkeletonCard/SkeletonGrid), export them (ArticleSkeleton and IdeasGridSkeleton),
replace the inline definitions in this file and the other duplicate file with
imports of those exports, and update usages to render the imported
ArticleSkeleton and IdeasGridSkeleton so the duplicated JSX is centralized and
reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/pages/ideas/`[year]/[slug].jsx:
- Around line 64-72: The router.isFallback branch in the component is dead code
because getStaticPaths was removed; remove the entire conditional block that
checks router.isFallback (and the accompanying Loading... JSX) and any
now-unused references to router/useRouter in this file (e.g., the useRouter()
call and its import) so the component no longer contains an unreachable fallback
rendering path.
- Around line 190-201: The code renderer passed to react-markdown still checks
the removed "inline" prop; update the code renderer (the code: ({ node, inline,
...props }) => ...) to instead destructure className (e.g. code: ({ node,
className, ...props }) => { const isCodeBlock = className &&
/language-/.test(className); return isCodeBlock ? /* block code markup with
fenced styling */ : /* inline code markup with inline styling */ }). Replace the
ternary that uses inline with a regex test on className (/language-/) to
distinguish fenced code blocks from inline backtick code and apply the
corresponding className/markup.

---

Duplicate comments:
In `@src/pages/ideas/2024/index.jsx`:
- Around line 127-130: The fetch error is currently only console.logged in the
catch block causing an empty grid with no user feedback; add an error state
(e.g., error / setError) in the component, setError(error) inside the catch of
the fetch routine (the catch around "Error fetching ideas:"), and update the
render logic to show a user-facing error message or call-to-action when error is
non-null (instead of just rendering the empty articles grid); keep
setIsLoading(false) in finally so loading behaves the same.
- Around line 120-135: getAllIdeas uses Node-only APIs and must not be invoked
from the browser; replace the direct client-side call inside useEffect/fetchData
with a server-side data fetch (e.g., move the getAllIdeas call into
getStaticProps or getServerSideProps and pass the mapped articles as props to
the component) or expose a server API route that calls getAllIdeas and have
fetchData call that API endpoint instead; update the component to consume the
props or the API response and remove any direct use of process.cwd()/fast-glob
from client-side code (look for getAllIdeas, useEffect, and fetchData to apply
the change).

In `@src/pages/ideas/index.jsx`:
- Around line 118-135: The currentYear state (useState/getCurrentYear and
setCurrentYear) is never updated so keeping setCurrentYear and listing
currentYear in useEffect dependencies is dead code; change to a plain constant
(const currentYear = getCurrentYear()) and remove currentYear from the
dependency array of the useEffect that defines fetchData (which calls
fetchIdeasFromGitHub and setArticles), leaving the effect to run only on mount
(or add any real changing deps if intended).
- Around line 125-130: fetchIdeasFromGitHub currently swallows errors so the
component never surfaces failures; update the page component to detect and
surface load failures by adding an error state (e.g., setError) and, after
calling fetchIdeasFromGitHub, check if the returned ideas array is empty or null
and set that error state (and optionally log the underlying problem) before
calling setArticles; also keep the existing catch in place for future re-throws
from fetchIdeasFromGitHub so you cover both cases—reference symbols:
fetchIdeasFromGitHub, setArticles, setIsLoading, and add setError (or similar)
to the component and use it to render an error UI instead of an empty grid.

---

Nitpick comments:
In `@src/pages/ideas/`[year]/[slug].jsx:
- Around line 48-59: The component-level try/catch in fetchData is ineffective
because fetchIdeaContent swallows errors and returns null; either remove the
redundant try/catch in fetchData (inside the component where fetchData sets
setIsLoading, setIdea) or change fetchIdeaContent (in
src/helper/fetchGitHubIdeas.js) to re-throw after logging so the catch in
fetchData can run — specifically update fetchIdeaContent to throw the caught
error (or return a result/throw pattern) and keep fetchData's try/catch to
setIsLoading(false) and setIdea(null)/show an error state, or remove the
try/catch in fetchData and rely on fetchIdeaContent's null return and handle
that case after awaiting the call.

In `@src/pages/ideas/2024/index.jsx`:
- Around line 78-114: The ArticleSkeleton and IdeasGridSkeleton components are
duplicated; extract both functions into a shared component module (e.g., a new
Skeletons component alongside the existing SkeletonCard/SkeletonGrid), export
them (ArticleSkeleton and IdeasGridSkeleton), replace the inline definitions in
this file and the other duplicate file with imports of those exports, and update
usages to render the imported ArticleSkeleton and IdeasGridSkeleton so the
duplicated JSX is centralized and reused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant