Remove Config Errors from TabBar, move to Settings / WaveConfig#3060
Remove Config Errors from TabBar, move to Settings / WaveConfig#3060
Conversation
WalkthroughThe changes refactor configuration error handling across multiple components. The TabBar component removes ConfigErrorIcon and ConfigErrorMessage exports along with related error logic. A new configErrorFilesAtom is added to WaveConfigViewModel to track files with configuration errors. The WaveConfigView component introduces error icons next to config file names and displays a persistent error panel. The Widgets component reads a hasConfigErrors flag from its environment and propagates it to UI elements including the settings button and floating window menu. A new preview file adds interactive controls for testing configuration error and development mode states. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Deploying waveterm with
|
| Latest commit: |
37f3443
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f19827cd.waveterm.pages.dev |
| Branch Preview URL: | https://sawka-update-config-error.waveterm.pages.dev |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/app/view/waveconfig/waveconfig.tsx (1)
301-310: Consider using a stable key for error items.Using array index as a React key is acceptable here since the error list is derived from atom state and re-renders fully on changes. However, if error order could change without the array length changing, consider using
cerr.fileor a composite key.💡 Optional: Use file path as key
- {configErrors.map((cerr, i) => ( - <div key={i} className="text-sm"> + {configErrors.map((cerr) => ( + <div key={cerr.file} className="text-sm">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/view/waveconfig/waveconfig.tsx` around lines 301 - 310, The current list rendering in waveconfig.tsx uses the array index as the React key for configErrors which can cause unstable identity when order changes; update the key in the map inside the JSX that renders configErrors (the map using configErrors.map((cerr, i) => ...)) to use a stable identifier such as cerr.file or a composite like `${cerr.file}-${cerr.err}` instead of i so React can track items reliably during reorders or updates.frontend/preview/previews/widgets.preview.tsx (1)
121-127: Stale environment reference may not reflect atom updates.The
envRef.currentis only set once whennull, so changes tohasConfigErrorsAtomwill be reflected since it's passed by reference. However, changes toisDevprop won't update the environment. ThemockVersionkey on the parent container forces a full re-mount, which works around this, but it's worth noting this coupling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/preview/previews/widgets.preview.tsx` around lines 121 - 127, envRef.current is initialized once and won't pick up changes to props like isDev (even though hasConfigErrorsAtom updates by reference); update the environment whenever relevant inputs change by recreating it via useMemo or in a useEffect that calls makeWidgetsEnv and assigns envRef.current when dependencies change (include baseEnv, isDev, hasCustomAIPresets, apps, and hasConfigErrorsAtom as deps). Locate the envRef, useWaveEnv, and makeWidgetsEnv usage in widgets.preview.tsx and ensure the ref is updated on those dependency changes so the environment reflects new prop values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/view/waveconfig/waveconfig.tsx`:
- Around line 301-310: The current list rendering in waveconfig.tsx uses the
array index as the React key for configErrors which can cause unstable identity
when order changes; update the key in the map inside the JSX that renders
configErrors (the map using configErrors.map((cerr, i) => ...)) to use a stable
identifier such as cerr.file or a composite like `${cerr.file}-${cerr.err}`
instead of i so React can track items reliably during reorders or updates.
In `@frontend/preview/previews/widgets.preview.tsx`:
- Around line 121-127: envRef.current is initialized once and won't pick up
changes to props like isDev (even though hasConfigErrorsAtom updates by
reference); update the environment whenever relevant inputs change by recreating
it via useMemo or in a useEffect that calls makeWidgetsEnv and assigns
envRef.current when dependencies change (include baseEnv, isDev,
hasCustomAIPresets, apps, and hasConfigErrorsAtom as deps). Locate the envRef,
useWaveEnv, and makeWidgetsEnv usage in widgets.preview.tsx and ensure the ref
is updated on those dependency changes so the environment reflects new prop
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 469bb236-0a15-4103-a06b-6c7723dbe1f4
📒 Files selected for processing (5)
frontend/app/tab/tabbar.tsxfrontend/app/view/waveconfig/waveconfig-model.tsfrontend/app/view/waveconfig/waveconfig.tsxfrontend/app/workspace/widgets.tsxfrontend/preview/previews/widgets.preview.tsx
No description provided.