Skip to content

Commit c3c22e4

Browse files
authored
improvement(react): replace unnecessary useEffect patterns with better React primitives (#3675)
* improvement(react): replace unnecessary useEffect patterns with better React primitives * fix(react): revert unsafe render-time side effects to useEffect * fix(react): restore useEffect for modals, scroll, and env sync - Modals (create-workspace, rename-document, edit-knowledge-base): restore useEffect watching `open` prop for form reset on programmatic open, since Radix onOpenChange doesn't fire for parent-driven prop changes - Popover: add useEffect watching `open` for programmatic close reset - Chat scroll: restore useEffect watching `isStreamingResponse` so the 1s suppression timer starts when streaming begins, not before the fetch - Credentials manager: revert render-time pattern to useEffect for initial sync from cached React Query data (useRef captures initial value, making the !== check always false on mount) * fix(react): restore useEffect for help/invite modals, combobox index reset - Help modal: restore useEffect watching `open` for form reset on programmatic open (same Radix onOpenChange pattern as other modals) - Invite modal: restore useEffect watching `open` to clear error on programmatic open - Combobox: restore useEffect to reset highlightedIndex when filtered options shrink (prevents stale index from reappearing when options grow) - Remove no-op handleOpenChange wrappers in rename-document and edit-knowledge-base modals (now pure pass-throughs after useEffect fix) * fix(context-menu): use requestAnimationFrame for ColorGrid focus, remove no-op wrapper in create-workspace-modal - ColorGrid: replaced setTimeout with requestAnimationFrame for initial focus to wait for submenu paint completion - create-workspace-modal: removed handleOpenChange pass-through wrapper, use onOpenChange directly * fix(files): restore filesRef pattern to prevent preview mode reset on refetch The useEffect that sets previewMode should only run when selectedFileId changes, not when files array reference changes from React Query refetch. Restores the filesRef pattern to read latest files without triggering the effect — prevents overriding user's manual mode selection. * fix(add-documents-modal, combobox): restore useEffect for modal reset, fix combobox dep array - add-documents-modal: handleOpenChange(true) is dead code in Radix controlled mode — restored useEffect watching open for reset-on-open - combobox: depend on filteredOptions array (not .length) so highlight resets when items change even with same count
1 parent ce3d2d5 commit c3c22e4

File tree

16 files changed

+231
-219
lines changed

16 files changed

+231
-219
lines changed

apps/sim/app/(auth)/login/login-form.tsx

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

3-
import { useEffect, useState } from 'react'
3+
import { useEffect, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import { Eye, EyeOff } from 'lucide-react'
66
import Link from 'next/link'
@@ -99,15 +99,21 @@ export default function LoginPage({
9999
const router = useRouter()
100100
const searchParams = useSearchParams()
101101
const [isLoading, setIsLoading] = useState(false)
102-
const [_mounted, setMounted] = useState(false)
103102
const [showPassword, setShowPassword] = useState(false)
104103
const [password, setPassword] = useState('')
105104
const [passwordErrors, setPasswordErrors] = useState<string[]>([])
106105
const [showValidationError, setShowValidationError] = useState(false)
107106
const buttonClass = useBrandedButtonClass()
108107

109-
const [callbackUrl, setCallbackUrl] = useState('/workspace')
110-
const [isInviteFlow, setIsInviteFlow] = useState(false)
108+
const callbackUrlParam = searchParams?.get('callbackUrl')
109+
const invalidCallbackRef = useRef(false)
110+
if (callbackUrlParam && !validateCallbackUrl(callbackUrlParam) && !invalidCallbackRef.current) {
111+
invalidCallbackRef.current = true
112+
logger.warn('Invalid callback URL detected and blocked:', { url: callbackUrlParam })
113+
}
114+
const callbackUrl =
115+
callbackUrlParam && validateCallbackUrl(callbackUrlParam) ? callbackUrlParam : '/workspace'
116+
const isInviteFlow = searchParams?.get('invite_flow') === 'true'
111117

112118
const [forgotPasswordOpen, setForgotPasswordOpen] = useState(false)
113119
const [forgotPasswordEmail, setForgotPasswordEmail] = useState('')
@@ -120,30 +126,11 @@ export default function LoginPage({
120126
const [email, setEmail] = useState('')
121127
const [emailErrors, setEmailErrors] = useState<string[]>([])
122128
const [showEmailValidationError, setShowEmailValidationError] = useState(false)
123-
const [resetSuccessMessage, setResetSuccessMessage] = useState<string | null>(null)
124-
125-
useEffect(() => {
126-
setMounted(true)
127-
128-
if (searchParams) {
129-
const callback = searchParams.get('callbackUrl')
130-
if (callback) {
131-
if (validateCallbackUrl(callback)) {
132-
setCallbackUrl(callback)
133-
} else {
134-
logger.warn('Invalid callback URL detected and blocked:', { url: callback })
135-
}
136-
}
137-
138-
const inviteFlow = searchParams.get('invite_flow') === 'true'
139-
setIsInviteFlow(inviteFlow)
140-
141-
const resetSuccess = searchParams.get('resetSuccess') === 'true'
142-
if (resetSuccess) {
143-
setResetSuccessMessage('Password reset successful. Please sign in with your new password.')
144-
}
145-
}
146-
}, [searchParams])
129+
const [resetSuccessMessage, setResetSuccessMessage] = useState<string | null>(() =>
130+
searchParams?.get('resetSuccess') === 'true'
131+
? 'Password reset successful. Please sign in with your new password.'
132+
: null
133+
)
147134

148135
useEffect(() => {
149136
const handleKeyDown = (event: KeyboardEvent) => {

apps/sim/app/(auth)/reset-password/reset-password-content.tsx

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

3-
import { Suspense, useEffect, useState } from 'react'
3+
import { Suspense, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import Link from 'next/link'
66
import { useRouter, useSearchParams } from 'next/navigation'
@@ -22,14 +22,9 @@ function ResetPasswordContent() {
2222
text: '',
2323
})
2424

25-
useEffect(() => {
26-
if (!token) {
27-
setStatusMessage({
28-
type: 'error',
29-
text: 'Invalid or missing reset token. Please request a new password reset link.',
30-
})
31-
}
32-
}, [token])
25+
const tokenError = !token
26+
? 'Invalid or missing reset token. Please request a new password reset link.'
27+
: null
3328

3429
const handleResetPassword = async (password: string) => {
3530
try {
@@ -87,8 +82,8 @@ function ResetPasswordContent() {
8782
token={token}
8883
onSubmit={handleResetPassword}
8984
isSubmitting={isSubmitting}
90-
statusType={statusMessage.type}
91-
statusMessage={statusMessage.text}
85+
statusType={tokenError ? 'error' : statusMessage.type}
86+
statusMessage={tokenError ?? statusMessage.text}
9287
/>
9388
</div>
9489

apps/sim/app/(auth)/signup/signup-form.tsx

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

3-
import { Suspense, useEffect, useState } from 'react'
3+
import { Suspense, useMemo, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import { Eye, EyeOff } from 'lucide-react'
66
import Link from 'next/link'
@@ -82,49 +82,32 @@ function SignupFormContent({
8282
const searchParams = useSearchParams()
8383
const { refetch: refetchSession } = useSession()
8484
const [isLoading, setIsLoading] = useState(false)
85-
const [, setMounted] = useState(false)
8685
const [showPassword, setShowPassword] = useState(false)
8786
const [password, setPassword] = useState('')
8887
const [passwordErrors, setPasswordErrors] = useState<string[]>([])
8988
const [showValidationError, setShowValidationError] = useState(false)
90-
const [email, setEmail] = useState('')
89+
const [email, setEmail] = useState(() => searchParams.get('email') ?? '')
9190
const [emailError, setEmailError] = useState('')
9291
const [emailErrors, setEmailErrors] = useState<string[]>([])
9392
const [showEmailValidationError, setShowEmailValidationError] = useState(false)
94-
const [redirectUrl, setRedirectUrl] = useState('')
95-
const [isInviteFlow, setIsInviteFlow] = useState(false)
9693
const buttonClass = useBrandedButtonClass()
9794

95+
const redirectUrl = useMemo(
96+
() => searchParams.get('redirect') || searchParams.get('callbackUrl') || '',
97+
[searchParams]
98+
)
99+
const isInviteFlow = useMemo(
100+
() =>
101+
searchParams.get('invite_flow') === 'true' ||
102+
redirectUrl.startsWith('/invite/') ||
103+
redirectUrl.startsWith('/credential-account/'),
104+
[searchParams, redirectUrl]
105+
)
106+
98107
const [name, setName] = useState('')
99108
const [nameErrors, setNameErrors] = useState<string[]>([])
100109
const [showNameValidationError, setShowNameValidationError] = useState(false)
101110

102-
useEffect(() => {
103-
setMounted(true)
104-
const emailParam = searchParams.get('email')
105-
if (emailParam) {
106-
setEmail(emailParam)
107-
}
108-
109-
// Check both 'redirect' and 'callbackUrl' params (login page uses callbackUrl)
110-
const redirectParam = searchParams.get('redirect') || searchParams.get('callbackUrl')
111-
if (redirectParam) {
112-
setRedirectUrl(redirectParam)
113-
114-
if (
115-
redirectParam.startsWith('/invite/') ||
116-
redirectParam.startsWith('/credential-account/')
117-
) {
118-
setIsInviteFlow(true)
119-
}
120-
}
121-
122-
const inviteFlowParam = searchParams.get('invite_flow')
123-
if (inviteFlowParam === 'true') {
124-
setIsInviteFlow(true)
125-
}
126-
}, [searchParams])
127-
128111
const validatePassword = (passwordValue: string): string[] => {
129112
const errors: string[] = []
130113

apps/sim/app/chat/components/input/input.tsx

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,6 @@ export const ChatInput: React.FC<{
7171
}
7272
}
7373

74-
// Adjust height on input change
75-
useEffect(() => {
76-
adjustTextareaHeight()
77-
}, [inputValue])
78-
7974
// Close the input when clicking outside (only when empty)
8075
useEffect(() => {
8176
const handleClickOutside = (event: MouseEvent) => {
@@ -94,17 +89,14 @@ export const ChatInput: React.FC<{
9489
return () => document.removeEventListener('mousedown', handleClickOutside)
9590
}, [inputValue])
9691

97-
// Handle focus and initial height when activated
98-
useEffect(() => {
99-
if (isActive && textareaRef.current) {
100-
textareaRef.current.focus()
101-
adjustTextareaHeight() // Adjust height when becoming active
102-
}
103-
}, [isActive])
104-
10592
const handleActivate = () => {
10693
setIsActive(true)
107-
// Focus is now handled by the useEffect above
94+
requestAnimationFrame(() => {
95+
if (textareaRef.current) {
96+
textareaRef.current.focus()
97+
adjustTextareaHeight()
98+
}
99+
})
108100
}
109101

110102
// Handle file selection
@@ -186,6 +178,7 @@ export const ChatInput: React.FC<{
186178

187179
const handleInputChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
188180
setInputValue(e.target.value)
181+
adjustTextareaHeight()
189182
}
190183

191184
// Handle voice start with smooth transition to voice-first mode

apps/sim/app/chat/components/voice-interface/voice-interface.tsx

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,10 @@ export function VoiceInterface({
7878
const currentStateRef = useRef<'idle' | 'listening' | 'agent_speaking'>('idle')
7979
const isCallEndedRef = useRef(false)
8080

81-
useEffect(() => {
82-
currentStateRef.current = state
83-
}, [state])
81+
const updateState = useCallback((next: 'idle' | 'listening' | 'agent_speaking') => {
82+
setState(next)
83+
currentStateRef.current = next
84+
}, [])
8485

8586
const recognitionRef = useRef<SpeechRecognition | null>(null)
8687
const mediaStreamRef = useRef<MediaStream | null>(null)
@@ -97,9 +98,10 @@ export function VoiceInterface({
9798
(window as WindowWithSpeech).webkitSpeechRecognition
9899
)
99100

100-
useEffect(() => {
101-
isMutedRef.current = isMuted
102-
}, [isMuted])
101+
const updateIsMuted = useCallback((next: boolean) => {
102+
setIsMuted(next)
103+
isMutedRef.current = next
104+
}, [])
103105

104106
const setResponseTimeout = useCallback(() => {
105107
if (responseTimeoutRef.current) {
@@ -108,7 +110,7 @@ export function VoiceInterface({
108110

109111
responseTimeoutRef.current = setTimeout(() => {
110112
if (currentStateRef.current === 'listening') {
111-
setState('idle')
113+
updateState('idle')
112114
}
113115
}, 5000)
114116
}, [])
@@ -123,10 +125,10 @@ export function VoiceInterface({
123125
useEffect(() => {
124126
if (isPlayingAudio && state !== 'agent_speaking') {
125127
clearResponseTimeout()
126-
setState('agent_speaking')
128+
updateState('agent_speaking')
127129
setCurrentTranscript('')
128130

129-
setIsMuted(true)
131+
updateIsMuted(true)
130132
if (mediaStreamRef.current) {
131133
mediaStreamRef.current.getAudioTracks().forEach((track) => {
132134
track.enabled = false
@@ -141,17 +143,17 @@ export function VoiceInterface({
141143
}
142144
}
143145
} else if (!isPlayingAudio && state === 'agent_speaking') {
144-
setState('idle')
146+
updateState('idle')
145147
setCurrentTranscript('')
146148

147-
setIsMuted(false)
149+
updateIsMuted(false)
148150
if (mediaStreamRef.current) {
149151
mediaStreamRef.current.getAudioTracks().forEach((track) => {
150152
track.enabled = true
151153
})
152154
}
153155
}
154-
}, [isPlayingAudio, state, clearResponseTimeout])
156+
}, [isPlayingAudio, state, clearResponseTimeout, updateState, updateIsMuted])
155157

156158
const setupAudio = useCallback(async () => {
157159
try {
@@ -310,7 +312,7 @@ export function VoiceInterface({
310312
return
311313
}
312314

313-
setState('listening')
315+
updateState('listening')
314316
setCurrentTranscript('')
315317

316318
if (recognitionRef.current) {
@@ -320,10 +322,10 @@ export function VoiceInterface({
320322
logger.error('Error starting recognition:', error)
321323
}
322324
}
323-
}, [isInitialized, isMuted, state])
325+
}, [isInitialized, isMuted, state, updateState])
324326

325327
const stopListening = useCallback(() => {
326-
setState('idle')
328+
updateState('idle')
327329
setCurrentTranscript('')
328330

329331
if (recognitionRef.current) {
@@ -333,15 +335,15 @@ export function VoiceInterface({
333335
// Ignore
334336
}
335337
}
336-
}, [])
338+
}, [updateState])
337339

338340
const handleInterrupt = useCallback(() => {
339341
if (state === 'agent_speaking') {
340342
onInterrupt?.()
341-
setState('listening')
343+
updateState('listening')
342344
setCurrentTranscript('')
343345

344-
setIsMuted(false)
346+
updateIsMuted(false)
345347
if (mediaStreamRef.current) {
346348
mediaStreamRef.current.getAudioTracks().forEach((track) => {
347349
track.enabled = true
@@ -356,14 +358,14 @@ export function VoiceInterface({
356358
}
357359
}
358360
}
359-
}, [state, onInterrupt])
361+
}, [state, onInterrupt, updateState, updateIsMuted])
360362

361363
const handleCallEnd = useCallback(() => {
362364
isCallEndedRef.current = true
363365

364-
setState('idle')
366+
updateState('idle')
365367
setCurrentTranscript('')
366-
setIsMuted(false)
368+
updateIsMuted(false)
367369

368370
if (recognitionRef.current) {
369371
try {
@@ -376,7 +378,7 @@ export function VoiceInterface({
376378
clearResponseTimeout()
377379
onInterrupt?.()
378380
onCallEnd?.()
379-
}, [onCallEnd, onInterrupt, clearResponseTimeout])
381+
}, [onCallEnd, onInterrupt, clearResponseTimeout, updateState, updateIsMuted])
380382

381383
useEffect(() => {
382384
const handleKeyDown = (event: KeyboardEvent) => {
@@ -397,7 +399,7 @@ export function VoiceInterface({
397399
}
398400

399401
const newMutedState = !isMuted
400-
setIsMuted(newMutedState)
402+
updateIsMuted(newMutedState)
401403

402404
if (mediaStreamRef.current) {
403405
mediaStreamRef.current.getAudioTracks().forEach((track) => {
@@ -410,7 +412,7 @@ export function VoiceInterface({
410412
} else if (state === 'idle') {
411413
startListening()
412414
}
413-
}, [isMuted, state, handleInterrupt, stopListening, startListening])
415+
}, [isMuted, state, handleInterrupt, stopListening, startListening, updateIsMuted])
414416

415417
useEffect(() => {
416418
if (isSupported) {

0 commit comments

Comments
 (0)