feat: comprehensive performance optimization#231
Open
Opulencechuks wants to merge 2 commits into
Open
Conversation
- Implemented route-based code splitting and dynamic imports for heavy components - Added a Service Worker for caching static assets and critical resources - Integrated Core Web Vitals monitoring with a new PerformanceMonitor component - Optimized large list rendering using CSS content-visibility - Added preloading for critical resources and improved font loading - Implemented CI performance budgets and bundle size monitoring
|
@Opulencechuks Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdds frontend performance work: CI bundle-size check, React strict mode, a service worker with cache-first fetch, client PerformanceMonitor for Web Vitals, dynamic loading of heavy providers/components, rendering containment hints, and service worker registration in the root layout. (50 words) Changes
Sequence DiagramssequenceDiagram
participant Client as Browser Client
participant SW as Service Worker
participant Cache as Browser Cache
participant Network as Network/Server
Client->>SW: Request resource
SW->>Cache: Check stellovault-cache-v1
alt Cache Hit
Cache-->>SW: Cached response
SW-->>Client: Return cached response
else Cache Miss
SW->>Network: Fetch resource
Network-->>SW: Return response
SW->>Cache: Store response in stellovault-cache-v1
SW-->>Client: Return network response
end
sequenceDiagram
participant App as App Init
participant PerfMon as PerformanceMonitor
participant WebVitals as Web Vitals API
participant Console as Browser Console
App->>PerfMon: Register web-vitals callback
WebVitals->>PerfMon: Emit metric (e.g., LCP)
PerfMon->>Console: Log metric name & value
PerfMon->>PerfMon: Compare value to budget
alt Exceeds budget
PerfMon->>Console: Log warning with value & threshold
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/providers/AppProviders.tsx (1)
3-11:⚠️ Potential issue | 🔴 CriticalCritical:
ssr: falseon a wrapper that renders{children}disables SSR for the entire app.
AppProvidersis used at the root offrontend/src/app/layout.tsxand wraps the entire page tree includingPerformanceMonitor,ServiceWorkerRegistration, and{children}. Withnext/dynamic(..., { ssr: false }), the component—and all its descendants—never render on the server. Every page's content will be missing from the initial HTML, forcing the browser to hydrate everything client-side after the OnboardingProvider bundle loads. This directly regresses FCP/LCP and harms SEO (opposite of this PR's goal).
OnboardingProvideritself is already marked"use client"and is fully compatible with server rendering. The only SSR-incompatible parts arereact-joyrideand thewindowAPI calls in theuseEffecthooks (lines 214, 220, 232, 235).Fix: Keep the context provider in the server-rendered tree and lazy-load only the Joyride widget inside it.
Move
dynamic(..., { ssr: false })from AppProviders.tsx into OnboardingProvider.tsx to wrap only the<Joyride>component itself. This allows the provider context to render server-side while the heavy, browser-dependent widget stays client-only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/providers/AppProviders.tsx` around lines 3 - 11, AppProviders currently uses next/dynamic with ssr: false which prevents server rendering of the entire tree; remove the dynamic import from AppProviders so AppProviders returns <OnboardingProvider>{children}</OnboardingProvider> server-side, and instead move the dynamic(..., { ssr: false }) into OnboardingProvider to wrap only the browser-only Joyride widget (the <Joyride> element and any window-using effects), keeping the OnboardingProvider context server-renderable while lazy-loading the Joyride component.
🧹 Nitpick comments (3)
.github/workflows/performance.yml (1)
20-23: Usenpm ciand enable npm cache for deterministic, faster CI.
npm installignorespackage-lock.jsondrift and is noticeably slower thannpm ci; combined withactions/setup-node's built-in npm cache, builds become both reproducible and faster.♻️ Suggested
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: '20' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json - name: Install dependencies run: | cd frontend - npm install + npm ci🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/performance.yml around lines 20 - 23, The CI step named "Install dependencies" currently runs "cd frontend" then "npm install"; change this to use "npm ci" for deterministic installs and faster performance, and enable npm caching by configuring the existing actions/setup-node step to use the npm cache (set its cache input to "npm") so the frontend install benefits from action-level caching; update the "Install dependencies" step to call "npm ci" instead of "npm install" and ensure actions/setup-node has cache: 'npm'.frontend/next.config.ts (1)
5-6: Remove staleswcMinifycomment — this config option was removed in Next.js 15.SWC minification is now always enabled by default in Next.js 15+ and cannot be toggled. The comment misleads future readers into thinking it's still a valid configuration option.
🔧 Suggested change
reactStrictMode: true, - /* Enable SWC minification for faster builds and smaller bundles */ - // swcMinify: true, // Next.js 13+ enables this by default, but keeping it explicit if needed. };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/next.config.ts` around lines 5 - 6, Remove the stale swcMinify comment in next.config.ts because Next.js 15 removed that option; locate the commented lines containing "swcMinify: true" (or its surrounding explanatory comment) and delete them so the file no longer suggests a togglable SWC minification setting.frontend/src/components/PerformanceMonitor.tsx (1)
6-22: Hoist thebudgetsmap out of the callback.
budgetsis a static configuration object, but it's reallocated on every metric event. Move it to module scope (or at least outside the callback) to avoid needless allocations and to make it easy to reuse/export for tests.♻️ Proposed refactor
-import { useReportWebVitals } from 'next/web-vitals'; +import { useReportWebVitals } from 'next/web-vitals'; + +const BUDGETS: Record<string, number> = { + FCP: 1500, + LCP: 2500, + CLS: 0.1, + INP: 200, + TTFB: 600, +}; export function PerformanceMonitor() { useReportWebVitals((metric) => { console.log(`[Performance Metric] ${metric.name}:`, metric.value); - - const budgets: Record<string, number> = { - FCP: 1500, - LCP: 2500, - CLS: 0.1, - FID: 100, - TTFB: 600, - }; - - if (budgets[metric.name] && metric.value > budgets[metric.name]) { - console.warn(...); - } + const budget = BUDGETS[metric.name]; + if (budget !== undefined && metric.value > budget) { + console.warn(`[Performance Warning] ${metric.name} exceeded budget: ${metric.value} (Budget: ${budget})`); + } }); return null; }Additionally, since this logs to
consoleon every metric, consider gating the logs behindprocess.env.NODE_ENV !== 'production'(or shipping the metrics to an analytics endpoint as the TODO comment suggests) to avoid console noise in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/PerformanceMonitor.tsx` around lines 6 - 22, The budgets map is recreated on every metric callback; move the const budgets = { FCP:1500, LCP:2500, CLS:0.1, FID:100, TTFB:600 } out of the useReportWebVitals callback to module scope (top of PerformanceMonitor.tsx) so it's allocated once and can be exported for tests, then update the callback in useReportWebVitals to reference the hoisted budgets; also gate console.log/console.warn behind a NODE_ENV check (e.g., only log when process.env.NODE_ENV !== 'production') or replace with a metrics-shipping call as indicated by the TODO.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/performance.yml:
- Around line 41-47: Replace the no-op Lighthouse CI step with a real lhci
autorun invocation: update the workflow step named "Lighthouse CI" to build the
frontend, start a preview server (e.g., run the build then `next start` or `npm
run start`) in the background, and invoke `lhci autorun
--config=lighthouserc.js`; add a minimal lighthouserc.js (referenced by name)
that defines the CI server URL, budgets and thresholds matching issue `#208`
(performance ≥ 90, FCP < 1.5s, LCP < 2.5s) so lhci enforces them; alternatively,
if you don't want CI enforcement yet, remove the step entirely to avoid
misleading coverage.
- Around line 30-39: The Check Bundle Size step currently assigns SIZE from a
glob that can yield multiple files (main-*.js) causing malformed integer
comparisons and also doesn't fail CI; change the logic in that step to (1)
aggregate sizes for all matching .next/static/chunks/main-*.js files (e.g., sum
the du -sk outputs) into a single numeric SIZE, (2) perform a numeric comparison
against the 500KB budget using that summed SIZE, and (3) enforce the budget by
exiting with non-zero status (re-enable exit 1) when SIZE exceeds the threshold;
update references to the SIZE variable and the main-*.js glob handling in the
Check Bundle Size step accordingly.
In `@frontend/public/sw.js`:
- Around line 1-14: The install handler is failing because ASSETS_TO_CACHE
includes an invalid static path ('/globals.css') which causes cache.addAll in
the install event to reject; update the precache list (ASSETS_TO_CACHE) to only
include stable, guaranteed URLs (e.g., '/', '/favicon.ico') and remove
hashed/build-time assets like '/globals.css', and instead implement runtime
caching in the service worker's fetch handler for Next.js hashed assets (or
integrate next-pwa/Workbox to inject the real build manifest) so CACHE_NAME and
the install event no longer fail atomically on 404s.
- Around line 16-22: The service worker's fetch handler and cache lifecycle are
too broad and cause stale HTML to be served; update the fetch listener
(self.addEventListener('fetch', ...)) to only intercept same-origin GET requests
and to treat navigation/HTML requests with a network-first or skip-cache
strategy (do not unconditionally return caches.match(event.request) for
navigation requests), while retaining a cache-first policy for static assets;
also add an activate listener (self.addEventListener('activate', ...)) that
iterates caches.keys() and deletes any cache whose name !== CACHE_NAME to remove
old v1/v2 caches, and consider calling self.clients.claim() and
self.skipWaiting() appropriately to control update timing.
In `@frontend/src/app/layout.tsx`:
- Around line 33-35: Remove the favicon preload link in the <head> of
layout.tsx: delete the <link rel="preload" href="/favicon.ico" as="image" />
element and either omit explicit favicon markup (rely on Next.js automatic
app/favicon.ico handling) or, if you need an explicit tag, replace it with a
standard <link rel="icon" href="/favicon.ico" />; update the head markup inside
the RootLayout (or the component that returns the <head>) accordingly so there
are no unused-preload warnings.
In `@frontend/src/components/PerformanceMonitor.tsx`:
- Around line 14-20: The performance warning logic incorrectly treats CLS as
milliseconds and still uses deprecated FID; update the budgets object to replace
the FID key with INP (set INP: 200) and set CLS: 0.1 (unitless), change the
budget existence check to use !== undefined instead of truthiness, and adjust
the warning format in the conditional that references metric.name and
metric.value so it appends "ms" only for time-based metrics (e.g., INP, TTFB)
but leaves CLS unitless when logging the breached value and budget.
- Line 3: Update the import to pull useReportWebVitals from 'next/web-vitals'
instead of 'next/navigation'; hoist the budgets object out of the metrics
callback (define it once at module or component scope) and replace the
deprecated "FID" key with "INP" in that budgets map; in the metric handler (the
callback passed to useReportWebVitals, and where you reference
metric.name/metric.value) treat CLS as unitless by removing "ms" and formatting
its value with toFixed(2) only (e.g., for metric.name === 'CLS' use
value.toFixed(2) without "ms"), while keeping "ms" for time-based metrics;
ensure all references use the hoisted budgets object and the updated metric
names.
In `@frontend/src/components/ServiceWorkerRegistration.tsx`:
- Around line 7-18: Update the SW registration guard and logging: only call
navigator.serviceWorker.register during non-local runs by expanding the hostname
check in the window.load handler to skip 'localhost', '127.0.0.1' and any plain
IP address (use a regex like /^\d+\.\d+\.\d+\.\d+$/ to detect IP hosts) and
optionally skip common preview hosts (e.g., host.endsWith('.vercel.app')); then
replace the noisy console.log calls in the navigator.serviceWorker.register
promise handlers with console.debug and/or wrap them in a NODE_ENV ===
'development' check so registration success/failure messages do not spam
production logs (refer to navigator.serviceWorker.register, the load event
callback, and the existing console.log calls in ServiceWorkerRegistration.tsx).
In `@frontend/src/components/transactions/TransactionHistoryDrawer.tsx`:
- Around line 68-74: In TransactionHistoryDrawer (the wrapping div with
className "p-4 border-b..."), replace the fixed containIntrinsicSize value ('0
80px') with an auto intrinsic-size (e.g., "auto 80px") so the browser can cache
variable row heights after first paint and avoid scrollbar jitter for rows with
transaction.hash; while there, reconsider or remove contentVisibility: 'auto' if
the list is short to avoid unnecessary rendering complexity. Ensure you update
the inline style on that div (the element whose style currently sets
contentVisibility and containIntrinsicSize) to use the new
contain-intrinsic-size approach and keep the rest of the styling intact.
---
Outside diff comments:
In `@frontend/src/components/providers/AppProviders.tsx`:
- Around line 3-11: AppProviders currently uses next/dynamic with ssr: false
which prevents server rendering of the entire tree; remove the dynamic import
from AppProviders so AppProviders returns
<OnboardingProvider>{children}</OnboardingProvider> server-side, and instead
move the dynamic(..., { ssr: false }) into OnboardingProvider to wrap only the
browser-only Joyride widget (the <Joyride> element and any window-using
effects), keeping the OnboardingProvider context server-renderable while
lazy-loading the Joyride component.
---
Nitpick comments:
In @.github/workflows/performance.yml:
- Around line 20-23: The CI step named "Install dependencies" currently runs "cd
frontend" then "npm install"; change this to use "npm ci" for deterministic
installs and faster performance, and enable npm caching by configuring the
existing actions/setup-node step to use the npm cache (set its cache input to
"npm") so the frontend install benefits from action-level caching; update the
"Install dependencies" step to call "npm ci" instead of "npm install" and ensure
actions/setup-node has cache: 'npm'.
In `@frontend/next.config.ts`:
- Around line 5-6: Remove the stale swcMinify comment in next.config.ts because
Next.js 15 removed that option; locate the commented lines containing
"swcMinify: true" (or its surrounding explanatory comment) and delete them so
the file no longer suggests a togglable SWC minification setting.
In `@frontend/src/components/PerformanceMonitor.tsx`:
- Around line 6-22: The budgets map is recreated on every metric callback; move
the const budgets = { FCP:1500, LCP:2500, CLS:0.1, FID:100, TTFB:600 } out of
the useReportWebVitals callback to module scope (top of PerformanceMonitor.tsx)
so it's allocated once and can be exported for tests, then update the callback
in useReportWebVitals to reference the hoisted budgets; also gate
console.log/console.warn behind a NODE_ENV check (e.g., only log when
process.env.NODE_ENV !== 'production') or replace with a metrics-shipping call
as indicated by the TODO.
🪄 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
Run ID: c04453df-1412-40f6-b166-a1ece9e00042
📒 Files selected for processing (9)
.github/workflows/performance.ymlfrontend/next.config.tsfrontend/public/sw.jsfrontend/src/app/layout.tsxfrontend/src/app/risk/[wallet]/page.tsxfrontend/src/components/PerformanceMonitor.tsxfrontend/src/components/ServiceWorkerRegistration.tsxfrontend/src/components/providers/AppProviders.tsxfrontend/src/components/transactions/TransactionHistoryDrawer.tsx
anonfedora
approved these changes
Apr 25, 2026
Owner
|
The performance ci you worked on isn't passing |
Author
|
Kindly approve this issue |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/public/sw.js`:
- Around line 1-20: Delete the incomplete first service-worker block: remove the
declarations CACHE_NAME = 'stellovault-cache-v1' and ASSETS_TO_CACHE plus the
associated self.addEventListener('install', ...) and its unfinished
self.addEventListener('fetch', ...) handler so there is only the later, complete
service-worker implementation (which uses "stellovault-mobile-v1"); ensure you
remove the duplicate symbols (CACHE_NAME, ASSETS_TO_CACHE, and the install/fetch
listeners) to avoid duplicate/conflicting service-worker logic and syntax
errors.
In `@frontend/src/app/layout.tsx`:
- Around line 7-19: Add the missing Google font imports and remove the
duplicated AppProviders import: import Geist and Geist_Mono from
"next/font/google" and add that import so the symbols Geist and Geist_Mono used
in the geistSans and geistMono declarations resolve, and delete the duplicate
AppProviders import so AppProviders is only imported once (referencing the
existing AppProviders import and the Geist/Geist_Mono usages).
- Around line 63-69: The JSX has mismatched provider tags: it opens
<TransactionStatusProvider> but closes </LanguageProvider>, causing a syntax
error; update the closing tag to match the opener (replace the incorrect
</LanguageProvider> with </TransactionStatusProvider>) so the provider nesting
around <AppProviders>, <PerformanceMonitor>, <ServiceWorkerRegistration>, and
{children} is correctly balanced and the LanguageProvider (used by
useTranslation()) remains intact elsewhere.
🪄 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
Run ID: e3c213fa-0550-4d96-a010-7bdf463e4236
📒 Files selected for processing (3)
frontend/next.config.tsfrontend/public/sw.jsfrontend/src/app/layout.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/next.config.ts
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
2 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.
This PR closes #208
PR Description: Comprehensive Performance Optimization
This PR implements a series of performance optimizations to improve the Lighthouse score and Core Web Vitals of the StelloVault frontend.
Key Changes
next/dynamicfor heavy components likeScoreHistoryChart(which usesrecharts) andOnboardingProvider(which usesreact-joyride).public/sw.js) to cache static assets and critical resources.ServiceWorkerRegistrationto handle the SW lifecycle.PerformanceMonitorcomponent that usesuseReportWebVitalsto track Core Web Vitals (FCP, LCP, CLS, FID, TTFB).content-visibility: autoandcontain-intrinsic-size. This provides the benefits of virtual scrolling (only rendering what's visible) without the overhead of external libraries.reactStrictModefor better performance profiling in development..github/workflows/performance.yml) to monitor bundle sizes and performance metrics during CI/CD.Impact
Verification
Summary by CodeRabbit
New Features
Performance Improvements
UI Changes
Configuration