refactor(web): organize devtools components#8
Conversation
… for devtool loaders
There was a problem hiding this comment.
Pull request overview
This PR refactors the devtools components by organizing them into a clearer directory structure with separate folders for TanStack and React Scan devtools, implementing a consistent loader pattern for lazy-loading development tools.
Changes:
- Reorganized devtools components into dedicated subdirectories (
devtools/tanstack/anddevtools/react-scan/) - Introduced loader components that handle lazy loading, error handling, and conditional rendering based on development environment
- Updated imports in query-client.tsx and layout.tsx to reference the new loader components
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/context/query-client.tsx | Updated to import and use the new TanStackDevtoolsLoader component |
| web/app/layout.tsx | Updated to import and use the new ReactScanLoader component |
| web/app/components/devtools/tanstack/loader.tsx | New loader component for TanStack devtools with lazy loading and error handling |
| web/app/components/devtools/tanstack/devtools.tsx | Extracted TanStack devtools wrapper component |
| web/app/components/devtools/react-scan/scan.tsx | Extracted React Scan component with scan initialization logic |
| web/app/components/devtools/react-scan/loader.tsx | New loader component for React Scan with lazy loading and error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const TanStackDevtoolsWrapper = lazy(() => | ||
| import('./devtools').then(module => ({ | ||
| default: module.TanStackDevtoolsWrapper, | ||
| })).catch(() => { |
There was a problem hiding this comment.
The error handling in the catch block silently suppresses errors without logging them. This is inconsistent with the error handling pattern used in the ReactScanLoader component, which logs errors with console.error. Silent failures in development tools can make debugging difficult when the devtools fail to load. Consider adding error logging similar to the ReactScanLoader for consistency and better debugging experience.
| })).catch(() => { | |
| })).catch(error => { | |
| console.error('Failed to load TanStack devtools:', error) |
| return null | ||
|
|
||
| return ( | ||
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> |
There was a problem hiding this comment.
The fallback UI for ReactScanLoader shows a visible loading message while the TanStackDevtoolsLoader uses null. This creates an inconsistent user experience between the two devtools loaders. For development tools that load asynchronously, using null for both fallbacks (like TanStackDevtoolsLoader) would be more appropriate since devtools shouldn't display loading UI to end users. Consider changing this to fallback={null} for consistency.
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> | |
| <Suspense fallback={null}> |
Benchmark PR from agentic-review-benchmarks#8