Fix AnalyticsRequest circular import #1189
Conversation
Brendonovich
changed the title
WalkthroughRefactors analytics querying by moving useVideosAnalyticsQuery from Requests to Queries, updating imports across dashboard components. Adds a new Queries/Analytics.ts implementing the hook, and removes the hook from Requests/AnalyticsRequest.ts. No changes to component logic beyond import paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as Component
participant Q as useVideosAnalyticsQuery (Queries/Analytics)
participant ER as useEffectQuery
participant DR as AnalyticsRequest.DataLoaderResolver
participant API as Analytics API
UI->>Q: invoke(videoIds, dubApiKeyEnabled)
alt dubApiKeyEnabled is false
Q-->>UI: return {}
else dubApiKeyEnabled is true
Q->>ER: run query (key: ["analytics", videoIds])
ER->>DR: resolve DataLoader
par For each videoId
DR->>API: fetch count(videoId)
API-->>DR: count or error
DR-->>ER: { videoId, count|0 }
end
ER-->>Q: aggregate { videoId: count }
Q-->>UI: result map
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/lib/Queries/Analytics.ts (1)
6-47: Consider bounding the concurrency for analytics requests.The implementation looks correct overall. However, line 33 uses
{ concurrency: "unbounded" }which could cause performance issues or rate limiting problems if there are many videos to fetch analytics for.Consider applying a reasonable concurrency limit:
const results = yield* Effect.all( videoIds.map((videoId) => Effect.request( new AnalyticsRequest.AnalyticsRequest({ videoId }), dataloader, ).pipe( Effect.catchAll((e) => { console.warn( `Failed to fetch analytics for video ${videoId}:`, e, ); return Effect.succeed({ count: 0 }); }), Effect.map(({ count }) => ({ videoId, count })), ), ), - { concurrency: "unbounded" }, + { concurrency: 10 }, );apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
12-14: Remove unused AnalyticsRequest import
TheAnalyticsRequestimport on line 14 isn’t used in this file; remove it.apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (1)
11-12: Remove unusedAnalyticsRequestimport
The importedAnalyticsRequestis not used in this file; remove it to keep imports clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(org)/dashboard/caps/Caps.tsx(1 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx(1 hunks)apps/web/lib/Queries/Analytics.ts(1 hunks)apps/web/lib/Requests/AnalyticsRequest.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/Requests/AnalyticsRequest.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx
🧬 Code graph analysis (1)
apps/web/lib/Queries/Analytics.ts (2)
apps/web/lib/EffectRuntime.ts (1)
useEffectQuery(22-22)apps/web/lib/Requests/AnalyticsRequest.ts (1)
AnalyticsRequest(8-12)
⏰ 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). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
| import { useVideosAnalyticsQuery } from "@/lib/Queries/Analytics"; | ||
| import { AnalyticsRequest } from "@/lib/Requests/AnalyticsRequest"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the necessity of the AnalyticsRequest import.
Similar to other files in this PR, the AnalyticsRequest import on line 11 appears unused. Consider removing it unless it's required for type definitions.
Run the following script to check usage:
🏁 Script executed:
#!/bin/bash
# Check if AnalyticsRequest is used beyond the import statement
rg -n 'AnalyticsRequest' apps/web/app/\(org\)/dashboard/folder/\[id\]/components/FolderVideosSection.tsxLength of output: 70
Remove unused import of AnalyticsRequest. The import on line 11 isn’t referenced elsewhere in this file—delete it.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx
around lines 10 to 11, the import AnalyticsRequest is unused; remove the
AnalyticsRequest import (delete the ", AnalyticsRequest" from the import
statement or remove the entire import line if it becomes empty) and then run the
TypeScript/ESLint checks to ensure no other unused imports remain.
1 participant
Summary by CodeRabbit