Skip to content

Commit 4c8a52a

Browse files
gtherondclaude
andcommitted
[1954] fix(ui): use useEffect instead of useMemo for onChange side effect
Devin review flagged the useMemoOnChange helper at CloudsYamlInput.tsx for misusing useMemo as a side-effect hook: useMemo is for pure memoization, not for calling a parent state-updating callback during render. The pattern produces the React 18 "Cannot update a component while rendering a different component" warning and risks a render loop when the parent re-renders the child with new props. In concurrent mode, useMemo callbacks may also fire multiple times without committing. Inlined the body into a regular useEffect with an explicit deps array and an eslint-disable comment documenting why onChange is intentionally excluded (parent-supplied callback whose identity may change every render; including it would re-fire the effect on every parent re-render). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cfad5e6 commit 4c8a52a

1 file changed

Lines changed: 9 additions & 14 deletions

File tree

ui/src/features/credentials/components/CloudsYamlInput.tsx

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
TextField,
1010
Typography,
1111
} from '@mui/material'
12-
import { ChangeEvent, useMemo, useRef, useState } from 'react'
12+
import { ChangeEvent, useEffect, useMemo, useRef, useState } from 'react'
1313
import {
1414
parseCloudsYAML,
1515
detectAuthMethod,
@@ -80,13 +80,19 @@ export default function CloudsYamlInput({
8080
effectiveCloud !== '' &&
8181
authMethod !== 'unsupported'
8282

83-
// Emit upstream whenever effective state changes.
84-
useMemoOnChange(() => {
83+
// Emit upstream whenever effective state changes. useEffect (not useMemo)
84+
// because this is a side effect — calling the parent's onChange during
85+
// render violates React rules and risks render-loop warnings in React 18.
86+
useEffect(() => {
8587
onChange({
8688
cloudsYaml: raw,
8789
cloudName: effectiveCloud,
8890
isValid,
8991
})
92+
// onChange intentionally excluded: it's a parent-supplied callback whose
93+
// identity may change every render. Including it would re-fire the effect
94+
// on every parent re-render.
95+
// eslint-disable-next-line react-hooks/exhaustive-deps
9096
}, [raw, effectiveCloud, isValid])
9197

9298
const onTextChange = (e: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => {
@@ -212,14 +218,3 @@ export default function CloudsYamlInput({
212218
)
213219
}
214220

215-
/**
216-
* Tiny effect helper: call `fn` whenever any of `deps` changes. Avoids
217-
* pulling React useEffect's lint dependency check noise into this file by
218-
* keeping the dep list explicit.
219-
*/
220-
function useMemoOnChange(fn: () => void, deps: unknown[]) {
221-
// eslint-disable-next-line react-hooks/exhaustive-deps
222-
useMemo(() => {
223-
fn()
224-
}, deps)
225-
}

0 commit comments

Comments
 (0)