refactor: instances add-modal

2026-05-15 16:22:07 +08:00
parent 9dfaf990fd
commit 3f9852c932
2 changed files with 393 additions and 150 deletions
@@ -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<Selection>({});
+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 });
+};
+```
@@ -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<unknown>, 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<AddModalProps> = ({
 }) => {
  const intl = useIntl();
  const form = useRef<any>(null);
-  const autoSelectedRef = useRef(false);
-  const [selectedInstanceType, setSelectedInstanceType] = useState<string>();
-  const [selectedManufacturer, setSelectedManufacturer] = useState<string>();
-  const [templateId, setTemplateId] = useState<number>();
+  const sessionRef = useRef(0);
+  const [instanceTypeSelection, setInstanceTypeSelection] =
+    useState<InstanceTypeSelection>(EMPTY_INSTANCE_TYPE_SELECTION);
+  const [templateId, setTemplateId] = useState<number | undefined>();
  const [instanceKeyword, setInstanceKeyword] = useState('');
  const [templateKeyword, setTemplateKeyword] = useState('');

@@ -101,118 +118,117 @@ const AddModal: React.FC<AddModalProps> = ({
  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<AddModalProps> = ({
  };

  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<AddModalProps> = ({
                />
              </div>
              <InstanceTypeList
-                value={selectedInstanceType}
+                value={instanceTypeSelection.instanceType}
                dataList={filteredInstanceTypes}
                loading={instanceTypesLoading}
                onChange={handleInstanceTypeChange}