refactor(web): organize devtools components#57
Conversation
… for devtool loaders
There was a problem hiding this comment.
Pull request overview
This PR refactors the devtools component organization by moving devtools-related code from scattered locations into a structured /devtools directory with separate subdirectories for TanStack and React Scan tooling. The functionality remains unchanged, but the code is better organized for maintainability.
Changes:
- Reorganized devtools components into dedicated directories (
/devtools/tanstack/and/devtools/react-scan/) - Separated loader logic from implementation for both TanStack and React Scan devtools
- Updated import paths in
query-client.tsxandlayout.tsxto reference the new component locations
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/context/query-client.tsx | Updated imports to use new TanStackDevtoolsLoader from organized devtools directory |
| web/app/layout.tsx | Updated imports to use new ReactScanLoader from organized devtools directory |
| 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 | New wrapper component for TanStack devtools with React Query and Form plugins |
| web/app/components/devtools/react-scan/scan.tsx | New component that initializes React Scan with dev-mode configuration |
| web/app/components/devtools/react-scan/loader.tsx | New loader component for React Scan with lazy loading and error handling |
| const TanStackDevtoolsWrapper = lazy(() => | ||
| import('./devtools').then(module => ({ | ||
| default: module.TanStackDevtoolsWrapper, | ||
| })).catch(() => { |
There was a problem hiding this comment.
Error handling is inconsistent between the two loader components. The React Scan loader logs errors with console.error, but the TanStack loader silently catches and ignores them. For consistency and better debugging, both loaders should log errors when the devtools fail to load. Consider adding console.error logging here similar to the React Scan loader.
| })).catch(() => { | |
| })).catch(error => { | |
| console.error('Failed to load TanStack devtools:', error) |
| 'use client' | ||
|
|
||
| import { lazy, Suspense } from 'react' | ||
| import { IS_DEV } from '@/config' | ||
|
|
||
| const TanStackDevtoolsWrapper = lazy(() => | ||
| import('./devtools').then(module => ({ | ||
| default: module.TanStackDevtoolsWrapper, | ||
| })).catch(() => { | ||
| return { default: () => null } | ||
| }), | ||
| ) | ||
|
|
||
| export const TanStackDevtoolsLoader = () => { | ||
| if (!IS_DEV) | ||
| return null | ||
|
|
||
| return ( | ||
| <Suspense fallback={null}> | ||
| <TanStackDevtoolsWrapper /> | ||
| </Suspense> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The TanStackDevtoolsLoader component lacks test coverage. The component has testable behavior including IS_DEV conditional rendering, lazy loading with Suspense, and error handling in the catch block. Tests should verify that the component returns null when IS_DEV is false, properly renders the Suspense wrapper when IS_DEV is true, and handles loading errors gracefully.
| 'use client' | ||
|
|
||
| import { lazy, Suspense } from 'react' | ||
| import { IS_DEV } from '@/config' | ||
|
|
||
| const ReactScan = lazy(() => | ||
| import('./scan').then(module => ({ | ||
| default: module.ReactScan, | ||
| })).catch((error) => { | ||
| console.error('Failed to load React Scan devtools:', error) | ||
| return { default: () => null } | ||
| }), | ||
| ) | ||
|
|
||
| export const ReactScanLoader = () => { | ||
| if (!IS_DEV) | ||
| return null | ||
|
|
||
| return ( | ||
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> | ||
| <ReactScan /> | ||
| </Suspense> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The ReactScanLoader component lacks test coverage. The component has testable behavior including IS_DEV conditional rendering, lazy loading with Suspense, error handling with console.error logging, and a visible fallback UI. Tests should verify that the component returns null when IS_DEV is false, properly renders the Suspense wrapper with fallback when IS_DEV is true, and handles loading errors with appropriate error logging.
| return null | ||
|
|
||
| return ( | ||
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> |
There was a problem hiding this comment.
The Suspense fallback for the React Scan loader displays a visible loading message, while the TanStack devtools loader uses fallback={null}. This creates an inconsistent user experience. Both devtools loaders should use the same fallback approach. Since these are development tools that should load quickly and unobtrusively, using fallback={null} would be more appropriate.
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> | |
| <Suspense fallback={null}> |
Benchmark PR from qodo-benchmark#449