refactor(web): organize devtools components#8
Conversation
… for devtool loaders
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| })).catch(() => { | ||
| return { default: () => null } | ||
| }), |
There was a problem hiding this comment.
🟡 The TanStack loader .catch() handler on line 9 silently swallows import errors, while the ReactScan loader in the same PR correctly logs them with console.error. For consistency and debuggability, the TanStack catch handler should log the error: .catch((error) => { console.error("Failed to load TanStack devtools:", error); return { default: () => null } }).
Extended reasoning...
What the bug is
The TanStack devtools loader (tanstack/loader.tsx) has an empty .catch() handler that silently discards any errors during dynamic import:
.catch(() => {
return { default: () => null }
})Meanwhile, the ReactScan loader (react-scan/loader.tsx), introduced in the same PR as part of the same standardized pattern, properly logs errors:
.catch((error) => {
console.error("Failed to load React Scan devtools:", error)
return { default: () => null }
})How it manifests
Walk through a concrete scenario: suppose @tanstack/react-devtools is missing from node_modules (e.g., after a failed install or version mismatch). The import('./devtools') call will reject. The .catch() handler catches the rejection and returns a no-op component. The developer sees no TanStack devtools panel, but also gets zero console output explaining why. They would need to manually investigate whether the package is installed, whether the import path is correct, etc. In contrast, if ReactScan fails to load, the developer immediately sees "Failed to load React Scan devtools:" with the full error in the console.
Why existing code doesn't prevent it
Both loaders are brand-new files in this PR. There is no existing error boundary or logging middleware that would catch and report this error. The .catch() is the only place where the import failure is handled, and in the TanStack case it simply discards it.
Impact
This is dev-only tooling, so the impact is limited to developer experience. A missing console.error during development means slower debugging when TanStack devtools fail to load. It does not affect production behavior since both loaders are gated behind IS_DEV.
How to fix
Add error logging to the TanStack loader catch handler to match the ReactScan pattern:
.catch((error) => {
console.error("Failed to load TanStack devtools:", error)
return { default: () => null }
})This is a one-line change that brings consistency to the standardized loader pattern being established in this PR.
| return null | ||
|
|
||
| return ( | ||
| <Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}> |
There was a problem hiding this comment.
🟡 The ReactScanLoader uses a visible Suspense fallback (<div className="text-xs text-gray-500">Loading devtools...</div>) while the TanStackDevtoolsLoader in the same PR uses fallback={null}. Since ReactScan only runs scan() as a side effect and returns null, the visible fallback causes a brief flash of "Loading devtools..." text at the top of the page body in dev mode. Change to fallback={null} to match the TanStack loader pattern.
Extended reasoning...
What the bug is
The ReactScanLoader component at web/app/components/devtools/react-scan/loader.tsx:20 wraps the lazy-loaded ReactScan component in a <Suspense> with a visible fallback: <div className="text-xs text-gray-500">Loading devtools...</div>. Meanwhile, the TanStackDevtoolsLoader at web/app/components/devtools/tanstack/loader.tsx:20 uses fallback={null} for the exact same lazy-loading pattern.
Why the visible fallback is wrong here
Looking at scan.tsx, the ReactScan component only runs scan() inside a useEffect and returns null — it renders nothing to the DOM. A visible loading indicator for a component that itself renders nothing is unnecessary and counterproductive. The TanStack loader correctly recognized this pattern and uses fallback={null}.
Step-by-step proof of the flash
- A developer loads the app in dev mode (
IS_DEVis true). layout.tsx:93renders<ReactScanLoader />as the first child of<body>, before<ThemeProvider>and all other content.- React encounters the lazy
<ReactScan />component inside<Suspense>. - While the dynamic import of
./scanis in flight, React renders the Suspense fallback: a<div>with "Loading devtools..." text. - This text appears at the very top of the page body, unstyled by the theme (since it renders before
ThemeProvider). - Once the import resolves,
ReactScanmounts, runsscan()in auseEffect, and returnsnull— the fallback div is replaced with nothing. - The result is a brief but visible flash of "Loading devtools..." text on every page load in dev mode.
Impact
This only affects developers running in dev mode, so the impact is minor. However, it is a visual glitch that appears on every page load and is inconsistent with the pattern established by the TanStack loader in the same PR.
Fix
Change line 20 of loader.tsx from:
<Suspense fallback={<div className="text-xs text-gray-500">Loading devtools...</div>}>to:
<Suspense fallback={null}>This matches the TanStackDevtoolsLoader pattern and eliminates the flash.
2 participants
Benchmark PR from agentic-review-benchmarks#8