Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe pull request refactors the metrics dashboard UI layer by extracting data-fetching logic into dedicated data-driven components, simplifies page-level state management, and consolidates backend version-checking conditions. Backend imports are reorganized, and prefetch strategies are updated to preload metrics and user data across dashboard routes. Changes
Sequence DiagramsequenceDiagram
participant MetricsPage as Metrics Page
participant GlobeSec as GlobeSectionWithData
participant ChartsSec as ChartsSectionWithData
participant AdminApp as useAdminApp
participant Metrics as useMetrics Hook
participant Users as useUsers Hook
rect rgb(200, 220, 240)
Note over MetricsPage: Old Flow (Direct Rendering)
MetricsPage->>Metrics: Direct fetch metrics
MetricsPage->>Users: Direct fetch users
MetricsPage->>MetricsPage: Render charts/globe inline
end
rect rgb(220, 240, 200)
Note over MetricsPage: New Flow (Component Delegation)
MetricsPage->>GlobeSec: Render with Suspense
GlobeSec->>AdminApp: useAdminApp(projectId)
AdminApp->>Metrics: useMetrics(includeAnonymous)
Metrics-->>GlobeSec: metrics data
GlobeSec->>GlobeSec: Render globe
MetricsPage->>ChartsSec: Render with Suspense
ChartsSec->>AdminApp: useAdminApp(projectId)
AdminApp->>Metrics: useMetrics(includeAnonymous)
AdminApp->>Users: useUsers()
Metrics-->>ChartsSec: metrics data
Users-->>ChartsSec: user data
ChartsSec->>ChartsSec: Render charts & tables
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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.
Pull Request Overview
This PR refactors the metrics loading architecture to improve performance and user experience. The key changes include implementing Suspense boundaries for asynchronous metrics data, refreshing metrics cache when users are updated, and enabling prefetching of metrics data.
- Adds metrics cache refresh when users are updated in the admin app
- Implements Suspense-based loading for metrics components with dedicated loading fallbacks
- Enables prefetching of metrics and user data for improved perceived performance
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts | Override _refreshUsers to also refresh metrics cache for consistency |
| apps/dashboard/src/lib/prefetch/url-prefetcher.tsx | Enable prefetching of metrics and user data, remove TODO comments |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/page-client.tsx | Replace metrics-based check with simpler useUsers to determine initial page state |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx | Extract metrics data fetching into separate components wrapped with Suspense |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-loading.tsx | New loading fallback component for Suspense boundaries |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/globe-section-with-data.tsx | New data wrapper component for globe section with metrics data |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/charts-section-with-data.tsx | New data wrapper component for charts section with metrics data |
| apps/backend/src/app/api/latest/internal/metrics/route.tsx | Minor import reordering for consistency |
| apps/backend/src/app/api/latest/check-version/route.ts | Simplify version check logic by consolidating conditions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
55
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | ||
| return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`); | ||
| } | ||
|
|
There was a problem hiding this comment.
This condition is unreachable. Line 49 already checks semver.lt(clientVersion, serverVersion), which catches all cases where the client is older than the server. The condition !semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion logically simplifies to clientVersion < serverVersion, which is already handled. This appears to be a regression from removing the semver.gt check. Consider either removing this unreachable code or restoring the original logic that checked for newer client versions separately.
Suggested change
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | |
| return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`); | |
| } |
Comment on lines
+6
to
+8
|
|
||
| const stackAppInternalsSymbol = Symbol.for("StackAuth--DO-NOT-USE-OR-YOU-WILL-BE-FIRED--StackAppInternals"); | ||
|
|
There was a problem hiding this comment.
The stackAppInternalsSymbol is duplicated across multiple files (globe-section-with-data.tsx, charts-section-with-data.tsx). This symbol is already imported in url-prefetcher.tsx from '@stackframe/stack' (line 4). Consider importing it from the package instead of redefining it to reduce duplication and ensure consistency.
Suggested change
| const stackAppInternalsSymbol = Symbol.for("StackAuth--DO-NOT-USE-OR-YOU-WILL-BE-FIRED--StackAppInternals"); | |
| import { stackAppInternalsSymbol } from '@stackframe/stack'; |
Comment on lines
+4
to
+11
| import { UserAvatar } from '@stackframe/stack'; | ||
| import { fromNow } from '@stackframe/stack-shared/dist/utils/dates'; | ||
| import { Card, CardContent, CardHeader, CardTitle, Table, TableBody, TableCell, TableRow, Typography } from '@stackframe/stack-ui'; | ||
| import { useAdminApp } from '../use-admin-app'; | ||
| import { DonutChartDisplay, LineChartDisplay, LineChartDisplayConfig } from './line-chart'; | ||
|
|
||
| const stackAppInternalsSymbol = Symbol.for("StackAuth--DO-NOT-USE-OR-YOU-WILL-BE-FIRED--StackAppInternals"); | ||
|
|
There was a problem hiding this comment.
The stackAppInternalsSymbol is duplicated across multiple files (globe-section-with-data.tsx, charts-section-with-data.tsx). This symbol is already exported from '@stackframe/stack' and imported in url-prefetcher.tsx. Consider importing it from the package instead of redefining it to reduce duplication and ensure consistency.
Suggested change
| import { UserAvatar } from '@stackframe/stack'; | |
| import { fromNow } from '@stackframe/stack-shared/dist/utils/dates'; | |
| import { Card, CardContent, CardHeader, CardTitle, Table, TableBody, TableCell, TableRow, Typography } from '@stackframe/stack-ui'; | |
| import { useAdminApp } from '../use-admin-app'; | |
| import { DonutChartDisplay, LineChartDisplay, LineChartDisplayConfig } from './line-chart'; | |
| const stackAppInternalsSymbol = Symbol.for("StackAuth--DO-NOT-USE-OR-YOU-WILL-BE-FIRED--StackAppInternals"); | |
| import { UserAvatar, stackAppInternalsSymbol } from '@stackframe/stack'; | |
| import { fromNow } from '@stackframe/stack-shared/dist/utils/dates'; | |
| import { Card, CardContent, CardHeader, CardTitle, Table, TableBody, TableCell, TableRow, Typography } from '@stackframe/stack-ui'; | |
| import { useAdminApp } from '../use-admin-app'; | |
| import { DonutChartDisplay, LineChartDisplay, LineChartDisplayConfig } from './line-chart'; |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Added loading indicators to the overview page metrics by wrapping data-fetching components in React Suspense boundaries, improving perceived performance and user experience during metrics recalculation.
Key changes:
- Extracted globe and charts sections into separate components (
GlobeSectionWithData,ChartsSectionWithData) that calluseMetrics()internally - Wrapped both components with
Suspenseboundaries showingMetricsLoadingFallbackduring data fetching - Changed initial page determination from checking
useMetrics()touseUsers({limit: 1})for faster initial load - Re-enabled metrics prefetching in URL prefetcher to populate cache before navigation
- Added metrics cache refresh when user data is updated for better consistency
- Simplified version check logic (but introduced a bug that removes dev branch warning)
Confidence Score: 3/5
- This PR is mostly safe but contains a logic bug in version checking that needs fixing
- The loading indicator implementation is well-designed and follows React best practices with proper Suspense usage. However, the version check logic change in
check-version/route.tscontains a critical bug where the condition!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersionwill never trigger when client > server, effectively removing the dev branch warning. The dashboard changes are solid and improve UX. - apps/backend/src/app/api/latest/check-version/route.ts requires immediate attention due to flawed version check logic
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/backend/src/app/api/latest/check-version/route.ts | 3/5 | Simplified version check logic by combining two conditions, but the new logic may have unintended behavior |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx | 5/5 | Wrapped globe and charts sections with Suspense boundaries for loading state handling |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/page-client.tsx | 5/5 | Switched from using metrics to useUsers for initial page determination, improving load time |
| packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts | 5/5 | Added metrics cache refresh when users data is refreshed for better data consistency |
Sequence Diagram
sequenceDiagram
participant User
participant PageClient
participant MetricsPage
participant Suspense
participant GlobeSection
participant ChartsSection
participant AdminApp
participant MetricsAPI
User->>PageClient: Navigate to overview page
PageClient->>AdminApp: useUsers({limit: 1})
AdminApp-->>PageClient: user count
alt users.length === 0
PageClient->>User: Show SetupPage
else users.length > 0
PageClient->>MetricsPage: Render MetricsPage
MetricsPage->>Suspense: Wrap GlobeSection
Suspense->>GlobeSection: Attempt render
GlobeSection->>AdminApp: useMetrics(includeAnonymous)
alt Metrics cached
AdminApp-->>GlobeSection: Return cached data
GlobeSection-->>User: Display globe
else Metrics not cached
AdminApp->>MetricsAPI: Fetch metrics
Suspense-->>User: Show MetricsLoadingFallback
MetricsAPI-->>AdminApp: Return metrics data
AdminApp-->>GlobeSection: Provide metrics
GlobeSection-->>User: Display globe
end
MetricsPage->>Suspense: Wrap ChartsSection
Suspense->>ChartsSection: Attempt render
ChartsSection->>AdminApp: useMetrics(includeAnonymous)
alt Metrics cached
AdminApp-->>ChartsSection: Return cached data
ChartsSection-->>User: Display charts
else Metrics not cached
AdminApp->>MetricsAPI: Fetch metrics
Suspense-->>User: Show MetricsLoadingFallback
MetricsAPI-->>AdminApp: Return metrics data
AdminApp-->>ChartsSection: Provide metrics
ChartsSection-->>User: Display charts
end
end
Note over AdminApp,MetricsAPI: Metrics cached for 1 hour
Note over AdminApp: Cache refreshed on user data updates
9 files reviewed, 1 comment
Comment on lines
+52
to
54
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | ||
| return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`); | ||
| } |
Contributor
There was a problem hiding this comment.
logic: logic issue with new condition - when clientVersion > serverVersion, the condition !semver.gt(clientVersion, serverVersion) is false, so the entire expression is false and no error is returned. this removes the previous dev branch warning entirely
Suggested change
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | |
| return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`); | |
| } | |
| if (semver.gt(clientVersion, serverVersion)) { | |
| return err(false, `You are running a version of Stack Auth that is newer than the newest known version (v${clientVersion} > v${serverVersion}). This is weird. Are you running on a development branch?`); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/check-version/route.ts
Line: 52:54
Comment:
**logic:** logic issue with new condition - when `clientVersion > serverVersion`, the condition `!semver.gt(clientVersion, serverVersion)` is false, so the entire expression is false and no error is returned. this removes the previous dev branch warning entirely
```suggestion
if (semver.gt(clientVersion, serverVersion)) {
return err(false, `You are running a version of Stack Auth that is newer than the newest known version (v${clientVersion} > v${serverVersion}). This is weird. Are you running on a development branch?`);
}
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/backend/src/app/api/latest/check-version/route.ts(1 hunks)apps/backend/src/app/api/latest/internal/metrics/route.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/charts-section-with-data.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/globe-section-with-data.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-loading.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx(3 hunks)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/page-client.tsx(1 hunks)apps/dashboard/src/lib/prefetch/url-prefetcher.tsx(2 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T04:13:19.308Z
Learnt from: N2D4
Repo: stack-auth/stack-auth PR: 943
File: examples/convex/app/action/page.tsx:23-28
Timestamp: 2025-10-11T04:13:19.308Z
Learning: In the stack-auth codebase, use `runAsynchronouslyWithAlert` from `stackframe/stack-shared/dist/utils/promises` for async button click handlers and form submissions instead of manual try/catch blocks. This utility automatically handles errors and shows alerts to users.
Applied to files:
apps/dashboard/src/lib/prefetch/url-prefetcher.tsx
🧬 Code graph analysis (6)
apps/backend/src/app/api/latest/check-version/route.ts (1)
packages/template/src/lib/stack-app/apps/implementations/common.ts (1)
clientVersion(17-17)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/globe-section-with-data.tsx (3)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
stackAppInternalsSymbol(444-453)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(29-44)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/globe.tsx (1)
GlobeSection(31-325)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/charts-section-with-data.tsx (5)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
stackAppInternalsSymbol(444-453)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/line-chart.tsx (3)
LineChartDisplayConfig(6-10)LineChartDisplay(17-69)DonutChartDisplay(147-183)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(29-44)apps/dashboard/src/components/router.tsx (1)
useRouter(15-33)packages/stack-shared/src/utils/dates.tsx (1)
fromNow(28-30)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-loading.tsx (1)
packages/stack-ui/src/components/ui/card.tsx (1)
CardContent(87-87)
apps/dashboard/src/lib/prefetch/url-prefetcher.tsx (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
useAdminApp(29-44)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
stackAppInternalsSymbol(444-453)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-loading.tsx (1)
MetricsLoadingFallback(6-18)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/globe-section-with-data.tsx (1)
GlobeSectionWithData(9-18)apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/charts-section-with-data.tsx (1)
ChartsSectionWithData(34-114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Vercel Agent Review
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
🔇 Additional comments (2)
apps/backend/src/app/api/latest/internal/metrics/route.tsx (1)
2-2: LGTM! Import reorganization improves maintainability.The import consolidation and addition of the
Prismanamespace are correct. All imports are properly used throughout the file:
Prismanamespace is required for SQL template tags (line 47)usersCrudHandlersis used for the admin list call (line 287)Also applies to: 5-5, 10-10
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
436-442: Nice optimization for preloading metrics with user data!The concurrent refresh of users and both metrics variants (with and without anonymous users) aligns well with the PR's goal of improving the overview page loading experience. Using
Promise.allensures these independent operations run in parallel for better performance.
Comment on lines
+52
to
54
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | ||
| return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`); | ||
| } |
Contributor
There was a problem hiding this comment.
Client version > server is no longer flagged
The new condition (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) only fires when the client is older (because the < case already returned earlier). If the client runs a newer build (patch ahead), we now fall through and report upToDate: true, which regresses the previous behaviour and hides a real mismatch. Please restore an explicit semver.gt guard for the "client newer than server" scenario.
- if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) {
+ if (semver.gt(clientVersion, serverVersion)) {
+ return err(true, `You are running a version of Stack Auth that is not the same as the newest known version (v${clientVersion} !== v${serverVersion}). Please update to the latest version as soon as possible to ensure that you get the latest feature and security updates.`);
+ }
+ if (clientVersion !== serverVersion) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/backend/src/app/api/latest/check-version/route.ts around lines 52 to 54,
the conditional that checks versions was changed so the "client newer than
server" case is no longer explicitly handled; reinstate an explicit
semver.gt(clientVersion, serverVersion) branch to detect when the client is
newer than the server and return the appropriate err response (same shape as
before) instead of falling through to upToDate:true, keeping the existing
messages and status format for that branch.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| return err(false, `You are running a version of Stack Auth that is newer than the newest known version (v${clientVersion} > v${serverVersion}). This is weird. Are you running on a development branch?`); | ||
| } | ||
| if (clientVersion !== serverVersion) { | ||
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { |
There was a problem hiding this comment.
Suggested change
| if (!semver.gt(clientVersion, serverVersion) && clientVersion !== serverVersion) { | |
| if (semver.gt(clientVersion, serverVersion)) { | |
| return err(false, `You are running a version of Stack Auth that is newer than the newest known version (v${clientVersion} > v${serverVersion}). This is weird. Are you running on a development branch?`); | |
| } | |
| if (clientVersion !== serverVersion) { |
The version check logic was refactored in a way that removes the handler for when the client version is newer than the server version. This will cause the API to incorrectly return "upToDate: true" when a newer client connects.
View Details
Analysis
Version check API incorrectly returns upToDate=true for newer client versions
What fails: /api/latest/check-version route in apps/backend/src/app/api/latest/check-version/route.ts line 52 combines semver.gt() and !== checks incorrectly, causing newer client versions to fall through to upToDate=true
How to reproduce:
# Send POST request with client version newer than server (same major.minor)
curl -X POST /api/latest/check-version \
-H "Content-Type: application/json" \
-d '{"clientVersion": "2.8.50"}'
# When server version is 2.8.49Result: Returns {"upToDate": true} instead of handling the newer client case
Expected: Should return error with message about client being newer than server, per original git history which had separate semver.gt(clientVersion, serverVersion) check
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements