Skip to content

Commit 6cb8b59

Browse files
committed
fix(FR-2815): break ResourceAllocationFormItems render loop with idempotent guards
Resolves #7248(FR-2815) User reported the 'use memo' directive alone did not stop the loop on rc2. Deeper analysis shows the loop is closed by `setFieldsValue` calls that repeatedly write the same value, which still notifies subscribers (including the page-level `Form.useWatch('resource')` added in FR-2324) and triggers a parent re-render, forcing the child to re-render and re-fire the effect. Fixes: 1. Wrap `acceleratorSlotsInRG = _.omitBy(...)` in an explicit `useMemo` with `[resourceSlotsInRG]` deps, so its identity is stable across renders even if the React Compiler does not memoize it. This stabilizes the downstream `supportedAcceleratorTypesInRGByImage` useMemo. 2. Add idempotent guards to the `useEffect` at line 226 so `setFieldsValue` is only called when the target value actually differs: - Skip `allocationPreset: 'auto-select'` write when it is already 'auto-select' or 'custom'. - Skip `resource.accelerator: 0` write when it is already `0`. 3. Add an idempotent early-return to `ensureValidAcceleratorType` when the target accelerator type equals the current one. This helper is called unconditionally from another effect at line 460-515; without the guard it pump-primes the same loop. Kept the `'use memo'` directives from the prior commit on this branch as they align with the project-wide React 19.2 + React Compiler convention (`.claude/rules/react-compiler-memoization.md`).
1 parent 00572ee commit 6cb8b59

2 files changed

Lines changed: 38 additions & 9 deletions

File tree

react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ const ResourceAllocationFormItems: React.FC<
104104
hideClusterFormItems = false,
105105
extraAcceleratorRules,
106106
}) => {
107+
'use memo';
107108
const form = Form.useFormInstance<MergedResourceAllocationFormValue>();
108109
const { t } = useTranslation();
109110
const { token } = theme.useToken();
@@ -182,12 +183,21 @@ const ResourceAllocationFormItems: React.FC<
182183
);
183184

184185
// When undefined, it means that the resourceSlots are not loaded yet.
185-
const acceleratorSlotsInRG = resourceSlotsInRG
186-
? _.omitBy(resourceSlotsInRG, (_value, key) => {
187-
if (['cpu', 'mem', 'shmem'].includes(key)) return true;
188-
return false;
189-
})
190-
: undefined;
186+
// Memoize so the identity is stable when `resourceSlotsInRG` is unchanged —
187+
// otherwise the downstream `supportedAcceleratorTypesInRGByImage` useMemo
188+
// is invalidated every render, which (combined with the page-level
189+
// `Form.useWatch('resource')` added in FR-2324) closes a render-loop
190+
// circuit on backends where `length === 0` is reached.
191+
const acceleratorSlotsInRG = useMemo(
192+
() =>
193+
resourceSlotsInRG
194+
? _.omitBy(resourceSlotsInRG, (_value, key) => {
195+
if (['cpu', 'mem', 'shmem'].includes(key)) return true;
196+
return false;
197+
})
198+
: undefined,
199+
[resourceSlotsInRG],
200+
);
191201

192202
// When undefined, it means that the image is not determined yet.
193203
const currentImageAcceleratorLimits = useMemo(
@@ -223,15 +233,25 @@ const ResourceAllocationFormItems: React.FC<
223233
}, [currentImage, acceleratorSlotsInRG, currentEnvironmentManual]);
224234

225235
useEffect(() => {
236+
// Idempotent guards: only call setFieldsValue when the target value
237+
// actually differs. Without these guards, the page-level
238+
// `Form.useWatch('resource')` added in FR-2324 propagates each
239+
// setFieldsValue back up to a parent re-render, which triggers a
240+
// child re-render, which re-fires this effect — an infinite loop on
241+
// backends where `length === 0` (FR-2815).
226242
if (
227243
!currentResourceValue &&
228-
form.getFieldValue('allocationPreset') !== 'custom'
244+
form.getFieldValue('allocationPreset') !== 'custom' &&
245+
form.getFieldValue('allocationPreset') !== 'auto-select'
229246
) {
230247
form.setFieldsValue({
231248
allocationPreset: 'auto-select',
232249
});
233250
}
234-
if (supportedAcceleratorTypesInRGByImage?.length === 0) {
251+
if (
252+
supportedAcceleratorTypesInRGByImage?.length === 0 &&
253+
form.getFieldValue(['resource', 'accelerator']) !== 0
254+
) {
235255
form.setFieldsValue({
236256
resource: {
237257
accelerator: 0,
@@ -278,9 +298,17 @@ const ResourceAllocationFormItems: React.FC<
278298
? currentAcceleratorType
279299
: _.first(_.keys(acceleratorSlotsInRG));
280300

301+
const targetAcceleratorType = nextAcceleratorType || currentAcceleratorType;
302+
// Idempotent guard: skip setFieldsValue when value is unchanged so this
303+
// helper does not pump-prime a render loop when called from effects
304+
// whose deps re-fire each render (FR-2815).
305+
if (targetAcceleratorType === currentAcceleratorType) {
306+
return;
307+
}
308+
281309
form.setFieldsValue({
282310
resource: {
283-
acceleratorType: nextAcceleratorType || currentAcceleratorType,
311+
acceleratorType: targetAcceleratorType,
284312
},
285313
});
286314
});

react/src/hooks/useResourceLimitAndRemaining.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export const useResourceLimitAndRemaining = ({
116116
ignorePerContainerConfig = false,
117117
fetchKey,
118118
}: Props) => {
119+
'use memo';
119120
const baiClient = useSuspendedBackendaiClient();
120121
const [resourceSlots] = useResourceSlots();
121122
const acceleratorSlots = _.omit(resourceSlots, ['cpu', 'mem', 'shmem']);

0 commit comments

Comments
 (0)