lee/pentagi
1
0
Fork 0
mirror of https://github.com/vxcontrol/pentagi.git synced 2026-08-28 22:16:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(knowledges): scope content subscription off the form + untangle save/guard cycle

Browse Source 
Typing in the knowledge markdown editor re-rendered the whole KnowledgeForm on
every keystroke: form.watch('content') — used only to toggle the anonymize
button's disabled state — subscribed the top-level component, dragging the form
body, layout, and metadata fields through a re-render per character. Measured on
a multi-KB document: max input processing 58ms -> 21ms, keystrokes over 25ms
20 -> 0, long tasks 1 -> 0.

- Move the content subscription into a small module-scope KnowledgeFormHeader
  wrapper (scoped useWatch), so only the header reacts to typing; the form body,
  layout, and editor stay put. Mirrors the existing pattern in
  knowledge-form-controls.tsx.
- Removing the watch let the React Compiler optimize KnowledgeForm, which then
  flagged the skipNextBlockRef latest-ref (react-hooks/immutability). Rather than
  suppress it, untangle the performSave<->guard cycle the ref existed to break:
  performSave now returns the result and the *caller* owns navigation. The form
  Save button (defined after the guard) does the CREATE redirect via the stable
  skipNextBlock; the unsaved-changes dialog's "Save and leave" saves and lets the
  guard proceed the navigation the user initiated. No eslint-disable, no memo.

