improvement(knowledge): react-doctor perf pass across the knowledge base module (#5332)

* improvement(knowledge): react-doctor perf pass across the knowledge base module * improvement(knowledge): drop stable mutate fn from useCallback deps * docs(rules): tighten sim-react-performance mutation + toSorted wording
2026-09-24 15:45:35 +08:00 · 2026-07-01 13:23:24 -07:00
parent 76537c890c
commit 9204b4a248
12 changed files with 114 additions and 83 deletions
@@ -75,6 +75,21 @@ return [...items].sort(compare)

 **Do NOT reach for `toSorted()` / `toReversed()` / `with()` / `toSpliced()` on client render paths.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is the correct default everywhere client code runs. Only consider the immutable methods in Node-only code (server routes, scripts) on Node ≥20, where the runtime is known.

+## Run independent awaits in parallel
+
+Sequential `await`s that don't consume each other's result serialize latency for nothing — in an async Server Component or a route handler this directly delays the response. Kick them off together with `Promise.all` and destructure.
+
+```typescript
+// ✗ Bad — waits for params, then separately waits for searchParams
+const { id } = await params
+const { kbName } = await searchParams
+
+// ✓ Good — one combined wait
+const [{ id }, { kbName }] = await Promise.all([params, searchParams])
+```
+
+Only keep awaits sequential when a later call genuinely uses an earlier result, or when the ordering is deliberate (rate-limited batches, retry loops, write-then-read).
+
 ## Local feature barrels are the convention — do not "fix" them

 Tooling (e.g. react-doctor's `no-barrel-import`) will flag imports from local `index.ts` barrels as a bundle cost. In this repo that is a **false positive**: barrel imports for 3+ export folders are mandated by `.claude/rules/sim-imports.md`. Leave them.
@@ -219,15 +219,19 @@ export function Document({
      ? Math.max(1, Math.min(currentPageFromURL, maxSearchPages))
      : 1
  const searchTotalPages = Math.max(1, maxSearchPages)
-  const searchStartIndex = (searchCurrentPage - 1) * SEARCH_PAGE_SIZE
-  const paginatedSearchResults = searchResults.slice(
-    searchStartIndex,
-    searchStartIndex + SEARCH_PAGE_SIZE
-  )

-  const rawDisplayChunks = showingSearch ? paginatedSearchResults : initialChunks
-
-  const displayChunks = rawDisplayChunks ?? []
+  /**
+   * Stable chunk list for the current view. Memoized so the many downstream
+   * `useMemo`/`useCallback` hooks that depend on it don't recompute every render
+   * (search pagination `.slice()` otherwise yields a fresh array each time).
+   */
+  const displayChunks = useMemo<ChunkData[]>(() => {
+    if (showingSearch) {
+      const start = (searchCurrentPage - 1) * SEARCH_PAGE_SIZE
+      return searchResults.slice(start, start + SEARCH_PAGE_SIZE)
+    }
+    return initialChunks ?? []
+  }, [showingSearch, searchResults, searchCurrentPage, initialChunks])

  const currentPage = showingSearch ? searchCurrentPage : initialPage
  const totalPages = showingSearch ? searchTotalPages : initialTotalPages
@@ -983,7 +987,7 @@ export function Document({
      ...editorBreadcrumbBase,
      { label: selectedChunk ? `Chunk #${selectedChunk.chunkIndex}` : '', terminal: true },
    ],
-    [editorBreadcrumbBase, selectedChunk?.chunkIndex]
+    [editorBreadcrumbBase, selectedChunk]
  )

  const loadingBreadcrumbs = useMemo<BreadcrumbItem[]>(
@@ -21,8 +21,7 @@ export async function generateMetadata({ searchParams }: DocumentPageProps): Pro
 }

 export default async function DocumentChunksPage({ params, searchParams }: DocumentPageProps) {
-  const { id, documentId } = await params
-  const { kbName, docName } = await searchParams
+  const [{ id, documentId }, { kbName, docName }] = await Promise.all([params, searchParams])

  return (
    <Suspense fallback={null}>
@@ -180,12 +180,13 @@ interface TagValue {
 */
 function getDocumentTags(doc: DocumentData, definitions: TagDefinition[]): TagValue[] {
  const result: TagValue[] = []
+  const defsBySlot = new Map(definitions.map((d) => [d.tagSlot, d]))

  for (const slot of ALL_TAG_SLOTS) {
    const raw = doc[slot]
    if (raw == null) continue

-    const def = definitions.find((d) => d.tagSlot === slot)
+    const def = defsBySlot.get(slot)
    const fieldType = def?.fieldType || getFieldTypeForSlot(slot) || 'text'

    let value: string
@@ -263,22 +264,21 @@ export function KnowledgeBase({

  const activeTagFilters: DocumentTagFilter[] = useMemo(
    () =>
-      tagFilterEntries
-        .filter((f) => {
-          if (!f.tagSlot || !f.value.trim()) return false
-          // A `between` filter only applies once both bounds are set. Sending it
-          // with just the lower bound would be rejected at the API boundary and
-          // break the whole list while the user is still entering the range.
-          if (f.operator === 'between' && !f.valueTo.trim()) return false
-          return true
-        })
-        .map((f) => ({
+      tagFilterEntries.reduce<DocumentTagFilter[]>((acc, f) => {
+        if (!f.tagSlot || !f.value.trim()) return acc
+        // A `between` filter only applies once both bounds are set. Sending it
+        // with just the lower bound would be rejected at the API boundary and
+        // break the whole list while the user is still entering the range.
+        if (f.operator === 'between' && !f.valueTo.trim()) return acc
+        acc.push({
          tagSlot: f.tagSlot,
          fieldType: f.fieldType,
          operator: f.operator,
          value: f.value,
          ...(f.operator === 'between' ? { valueTo: f.valueTo } : {}),
-        })),
+        })
+        return acc
+      }, []),
    [tagFilterEntries]
  )

@@ -1023,9 +1023,9 @@ export function KnowledgeBase({
            },
          ]
        : []),
-      ...tagFilterEntries
-        .filter((f) => f.tagSlot && f.value.trim())
-        .map((f) => ({
+      ...tagFilterEntries.reduce<{ label: string; onRemove: () => void }[]>((acc, f) => {
+        if (!f.tagSlot || !f.value.trim()) return acc
+        acc.push({
          label: `${f.tagName}: ${f.value}`,
          onRemove: () => {
            const updated = tagFilterEntries.filter((e) => e.id !== f.id)
@@ -1034,7 +1034,9 @@ export function KnowledgeBase({
            setSelectedDocuments(new Set())
            setIsSelectAllMode(false)
          },
-        })),
+        })
+        return acc
+      }, []),
    ],
    [enabledFilter, tagFilterEntries]
  )
@@ -170,13 +170,13 @@ export function BaseTagsModal({ open, onOpenChange, knowledgeBaseId }: BaseTagsM
  }

  const fieldTypeOptions: ComboboxOption[] = useMemo(() => {
-    return SUPPORTED_FIELD_TYPES.filter((type) => hasAvailableSlots(type)).map((type) => {
+    return SUPPORTED_FIELD_TYPES.reduce<ComboboxOption[]>((acc, type) => {
      const { used, max } = getSlotUsageByFieldType(type)
-      return {
-        value: type,
-        label: `${FIELD_TYPE_LABELS[type]} (${used}/${max})`,
+      if (used < max) {
+        acc.push({ value: type, label: `${FIELD_TYPE_LABELS[type]} (${used}/${max})` })
      }
-    })
+      return acc
+    }, [])
  }, [kbTagDefinitions])

  const saveTagDefinition = async () => {
@@ -40,8 +40,9 @@ export function ConnectorSelectorField({
    if (credentialId) ctx.oauthCredential = credentialId
    if (field.mimeType) ctx.mimeType = field.mimeType

+    const fieldsById = new Map(configFields.map((f) => [f.id, f]))
    for (const depFieldId of getDependsOnFields(field.dependsOn)) {
-      const depField = configFields.find((f) => f.id === depFieldId)
+      const depField = fieldsById.get(depFieldId)
      const canonicalId = depField?.canonicalParamId ?? depFieldId
      const depValue = resolveDepValue(depFieldId, configFields, canonicalModes, sourceConfig)
      if (depValue && SELECTOR_CONTEXT_FIELDS.has(canonicalId as keyof SelectorContext)) {
@@ -96,12 +96,13 @@ export function ConnectorsSection({
  }, [])

  const syncTriggeredAt = useRef<Record<string, number>>({})
-  const cooldownTimers = useRef<Set<ReturnType<typeof setTimeout>>>(new Set())
+  const cooldownTimersRef = useRef<Set<ReturnType<typeof setTimeout>> | null>(null)
+  cooldownTimersRef.current ??= new Set()
  const [, forceUpdate] = useState(0)

  useEffect(() => {
    return () => {
-      for (const timer of cooldownTimers.current) {
+      for (const timer of cooldownTimersRef.current ?? []) {
        clearTimeout(timer)
      }
    }
@@ -126,10 +127,10 @@ export function ConnectorsSection({
          onSuccess: () => {
            setError(null)
            const timer = setTimeout(() => {
-              cooldownTimers.current.delete(timer)
+              cooldownTimersRef.current?.delete(timer)
              forceUpdate((n) => n + 1)
            }, SYNC_COOLDOWN_MS)
-            cooldownTimers.current.add(timer)
+            cooldownTimersRef.current?.add(timer)
          },
          onError: (err) => {
            logger.error('Sync trigger failed', { error: err.message })
@@ -302,8 +303,10 @@ function ConnectorCard({

  const serviceId = connectorDef?.auth.mode === 'oauth' ? connectorDef.auth.provider : undefined
  const providerId = serviceId ? getProviderIdFromServiceId(serviceId) : undefined
-  const requiredScopes =
-    connectorDef?.auth.mode === 'oauth' ? (connectorDef.auth.requiredScopes ?? []) : []
+  const requiredScopes = useMemo(
+    () => (connectorDef?.auth.mode === 'oauth' ? (connectorDef.auth.requiredScopes ?? []) : []),
+    [connectorDef]
+  )

  const { data: credentials, refetch: refetchCredentials } = useOAuthCredentials(providerId, {
    workspaceId,
@@ -79,10 +79,10 @@ function valuesEqual(a: unknown, b: unknown): boolean {
  const toArray = (v: unknown): string[] | null => {
    if (Array.isArray(v)) return v.filter((x): x is string => typeof x === 'string')
    if (typeof v === 'string') {
-      return v
-        .split(',')
-        .map((s) => s.trim())
-        .filter(Boolean)
+      return v.split(',').flatMap((s) => {
+        const t = s.trim()
+        return t ? [t] : []
+      })
    }
    return null
  }
@@ -159,10 +159,10 @@ export function EditConnectorModal({
        if (Array.isArray(rawValue)) {
          config[field.id] = rawValue.filter((v): v is string => typeof v === 'string')
        } else if (typeof rawValue === 'string') {
-          config[field.id] = rawValue
-            .split(',')
-            .map((s) => s.trim())
-            .filter(Boolean)
+          config[field.id] = rawValue.split(',').flatMap((s) => {
+            const t = s.trim()
+            return t ? [t] : []
+          })
        } else {
          config[field.id] = []
        }
@@ -44,10 +44,10 @@ function coerceForField(field: ConnectorConfigField, raw: unknown): ConfigFieldV
    if (typeof raw === 'string') {
      const trimmed = raw.trim()
      if (!trimmed) return []
-      return trimmed
-        .split(',')
-        .map((s) => s.trim())
-        .filter(Boolean)
+      return trimmed.split(',').flatMap((s) => {
+        const t = s.trim()
+        return t ? [t] : []
+      })
    }
    return []
  }
@@ -119,7 +119,7 @@ export function useConnectorConfigFields({
      for (const field of group) {
        for (const dep of map.get(field.id) ?? []) {
          allDependents.add(dep)
-          const depField = connectorConfig.configFields.find((f) => f.id === dep)
+          const depField = fieldsById.get(dep)
          if (depField?.canonicalParamId) {
            for (const sibling of canonicalGroups.get(depField.canonicalParamId) ?? []) {
              allDependents.add(sibling.id)
@@ -133,7 +133,7 @@ export function useConnectorConfigFields({
    }
    for (const [key, value] of map) result.set(key, [...value])
    return result
-  }, [connectorConfig, canonicalGroups])
+  }, [connectorConfig, canonicalGroups, fieldsById])

  const isFieldVisible = useCallback(
    (field: ConnectorConfigField): boolean => {
@@ -17,8 +17,7 @@ export async function generateMetadata({ searchParams }: PageProps): Promise<Met
 }

 export default async function KnowledgeBasePage({ params, searchParams }: PageProps) {
-  const { id } = await params
-  const { kbName } = await searchParams
+  const [{ id }, { kbName }] = await Promise.all([params, searchParams])

  return (
    <Suspense fallback={null}>
@@ -8,6 +8,7 @@ import { BaseTagsModal } from '@/app/workspace/[workspaceId]/knowledge/[id]/comp
 import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
 import { useContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/hooks'
 import { CONNECTOR_META_REGISTRY } from '@/connectors/registry'
+import type { ConnectorMeta } from '@/connectors/types'
 import { DeleteKnowledgeBaseModal } from '../delete-knowledge-base-modal/delete-knowledge-base-modal'
 import { EditKnowledgeBaseModal } from '../edit-knowledge-base-modal/edit-knowledge-base-modal'
 import { KnowledgeBaseContextMenu } from '../knowledge-base-context-menu/knowledge-base-context-menu'
@@ -104,9 +105,11 @@ export function BaseCard({
  const [isDeleting, setIsDeleting] = useState(false)
  const connectorEntries = useMemo(
    () =>
-      connectorTypes
-        .map((type) => ({ type, config: CONNECTOR_META_REGISTRY[type] }))
-        .filter((entry) => Boolean(entry.config?.icon)),
+      connectorTypes.reduce<{ type: string; config: ConnectorMeta }[]>((acc, type) => {
+        const config = CONNECTOR_META_REGISTRY[type]
+        if (config?.icon) acc.push({ type, config })
+        return acc
+      }, []),
    [connectorTypes]
  )
  const visibleConnectorEntries = useMemo(() => connectorEntries.slice(0, 3), [connectorEntries])
@@ -203,6 +203,36 @@ const uploadFileThroughAPI = async (
 const toAbsoluteUrl = (path: string): string =>
  path.startsWith('http') ? path : `${window.location.origin}${path}`

+/**
+ * Build the {@link UploadedFile} payload from a `File`, carrying through any
+ * `tagN` fields the caller attached to it. Pure — kept at module scope so it
+ * isn't rebuilt on every render of the hook.
+ */
+const buildUploadedFile = (file: File, fileUrl: string): UploadedFile => {
+  const f = file as File & {
+    tag1?: string
+    tag2?: string
+    tag3?: string
+    tag4?: string
+    tag5?: string
+    tag6?: string
+    tag7?: string
+  }
+  return {
+    filename: file.name,
+    fileUrl,
+    fileSize: file.size,
+    mimeType: getFileContentType(file),
+    tag1: f.tag1,
+    tag2: f.tag2,
+    tag3: f.tag3,
+    tag4: f.tag4,
+    tag5: f.tag5,
+    tag6: f.tag6,
+    tag7: f.tag7,
+  }
+}
+
 export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) {
  const queryClient = useQueryClient()
  const [isUploading, setIsUploading] = useState(false)
@@ -213,31 +243,6 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) {
  })
  const [uploadError, setUploadError] = useState<UploadError | null>(null)

-  const buildUploadedFile = (file: File, fileUrl: string): UploadedFile => {
-    const f = file as File & {
-      tag1?: string
-      tag2?: string
-      tag3?: string
-      tag4?: string
-      tag5?: string
-      tag6?: string
-      tag7?: string
-    }
-    return {
-      filename: file.name,
-      fileUrl,
-      fileSize: file.size,
-      mimeType: getFileContentType(file),
-      tag1: f.tag1,
-      tag2: f.tag2,
-      tag3: f.tag3,
-      tag4: f.tag4,
-      tag5: f.tag5,
-      tag6: f.tag6,
-      tag7: f.tag7,
-    }
-  }
-
  const updateFileStatus = (fileIndex: number, patch: Partial<FileUploadStatus>) => {
    setUploadProgress((prev) => ({
      ...prev,