8000 fix: resolve provider switching issues and React Hook dependency warnings (fixes #3813, #3874) by indiesewell · Pull Request #3949 · RooCodeInc/Roo-Code · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: resolve provider switching issues and React Hook dependency warnings (fixes #3813, #3874) #3949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

indiesewell
Copy link
@indiesewell indiesewell commented May 25, 2025

Closes #3813
Closes #3874

🎯 Overview

This PR fixes critical provider switching issues and resolves React Hook dependency warnings that were causing model display problems when switching between API providers.

🐛 Issues Fixed

⚡ Provider Switching Problems

  • Issue: Model values not updating correctly when switching between providers
  • Issue: Gemini and static providers showing incorrect model selections
  • Issue: Provider switching causing UI inconsistencies

🔧 React Hook Warnings

  • Issue: useSelectedModel hook missing proper dependency management
  • Issue: React Hook dependency warnings in console
  • Issue: Potential memory leaks from improper hook usage

🚀 Solutions Implemented

🎯 Smart Model Management

  • Fixed useSelectedModel Hook: Added proper useMemo with correct dependencies
  • Optimized Provider Logic: Only apply apiModelId updates to static providers
  • Enhanced State Management: Ensure correct model values display when switching providers

📈 Performance Improvements

  • Reduced Re-renders: Proper memoization prevents unnecessary component updates
  • Faster Provider Switching: Optimized switching logic for better responsiveness
  • Memory Optimization: Fixed potential memory leaks from improper hook dependencies

📁 Files Changed

  • webview-ui/src/components/ui/hooks/useSelectedModel.ts - Fixed React Hook with proper useMemo and dependencies

🔍 Technical Details

Before (Problematic Code)

// Missing proper dependency management
// Causing unnecessary re-renders and warnings

Important

Fixes provider switching issues and React Hook dependency warnings in ApiOptions.tsx and useSelectedModel.ts.

  • Provider Switching Fixes:
    • In ApiOptions.tsx, update apiModelId only for static providers when selectedModelId changes.
    • Prevent overwriting user-saved values during provider switching.
  • React Hook Dependency Fixes:
    • In useSelectedModel.ts, use useMemo for emptyRouterModels and getSelectedModel to manage dependencies properly.
    • Fix potential memory leaks and reduce unnecessary re-renders.
  • Performance Improvements:
    • Optimize provider switching logic for better responsiveness.
    • Ensure correct model values display when switching providers.

This description was created by Ellipsis for e106947. You can customize this summary. It will automatically update as commits are pushed.

🔧 Provider Switching Fixes:
- Fix useSelectedModel hook with React useMemo for proper dependency management
- Resolve Gemini and static providers model display issues when switching
- Optimize apiModelId update logic to only apply to static providers
- Ensure correct model values are displayed when switching between API providers

🚀 Performance Improvements:
- Improve provider switching responsiveness
- Fix React Hook dependency warnings in useSelectedModel

Based on upstream v3.18.3 with proper Git merge
setApiConfigurationField("apiModelId", selectedModelId)
if (selectedModelId && selectedProvider) {
// Only update apiModelId for static providers, not router providers
const staticProviders = [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the staticProviders array outside the effect (as a module-level constant) for clarity and to avoid re-creation on each render.

@adamhill
Copy link
Collaborator

Why did elipsis redact the example of problematic code for the rest of us?

@indiesewell indiesewell changed the title fix: resolve provider switching issues and React Hook dependency warnings fix: resolve provider switching issues and React Hook dependency warnings (fixes #3813, #3874) May 27, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label May 27, 2025
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 28, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Preliminary Review] in Roo Code Roadmap May 28, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels May 28, 2025
@daniel-lxs
Copy link
Collaborator

Hey, @indiesewell

Unfortunately I can't seem to find any repro steps to test your solution against.

I'll be closing this temporarily. but feel free to reopen if you gather enough information about this issue.

@daniel-lxs daniel-lxs closed this May 29, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap May 29, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR - Needs Preliminary Review size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Cannot select any model - the drop down after selection blanks out after selecting the model. API cannot select models other than Claude 3.7 Sonnet
4 participants
0