fix(ui): stop recreating markdown mention observers (#3809)
Fixes #3759 ## Thinking Path > - Paperclip orchestrates AI agents and issue workflows, so the comment composer has to stay stable under normal typing. > - The affected subsystem is the shared markdown comment editor used across issue and workflow surfaces. > - That editor decorates mention links after Lexical updates the editable DOM. > - The current mention decoration effect recreates its `MutationObserver` whenever `value` changes, which happens on every keystroke. > - That observer also reacts to the DOM mutations produced by mention decoration itself, creating unnecessary observer churn in Chrome. > - This pull request keeps one observer instance alive, batches decoration work into `requestAnimationFrame`, and disconnects the observer while decoration writes run. > - The benefit is lower observer churn while preserving the existing mention chip behavior. ## What Changed - Removed `value` from the mention-decoration observer effect dependency list so the observer is not recreated on every external value update. - Batched mention decoration with `requestAnimationFrame` and temporarily disconnected the observer while DOM decorations are applied to avoid self-triggered feedback loops. - Added a regression test that verifies external value changes do not recreate the mention decoration observer. ## Verification - `pnpm install --frozen-lockfile` - `pnpm --filter @paperclipai/ui exec tsc --noEmit` - `pnpm --filter @paperclipai/ui exec vitest run src/components/MarkdownEditor.test.tsx` currently fails before test collection on the existing repo baseline with `TypeError: undefined is not an object (evaluating 'z.string')` from `packages/shared/src/adapter-type.ts` ## Risks - Low risk. The change is scoped to the mention decoration observer lifecycle and keeps the existing decoration logic intact. - The main behavior change is deferring decoration to the next animation frame instead of running immediately on every observed mutation. ## Model Used - OpenAI Codex, GPT-5-based coding agent with local tool use in the Codex CLI environment. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
8b04147ca4
commit
d62c5adb7a
|
|
@ -302,6 +302,70 @@ describe("MarkdownEditor", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("does not recreate the mention decoration observer when the external value changes", async () => {
|
||||
const originalMutationObserver = globalThis.MutationObserver;
|
||||
|
||||
class MockMutationObserver implements MutationObserver {
|
||||
static instances: MockMutationObserver[] = [];
|
||||
|
||||
readonly observe = vi.fn();
|
||||
readonly disconnect = vi.fn();
|
||||
readonly takeRecords = vi.fn<() => MutationRecord[]>(() => []);
|
||||
|
||||
constructor(readonly callback: MutationCallback) {
|
||||
MockMutationObserver.instances.push(this);
|
||||
}
|
||||
}
|
||||
|
||||
vi.stubGlobal("MutationObserver", MockMutationObserver);
|
||||
const root = createRoot(container);
|
||||
|
||||
try {
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<MarkdownEditor
|
||||
value="First value"
|
||||
onChange={() => {}}
|
||||
placeholder="Markdown body"
|
||||
/>,
|
||||
);
|
||||
});
|
||||
|
||||
await flush();
|
||||
const editable = container.querySelector('[contenteditable="true"]');
|
||||
expect(editable).not.toBeNull();
|
||||
const mentionObserverCountAfterInitialRender = MockMutationObserver.instances.filter(
|
||||
(observer) => observer.observe.mock.calls.some(([target]) => target === editable),
|
||||
).length;
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<MarkdownEditor
|
||||
value="Updated value"
|
||||
onChange={() => {}}
|
||||
placeholder="Markdown body"
|
||||
/>,
|
||||
);
|
||||
});
|
||||
|
||||
await flush();
|
||||
|
||||
// A separate rich-editor health observer is expected to recreate when the
|
||||
// controlled value changes. This assertion only covers the mention
|
||||
// decoration observer that attaches to the editable element itself.
|
||||
expect(
|
||||
MockMutationObserver.instances.filter(
|
||||
(observer) => observer.observe.mock.calls.some(([target]) => target === editable),
|
||||
),
|
||||
).toHaveLength(mentionObserverCountAfterInitialRender);
|
||||
} finally {
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
vi.stubGlobal("MutationObserver", originalMutationObserver);
|
||||
}
|
||||
});
|
||||
|
||||
it("converts advisory-style html image tags to markdown image syntax before mounting the editor", async () => {
|
||||
const root = createRoot(container);
|
||||
|
||||
|
|
|
|||
|
|
@ -1000,17 +1000,39 @@ export const MarkdownEditor = forwardRef<MarkdownEditorRef, MarkdownEditorProps>
|
|||
useEffect(() => {
|
||||
const editable = containerRef.current?.querySelector('[contenteditable="true"]');
|
||||
if (!editable) return;
|
||||
decorateProjectMentions();
|
||||
const observer = new MutationObserver(() => {
|
||||
let frameId: number | null = null;
|
||||
let disposed = false;
|
||||
const observe = () => {
|
||||
observer.observe(editable, {
|
||||
subtree: true,
|
||||
childList: true,
|
||||
characterData: true,
|
||||
});
|
||||
};
|
||||
const flushDecorations = () => {
|
||||
frameId = null;
|
||||
if (disposed) return;
|
||||
observer.disconnect();
|
||||
decorateProjectMentions();
|
||||
if (!disposed) observe();
|
||||
};
|
||||
const scheduleDecorations = () => {
|
||||
if (frameId !== null) return;
|
||||
frameId = requestAnimationFrame(flushDecorations);
|
||||
};
|
||||
const observer = new MutationObserver(() => {
|
||||
scheduleDecorations();
|
||||
});
|
||||
observer.observe(editable, {
|
||||
subtree: true,
|
||||
childList: true,
|
||||
characterData: true,
|
||||
});
|
||||
return () => observer.disconnect();
|
||||
}, [decorateProjectMentions, value]);
|
||||
|
||||
flushDecorations();
|
||||
return () => {
|
||||
disposed = true;
|
||||
observer.disconnect();
|
||||
if (frameId !== null) {
|
||||
cancelAnimationFrame(frameId);
|
||||
}
|
||||
};
|
||||
}, [decorateProjectMentions]);
|
||||
|
||||
const selectMention = useCallback(
|
||||
(option: AutocompleteOption) => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue