diff --git a/AGENT.md b/AGENT.md new file mode 100644 index 00000000..52e8f412 --- /dev/null +++ b/AGENT.md @@ -0,0 +1,265 @@ +# React State and Request Patterns + +These guidelines define preferred patterns for request handling, state updates, and side-effect management in React applications. + +The primary goal is to keep data flow explicit, predictable, maintainable, and performant while avoiding unnecessary rerenders and effect-driven logic. + +--- + +## 1. Avoid Effect-Driven Requests + +Do not use request functions themselves as dependencies in `useEffect`. + +Avoid patterns like: + +```ts +useEffect(() => { + fetchData(); +}, [fetchData]); +``` + +Requests should be triggered explicitly by user actions or lifecycle entry points. + +--- + +## 2. Form Requests Should Be Action-Driven + +For form-related requests (such as loading `Select` options): + +- Fetch data when the form is opened for the first time. +- If later requests depend on user interactions, trigger them directly inside the interaction handler. +- Do not rely on `useEffect` dependency changes to trigger requests. + +Recommended: + +```ts +const handleOnChange = (value) => { + fetchData(value); +}; +``` + +Avoid: + +```ts +useEffect(() => { + fetchData(value); +}, [value]); +``` + +The action itself should control the request. + +--- + +## 3. Update Related States Together + +If a single action updates multiple related states: + +- Do not synchronize them through `useEffect` +- Do not derive them indirectly through `useMemo` + +Instead, update all related states directly inside the action handler. + +Recommended: + +```ts +const handleOnChange = (value) => { + setState1(...); + setState2(...); + buildState(...); +}; +``` + +Avoid implicit state synchronization chains. + +--- + +## 4. Group Strongly Related State + +If multiple states are always updated together: + +- Do not split them into multiple `useState` calls. +- Prefer a single state object. + +Recommended: + +```ts +const [state, setState] = useState({ + state1: ..., + state2: ..., + state3: ..., +}); +``` + +This reduces unnecessary rerenders and keeps state transitions predictable. + +--- + +## 5. Prefer Explicit State Flow + +Avoid chaining business logic through multiple `useEffect` hooks. + +Keep: + +- request execution +- state updates +- derived calculations + +close to the triggering action whenever possible. + +Prefer: + +```ts +const handleAction = () => { + fetchData(); + setTableData(...); + setSelectedRow(...); +}; +``` + +Over: + +```ts +useEffect(() => { + buildTable(); +}, [data]); + +useEffect(() => { + updateSelection(); +}, [tableData]); +``` + +--- + +## 6. Avoid Premature Memoization + +Do not use `useMemo` or `useCallback` unless there is a confirmed rendering or computation bottleneck. + +Overusing memoization: + +- increases complexity +- makes state flow harder to understand +- may introduce stale dependency issues + +Prefer simple and explicit logic first. + +Optimize only when necessary. + +--- + +## 7. Keep Request Logic Predictable + +A user interaction should clearly show: + +- what request is triggered +- which states are updated +- how the UI changes + +Avoid indirect update chains caused by dependency-driven effects. + +The code should make the request and update flow easy to trace. + +--- + +## 8. Prefer Action-Driven Architecture + +Prefer: + +- action-driven updates +- explicit handlers +- localized state transitions + +Over: + +- effect-driven synchronization +- cross-hook implicit updates +- reactive chains between states + +The triggering action should remain the primary source of truth for UI updates. + +--- + +# Form + +Form-specific patterns that build on the rules above. The theme: keep cascading selections (pick A → derive B → write form) on a single, predictable path. + +## 1. No Fallback for Derived Selection + +When "pick A then auto-pick B", match by rule and return `undefined` if no match — let the corresponding form field stay empty. + +Do not silently fall back to `list[0]` or another default. A fallback hides data issues and tricks the user into thinking they have a valid selection. + +```ts +const findB = (key, list) => + key ? list.find((x) => x.key === key) : undefined; +``` + +For form fields, prefer clearing with `undefined` over `''`. With Ant Design, `undefined` restores the placeholder; `''` is treated as a real value. + +## 2. Async Race Protection + +For fetches triggered by a lifecycle entry (e.g., modal open), tag each invocation with a session ref. Discard stale results if the session has rotated (the modal was closed and re-opened) by the time the response arrives. + +```ts +const sessionRef = useRef(0); + +useEffect(() => { + if (!open) { + sessionRef.current += 1; + return; + } + const session = ++sessionRef.current; + Promise.all([fetchA(), fetchB()]).then(([as, bs]) => { + if (sessionRef.current !== session) return; + applySelection(as.items[0], findB(as.items[0].key, bs.items)); + }); +}, [open]); +``` + +## 3. Reference Template + +A typical form with two cascading selectors backed by a single shared state: + +```ts +type Selection = { a?: string; b?: number }; + +const [selection, setSelection] = useState({}); +const sessionRef = useRef(0); + +const findB = (key, list) => + key ? list.find((x) => x.key === key) : undefined; + +// Single atomic write: state + form together. +const applySelection = (a, b) => { + setSelection({ a: a.name, b: b?.id }); + form.current?.setFieldsValue({ + field: b?.field, + spec: { ...currentSpec, ...b?.spec } + }); +}; + +// Trigger 1: modal opened +useEffect(() => { + if (!open) { + sessionRef.current++; + setSelection({}); + return; + } + const session = ++sessionRef.current; + Promise.all([fetchA(), fetchB()]).then(([as, bs]) => { + if (sessionRef.current !== session) return; + const first = as.items[0]; + applySelection(first, findB(first.key, bs.items)); + }); +}, [open]); + +// Trigger 2: user picks A +const handleAChange = (a) => { + applySelection(a, findB(a.key, listB)); +}; + +// Trigger 3: user picks B +const handleBChange = (b) => { + setSelection((prev) => ({ ...prev, b: b.id })); + form.current?.setFieldsValue({ ...b.fields }); +}; +``` diff --git a/src/pages/gpu-service/instances/components/add-modal.tsx b/src/pages/gpu-service/instances/components/add-modal.tsx index 2c5a2033..c055173c 100644 --- a/src/pages/gpu-service/instances/components/add-modal.tsx +++ b/src/pages/gpu-service/instances/components/add-modal.tsx @@ -5,7 +5,7 @@ import { SearchOutlined } from '@ant-design/icons'; import { ColumnWrapper, GSDrawer, ModalFooter } from '@gpustack/core-ui'; import { useIntl } from '@umijs/max'; import { Empty, Input, Typography } from 'antd'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import styled from 'styled-components'; import { ListItem as TemplateItem } from '../../templates/config/types'; import useQueryTemplates from '../../templates/services/use-query-templates'; @@ -52,6 +52,23 @@ type AddModalProps = { onCancel: () => void; }; +type InstanceTypeSelection = { + instanceType?: string; + manufacturer?: string; +}; + +const EMPTY_INSTANCE_TYPE_SELECTION: InstanceTypeSelection = {}; + +const matchKeyword = (fields: Array, keyword: string) => { + const trimmed = keyword.trim().toLowerCase(); + if (!trimmed) return true; + return fields.some((text) => + String(text ?? '') + .toLowerCase() + .includes(trimmed) + ); +}; + const ColTitle: React.FC<{ children: React.ReactNode; style?: React.CSSProperties; @@ -86,10 +103,10 @@ const AddModal: React.FC = ({ }) => { const intl = useIntl(); const form = useRef(null); - const autoSelectedRef = useRef(false); - const [selectedInstanceType, setSelectedInstanceType] = useState(); - const [selectedManufacturer, setSelectedManufacturer] = useState(); - const [templateId, setTemplateId] = useState(); + const sessionRef = useRef(0); + const [instanceTypeSelection, setInstanceTypeSelection] = + useState(EMPTY_INSTANCE_TYPE_SELECTION); + const [templateId, setTemplateId] = useState(); const [instanceKeyword, setInstanceKeyword] = useState(''); const [templateKeyword, setTemplateKeyword] = useState(''); @@ -101,118 +118,117 @@ const AddModal: React.FC = ({ const { detailData: templatesData, fetchData: fetchTemplates } = useQueryTemplates(); - useEffect(() => { - if (open) { - autoSelectedRef.current = false; - fetchData({}); - fetchTemplates({ page: -1 }); - } else { - setSelectedInstanceType(undefined); - setSelectedManufacturer(undefined); - setTemplateId(undefined); - setInstanceKeyword(''); - setTemplateKeyword(''); - } - }, [open, fetchData, fetchTemplates]); - const instanceTypeList = detailData?.items || []; const templateList = templatesData?.items || []; - const filteredInstanceTypes = useMemo(() => { - const keyword = instanceKeyword.trim().toLowerCase(); - if (!keyword) { - return instanceTypeList; - } - return instanceTypeList.filter((item) => { - const name = item.metadata?.name || ''; - return [ - name, - item.spec?.memory ?? '', - item.status?.cpu?.capacity ?? '', - item.status?.ram?.capacity ?? '', - item.status?.accelerator?.remaining ?? '' - ].some((text) => String(text).toLowerCase().includes(keyword)); - }); - }, [instanceKeyword, instanceTypeList]); + const findTemplateByManufacturer = ( + manufacturer: string | undefined, + templates: TemplateItem[] + ) => { + return manufacturer + ? templates.find((t) => t.manufacturer === manufacturer) + : undefined; + }; - const manufacturerMatchedTemplates = useMemo(() => { - console.log( - 'Filtering templates by manufacturer:', - templateList, - selectedManufacturer - ); - if (!selectedManufacturer) { - return templateList; - } - return templateList.filter( - (item) => item.manufacturer === selectedManufacturer - ); - }, [selectedManufacturer, templateList]); + const applySelection = ( + instanceType: InstanceTypeItem, + template: TemplateItem | undefined + ) => { + const name = instanceType.metadata?.name; + const manufacturer = instanceType.spec?.manufacturer; - const filteredTemplates = useMemo(() => { - const keyword = templateKeyword.trim().toLowerCase(); - if (!keyword) { - return manufacturerMatchedTemplates; - } - return manufacturerMatchedTemplates.filter((item) => - [item.name, item.spec?.image, item.spec?.volumeMount].some((text) => - (text || '').toLowerCase().includes(keyword) - ) - ); - }, [templateKeyword, manufacturerMatchedTemplates]); + setInstanceTypeSelection({ instanceType: name, manufacturer }); + setTemplateId(template?.id); - useEffect(() => { - if (!open) return; - if (action !== PageAction.CREATE) return; - if (autoSelectedRef.current) return; - if (!instanceTypeList.length) return; - if (!templatesData) return; - - const firstInstanceType = instanceTypeList[0]; - const manufacturer = firstInstanceType.spec?.manufacturer; - const name = firstInstanceType.metadata?.name; - const matchedTemplate = manufacturer - ? templateList.find((t) => t.manufacturer === manufacturer) - : templateList[0]; - - autoSelectedRef.current = true; - setSelectedInstanceType(name); - setSelectedManufacturer(manufacturer); - if (matchedTemplate) { - setTemplateId(matchedTemplate.id); - } - - const applyToForm = () => { - const currentSpec = form.current?.getFieldsValue()?.spec || {}; - const updatedSpec = { - ...currentSpec, - type: name, - resources: { ...(currentSpec.resources || {}), accelerator: '1' } - }; - - if (matchedTemplate) { - form.current?.setFieldsValue({ - manufacturer: matchedTemplate.manufacturer, - spec: { - ...updatedSpec, - ...matchedTemplate.spec, - resources: { - ...(updatedSpec.resources || {}), - ...(matchedTemplate.spec?.resources || {}) - } - } - }); - } else { - form.current?.setFieldsValue({ spec: updatedSpec }); - } + const currentSpec = form.current?.getFieldsValue()?.spec || {}; + const updatedSpec = { + ...currentSpec, + type: name, + resources: { ...(currentSpec.resources || {}), accelerator: '1' } }; - if (form.current) { - applyToForm(); + if (template) { + form.current?.setFieldsValue({ + manufacturer: template.manufacturer, + spec: { + ...updatedSpec, + ...template.spec, + resources: { + ...(updatedSpec.resources || {}), + ...(template.spec?.resources || {}) + } + } + }); } else { - queueMicrotask(applyToForm); + form.current?.setFieldsValue({ + manufacturer: undefined, + spec: updatedSpec + }); } - }, [open, action, instanceTypeList, templateList, templatesData]); + }; + + const applyAutoSelection = ( + instanceTypes: InstanceTypeItem[], + templates: TemplateItem[] + ) => { + if (action !== PageAction.CREATE) return; + if (!instanceTypes.length) return; + + const first = instanceTypes[0]; + const template = findTemplateByManufacturer( + first.spec?.manufacturer, + templates + ); + + applySelection(first, template); + }; + + useEffect(() => { + if (!open) { + sessionRef.current += 1; + setInstanceTypeSelection(EMPTY_INSTANCE_TYPE_SELECTION); + setTemplateId(undefined); + setInstanceKeyword(''); + setTemplateKeyword(''); + return; + } + + const session = ++sessionRef.current; + Promise.all([fetchData({}), fetchTemplates({ page: -1 })]).then( + ([instanceRes, templatesRes]) => { + if (sessionRef.current !== session) return; + applyAutoSelection(instanceRes?.items || [], templatesRes?.items || []); + } + ); + }, [open]); + + // filter instance types + const filteredInstanceTypes = instanceTypeList.filter((item) => + matchKeyword( + [ + item.metadata?.name, + item.spec?.memory, + item.status?.cpu?.capacity, + item.status?.ram?.capacity, + item.status?.accelerator?.remaining + ], + instanceKeyword + ) + ); + + // filter templates based on selection and keyword + const filteredTemplates = templateList.filter((item) => { + if ( + instanceTypeSelection.manufacturer && + item.manufacturer !== instanceTypeSelection.manufacturer + ) { + return false; + } + return matchKeyword( + [item.name, item.spec?.image, item.spec?.volumeMount], + templateKeyword + ); + }); const handleSubmit = () => { form.current?.submit(); @@ -230,54 +246,16 @@ const AddModal: React.FC = ({ }; const handleInstanceTypeChange = (item: InstanceTypeItem) => { - const name = item.metadata?.name; - const manufacturer = item.spec?.manufacturer; - setSelectedInstanceType(name); - setSelectedManufacturer(manufacturer); - - const currentSpec = form.current?.getFieldsValue()?.spec || {}; - const updatedSpec = { - ...currentSpec, - type: name, - resources: { ...(currentSpec.resources || {}), accelerator: '1' } - }; - - const currentTemplate = templateList.find((t) => t.id === templateId); - const isMatched = - !manufacturer || - (!!currentTemplate && currentTemplate.manufacturer === manufacturer); - - if (isMatched) { - form.current?.setFieldsValue({ spec: updatedSpec }); - return; - } - - const firstTemplate = templateList.find( - (t) => t.manufacturer === manufacturer + const template = findTemplateByManufacturer( + item.spec?.manufacturer, + templateList ); - - if (firstTemplate) { - setTemplateId(firstTemplate.id); - form.current?.setFieldsValue({ - manufacturer: firstTemplate.manufacturer, - spec: { - ...updatedSpec, - ...firstTemplate.spec - } - }); - } else { - setTemplateId(undefined); - form.current?.setFieldsValue({ - manufacturer: undefined, - spec: updatedSpec - }); - } + applySelection(item, template); }; const handleTemplateChange = (id: number, item: TemplateItem) => { setTemplateId(id); form.current?.setFieldsValue({ - manufacturer: item.manufacturer, spec: { ...form.current?.getFieldsValue()?.spec, ...item.spec @@ -328,7 +306,7 @@ const AddModal: React.FC = ({ />