Behavior note: on CREATE, the Save button lands on the new document; the
"Save and leave" dialog now lands on the destination the user was navigating to
(it previously raced between the two). Verified live on the local stack:
create/update via button and dialog, delete on a dirty form, anonymize toggle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Sergey Kozyrenko
2026-06-27 12:58:51 +07:00
co-authored by Claude Opus 4.8
parent bb8739072b
commit 01fa02d3b4
1 changed files with 71 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/features/knowledges/knowledge-form.tsx
+71 -57
View File
@@ -1,8 +1,8 @@
import { useMutation } from '@apollo/client/react';
import { zodResolver } from '@hookform/resolvers/zod';
import { Save } from 'lucide-react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { type FieldPath, type SubmitHandler, useForm } from 'react-hook-form';
import { type ComponentProps, useCallback, useState } from 'react';
import { type Control, type FieldPath, type SubmitHandler, useForm, useWatch } from 'react-hook-form';
import { useNavigate } from 'react-router-dom';
import { toast } from 'sonner';
import { z } from 'zod';
@@ -181,6 +181,11 @@ export interface SubmitResult {
redirectTo?: string;
}
interface KnowledgeFormHeaderProps extends Omit<ComponentProps<typeof KnowledgeHeader>, 'isAnonymizeDisabled'> {
control: Control<FormValues>;
isSaving?: boolean;
}
interface KnowledgeFormProps {
initialValues: FormValues;
isNew: boolean;
@@ -224,8 +229,13 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
const { control, formState, handleSubmit, reset } = form;
const { isDirty, isValid } = formState;
// Saves and resets the form; returns the page's submit result (with an
// optional `redirectTo`) so the *caller* owns navigation. Keeping navigation
// out of here lets `onSaveFromDialog` feed the guard without depending on
// `guard.skipNextBlock` — which would otherwise form a real cycle
// (guard ← onSaveFromDialog ← performSave ← guard.skipNextBlock).
const performSave = useCallback(
async (values: FormValues): Promise<boolean> => {
async (values: FormValues): Promise<null | SubmitResult> => {
try {
// Snapshot dirty flags from the latest formState. We read it
// here (instead of capturing into deps) so partial-update
@@ -240,54 +250,19 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
// saved fragment for some reason.
const resetValues = result.document ? documentToFormValues(result.document) : values;
// Reset BEFORE navigate so `isDirty` is false by the time the
// blocker re-evaluates. We also `skipNextBlock` defensively
// because reset's state propagation is async.
// Reset BEFORE the caller navigates so `isDirty` is false by the
// time the blocker re-evaluates (the caller also calls
// `skipNextBlock` to cover reset's async state propagation).
reset(resetValues, { keepDefaultValues: false });
if (result.redirectTo) {
skipNextBlockRef.current();
navigate(result.redirectTo);
}
return true;
return result;
} catch (error) {
Log.error('Failed to save knowledge document', error);
return false;
return null;
}
},
[form, navigate, onSubmit, reset],
);
// The ref below breaks an otherwise circular hook dependency:
//
// performSave → skipNextBlockRef.current() (ref filled by effect below)
// onSaveFromDialog → performSave
// useUnsavedChangesGuard({ onSave: onSaveFromDialog }) → exposes skipNextBlock
// useEffect → wires the exposed skipNextBlock back into the ref
//
// Replacing the ref with a plain dep would force `performSave` to depend
// on `guard.skipNextBlock`, which is produced by a hook (`guard`) whose
// own input (`onSave`) closes over `performSave` — a real cycle that
// can't be expressed in deps without `useRef`.
const skipNextBlockRef = useRef<() => void>(() => {});
const onSubmitWithGuard: SubmitHandler<FormValues> = useCallback(
async (values) => {
if (isSaving) {
return;
}
setIsSaving(true);
try {
await performSave(values);
} finally {
setIsSaving(false);
}
},
[isSaving, performSave],
[form, onSubmit, reset],
);
const onSaveFromDialog = useCallback(async (): Promise<boolean> => {
@@ -308,7 +283,12 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
setIsSaving(true);
try {
return await performSave(parsed.data);
// The guard proceeds the originally-blocked navigation on success;
// a CREATE's `redirectTo` is intentionally not honored here ("Save
// and leave" leaves to where the user was going, not the new doc).
const result = await performSave(parsed.data);
return result !== null;
} finally {
setIsSaving(false);
}
@@ -319,10 +299,32 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
isFormValid: isValid,
onSave: onSaveFromDialog,
});
const { skipNextBlock } = guard;
useEffect(() => {
skipNextBlockRef.current = guard.skipNextBlock;
}, [guard.skipNextBlock]);
// Defined after `guard` so it can own the post-save redirect (CREATE only)
// via the stable `skipNextBlock` — the form-button path navigates to the new
// document; the dialog path above deliberately does not.
const onSubmitWithGuard: SubmitHandler<FormValues> = useCallback(
async (values) => {
if (isSaving) {
return;
}
setIsSaving(true);
try {
const result = await performSave(values);
if (result?.redirectTo) {
skipNextBlock();
navigate(result.redirectTo);
}
} finally {
setIsSaving(false);
}
},
[isSaving, navigate, performSave, skipNextBlock],
);
const canSubmit = !isSaving && isValid && (isNew || isDirty);
@@ -335,12 +337,6 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
/>
);
// Subscribe to `content` so the anonymize button toggles its disabled
// state as the user types. `form.watch('content')` triggers a re-render
// on every keystroke, which is what we want for snappy UX.
const contentValue = form.watch('content');
const isAnonymizeDisabled = isAnonymizing || isSaving || !contentValue?.trim();
const handleAnonymize = useCallback(async () => {
const currentContent = form.getValues('content');
@@ -387,14 +383,15 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
className={isDesktop ? 'flex h-[100dvh] min-h-0 w-full flex-col' : 'flex min-h-[100dvh] flex-col'}
onSubmit={handleSubmit(onSubmitWithGuard)}
>
<KnowledgeHeader
<KnowledgeFormHeader
canAnonymize={canAnonymize}
isAnonymizeDisabled={isAnonymizeDisabled}
control={control}
isAnonymizing={isAnonymizing}
isNew={isNew}
isSaving={isSaving}
knowledge={knowledge}
onAnonymize={handleAnonymize}
onBeforeNavigateAway={() => skipNextBlockRef.current()}
onBeforeNavigateAway={skipNextBlock}
saveButton={saveButton}
/>
{isDesktop ? (
@@ -426,3 +423,20 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
</>
);
}
// Scoped subscription: the `content` watch lives here, not in KnowledgeForm, so
// only the header re-renders as the user types — the form body, layout, and
// markdown editor stay still. Just enough reactivity to toggle the anonymize
// button's disabled state.
function KnowledgeFormHeader({ control, isAnonymizing, isSaving = false, ...rest }: KnowledgeFormHeaderProps) {
const content = useWatch({ control, name: 'content' });
const isAnonymizeDisabled = isAnonymizing || isSaving || !content?.trim();
return (
<KnowledgeHeader
{...rest}
isAnonymizeDisabled={isAnonymizeDisabled}
isAnonymizing={isAnonymizing}
/>
);
}