Skip to content

Commit 64cc3b8

Browse files
waleedlatif1claude
andcommitted
chore(mcp): drop unobserved useCallback/useMemo, simplify state
- mcp.tsx: drop 8 useCallback wrappers (no React.memo'd children, no effect/memo deps observe them) - mcp.tsx: drop filteredServers useMemo (cheap O(n) filter, no memoized consumers) - mcp.tsx: serverToDelete {id, name} → serverToDeleteId; derive name from servers cache - mcp-server-form-modal.tsx: drop 8 useCallback wrappers (same rationale) - mcp-server-form-modal.tsx: drop hasChanges useMemo — deps change every keystroke so memo never caches - mcp-server-form-modal.tsx: hover: → hover-hover: for codebase pointer:fine consistency Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent b8532e6 commit 64cc3b8

2 files changed

Lines changed: 199 additions & 230 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.tsx

Lines changed: 92 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
3+
import { useEffect, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import { toError } from '@sim/utils/errors'
66
import { ChevronDown, ChevronRight } from 'lucide-react'
@@ -361,72 +361,66 @@ export function McpServerFormModal({
361361
}
362362
}, [open, clearTestResult])
363363

364-
const resetEnvVarState = useCallback(() => {
364+
const resetEnvVarState = () => {
365365
setShowEnvVars(false)
366366
setActiveInputField(null)
367367
setActiveHeaderIndex(null)
368-
}, [])
369-
370-
const handleInputChange = useCallback(
371-
(field: InputFieldType, value: string, headerIndex?: number) => {
372-
const input = document.activeElement as HTMLInputElement
373-
const pos = input?.selectionStart || 0
374-
setCursorPosition(pos)
375-
376-
if (testResult) clearTestResult()
377-
if (submitError) setSubmitError(null)
378-
379-
const envVarTrigger = checkEnvVarTrigger(value, pos)
380-
setShowEnvVars(envVarTrigger.show)
381-
setEnvSearchTerm(envVarTrigger.show ? envVarTrigger.searchTerm : '')
382-
383-
if (envVarTrigger.show) {
384-
setActiveInputField(field)
385-
setActiveHeaderIndex(headerIndex ?? null)
386-
} else {
387-
resetEnvVarState()
388-
}
368+
}
389369

390-
if (field === 'url') {
391-
setFormData((prev) => ({ ...prev, url: value }))
392-
} else if (headerIndex !== undefined) {
393-
const headerField = field === 'header-key' ? 'key' : 'value'
394-
setFormData((prev) => ({
395-
...prev,
396-
headers: updateHeadersArray(prev.headers || [], headerIndex, headerField, value),
397-
}))
398-
}
399-
},
400-
[testResult, clearTestResult, submitError, resetEnvVarState]
401-
)
370+
const handleInputChange = (field: InputFieldType, value: string, headerIndex?: number) => {
371+
const input = document.activeElement as HTMLInputElement
372+
const pos = input?.selectionStart || 0
373+
setCursorPosition(pos)
402374

403-
const handleEnvVarSelect = useCallback(
404-
(newValue: string) => {
405-
if (activeInputField === 'url') {
406-
setFormData((prev) => ({ ...prev, url: newValue }))
407-
} else if (activeHeaderIndex !== null) {
408-
const field = activeInputField === 'header-key' ? 'key' : 'value'
409-
const processedValue = field === 'key' ? newValue.replace(/[{}]/g, '') : newValue
410-
setFormData((prev) => ({
411-
...prev,
412-
headers: updateHeadersArray(prev.headers || [], activeHeaderIndex, field, processedValue),
413-
}))
414-
}
375+
if (testResult) clearTestResult()
376+
if (submitError) setSubmitError(null)
377+
378+
const envVarTrigger = checkEnvVarTrigger(value, pos)
379+
setShowEnvVars(envVarTrigger.show)
380+
setEnvSearchTerm(envVarTrigger.show ? envVarTrigger.searchTerm : '')
381+
382+
if (envVarTrigger.show) {
383+
setActiveInputField(field)
384+
setActiveHeaderIndex(headerIndex ?? null)
385+
} else {
415386
resetEnvVarState()
416-
},
417-
[activeInputField, activeHeaderIndex, resetEnvVarState]
418-
)
387+
}
419388

420-
const handleHeaderScroll = useCallback((key: string, sl: number) => {
389+
if (field === 'url') {
390+
setFormData((prev) => ({ ...prev, url: value }))
391+
} else if (headerIndex !== undefined) {
392+
const headerField = field === 'header-key' ? 'key' : 'value'
393+
setFormData((prev) => ({
394+
...prev,
395+
headers: updateHeadersArray(prev.headers || [], headerIndex, headerField, value),
396+
}))
397+
}
398+
}
399+
400+
const handleEnvVarSelect = (newValue: string) => {
401+
if (activeInputField === 'url') {
402+
setFormData((prev) => ({ ...prev, url: newValue }))
403+
} else if (activeHeaderIndex !== null) {
404+
const field = activeInputField === 'header-key' ? 'key' : 'value'
405+
const processedValue = field === 'key' ? newValue.replace(/[{}]/g, '') : newValue
406+
setFormData((prev) => ({
407+
...prev,
408+
headers: updateHeadersArray(prev.headers || [], activeHeaderIndex, field, processedValue),
409+
}))
410+
}
411+
resetEnvVarState()
412+
}
413+
414+
const handleHeaderScroll = (key: string, sl: number) => {
421415
setHeaderScrollLeft((prev) => ({ ...prev, [key]: sl }))
422-
}, [])
416+
}
423417

424418
const isDomainBlocked =
425419
!!formData.url?.trim() && !isDomainAllowed(formData.url, allowedMcpDomains)
426420
const isFormValid = !!(formData.name.trim() && formData.url?.trim())
427421
const testButtonLabel = getTestButtonLabel(testResult, isTestingConnection)
428422

429-
const hasChanges = useMemo(() => {
423+
const computeHasChanges = (): boolean => {
430424
if (mode === 'add') return true
431425
if (formData.name !== originalData.name) return true
432426
if (formData.url !== originalData.url) return true
@@ -444,49 +438,49 @@ export function McpServerFormModal({
444438
return true
445439
}
446440
return false
447-
}, [mode, formData, originalData, oauthClientSecretTouched])
448-
449-
const parseJsonConfig = useCallback(
450-
(json: string): { name: string; url: string; headers: Record<string, string> } | null => {
451-
try {
452-
const parsed = JSON.parse(json)
453-
454-
if (parsed.mcpServers && typeof parsed.mcpServers === 'object') {
455-
const entries = Object.entries(parsed.mcpServers)
456-
if (entries.length === 0) {
457-
setJsonError('No servers found in mcpServers')
458-
return null
459-
}
460-
if (entries.length > 1) {
461-
setJsonError(
462-
`Only the first server ("${entries[0][0]}") will be imported. Paste each config separately to add others.`
463-
)
464-
}
465-
const [name, config] = entries[0] as [string, Record<string, unknown>]
466-
if (!config.url || typeof config.url !== 'string') {
467-
setJsonError('Server config must include a "url" field')
468-
return null
469-
}
470-
if (entries.length <= 1) setJsonError(null)
471-
return { name, url: config.url, headers: extractStringHeaders(config.headers) }
472-
}
441+
}
442+
const hasChanges = computeHasChanges()
443+
444+
const parseJsonConfig = (
445+
json: string
446+
): { name: string; url: string; headers: Record<string, string> } | null => {
447+
try {
448+
const parsed = JSON.parse(json)
473449

474-
if (parsed.url && typeof parsed.url === 'string') {
475-
setJsonError(null)
476-
return { name: '', url: parsed.url, headers: extractStringHeaders(parsed.headers) }
450+
if (parsed.mcpServers && typeof parsed.mcpServers === 'object') {
451+
const entries = Object.entries(parsed.mcpServers)
452+
if (entries.length === 0) {
453+
setJsonError('No servers found in mcpServers')
454+
return null
455+
}
456+
if (entries.length > 1) {
457+
setJsonError(
458+
`Only the first server ("${entries[0][0]}") will be imported. Paste each config separately to add others.`
459+
)
477460
}
461+
const [name, config] = entries[0] as [string, Record<string, unknown>]
462+
if (!config.url || typeof config.url !== 'string') {
463+
setJsonError('Server config must include a "url" field')
464+
return null
465+
}
466+
if (entries.length <= 1) setJsonError(null)
467+
return { name, url: config.url, headers: extractStringHeaders(config.headers) }
468+
}
478469

479-
setJsonError('JSON must contain "mcpServers" or a "url" field')
480-
return null
481-
} catch {
482-
setJsonError('Invalid JSON')
483-
return null
470+
if (parsed.url && typeof parsed.url === 'string') {
471+
setJsonError(null)
472+
return { name: '', url: parsed.url, headers: extractStringHeaders(parsed.headers) }
484473
}
485-
},
486-
[]
487-
)
488474

489-
const handleTestConnection = useCallback(async () => {
475+
setJsonError('JSON must contain "mcpServers" or a "url" field')
476+
return null
477+
} catch {
478+
setJsonError('Invalid JSON')
479+
return null
480+
}
481+
}
482+
483+
const handleTestConnection = async () => {
490484
if (!isFormValid) return
491485

492486
await testConnection({
@@ -497,9 +491,9 @@ export function McpServerFormModal({
497491
timeout: formData.timeout,
498492
workspaceId,
499493
})
500-
}, [formData, isFormValid, testConnection, workspaceId])
494+
}
501495

502-
const handleSubmitForm = useCallback(async () => {
496+
const handleSubmitForm = async () => {
503497
if (!isFormValid || isDomainBlocked) return
504498

505499
setIsSubmitting(true)
@@ -562,20 +556,9 @@ export function McpServerFormModal({
562556
} finally {
563557
setIsSubmitting(false)
564558
}
565-
}, [
566-
formData,
567-
isFormValid,
568-
isDomainBlocked,
569-
testConnection,
570-
workspaceId,
571-
onSubmit,
572-
onOpenChange,
573-
mode,
574-
oauthClientSecretTouched,
575-
originalData,
576-
])
577-
578-
const handleSubmitJson = useCallback(async () => {
559+
}
560+
561+
const handleSubmitJson = async () => {
579562
const config = parseJsonConfig(jsonInput)
580563
if (!config) return
581564

@@ -623,15 +606,7 @@ export function McpServerFormModal({
623606
} finally {
624607
setIsSubmitting(false)
625608
}
626-
}, [
627-
jsonInput,
628-
parseJsonConfig,
629-
allowedMcpDomains,
630-
testConnection,
631-
workspaceId,
632-
onSubmit,
633-
onOpenChange,
634-
])
609+
}
635610

636611
const isSubmitDisabled =
637612
isSubmitting || !isFormValid || isDomainBlocked || (mode === 'edit' && !hasChanges)
@@ -727,7 +702,7 @@ export function McpServerFormModal({
727702
<button
728703
type='button'
729704
onClick={() => setShowAdvanced((v) => !v)}
730-
className='mt-1 flex items-center gap-1 self-start text-[var(--text-secondary)] text-small hover:text-[var(--text-primary)]'
705+
className='mt-1 flex items-center gap-1 self-start text-[var(--text-secondary)] text-small hover-hover:text-[var(--text-primary)]'
731706
>
732707
{showAdvanced ? (
733708
<ChevronDown className='h-[14px] w-[14px]' />

0 commit comments

Comments
 (0)