feat(ui): make kopaiClient a param#64
Conversation
📝 WalkthroughWalkthroughThe ObservabilityPage component is modified to accept an optional KopaiClient prop. A lazy default client initialization is implemented, allowing the component to use a provided client or fall back to a lazily-instantiated default when none is supplied. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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. Comment |
5f6d620 to
ab14c0b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/pages/observability.tsx (1)
742-746: Confirm default KopaiClient lifecycle is safe to cache globally.The module-level singleton may hold sockets, timers, or auth state across unmounts/tests. If the SDK expects cleanup or per-mount isolation, this could leak resources. Please verify the KopaiClient lifecycle contract and consider component-scoped memoization with optional cleanup if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/pages/observability.tsx` around lines 742 - 746, The module-scoped singleton using _defaultClient and getDefaultClient may leak sockets/timers/auth across mounts/tests; verify KopaiClient's lifecycle contract and if it requires per-instance cleanup or isolation, remove the module-level cache and instead instantiate KopaiClient inside the component (e.g., via React.useMemo or useRef) so each mount gets its own client, and add explicit cleanup (call the SDK's close/dispose/logout method) in useEffect cleanup; alternatively, provide explicit init()/dispose() functions that wrap KopaiClient so tests can reset the global instance if truly necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/pages/observability.tsx`:
- Around line 742-746: The module-scoped singleton using _defaultClient and
getDefaultClient may leak sockets/timers/auth across mounts/tests; verify
KopaiClient's lifecycle contract and if it requires per-instance cleanup or
isolation, remove the module-level cache and instead instantiate KopaiClient
inside the component (e.g., via React.useMemo or useRef) so each mount gets its
own client, and add explicit cleanup (call the SDK's close/dispose/logout
method) in useEffect cleanup; alternatively, provide explicit init()/dispose()
functions that wrap KopaiClient so tests can reset the global instance if truly
necessary.
Summary by CodeRabbit