perf(comment): static comment renderer + lazy editors + memoized list (#340)
The comment panel lagged for seconds on open and stuttered on every resolve/apply
with many comments (real case: 30 open + 326 resolved ≈ 356 threads), because each
comment body mounted a full TipTap/ProseMirror editor, both tabs mounted at once,
and any mutation re-rendered the whole list.
- CommentContentView: static recursive renderer of comment ProseMirror JSON (no
editor instance) for the read-only body — supports exactly CommentEditor's node
set (doc/paragraph/text/hardBreak/mention) + marks (bold/italic/strike/code/
link), reproducing the 3-level DOM nesting for pixel-identical CSS. Unknown
node/mark or unparseable content degrades that one comment to the read-only
CommentEditor; legacy non-JSON strings render as plain text.
SECURITY: link hrefs are protocol-allowlisted (safeHref, mirroring
@tiptap/extension-link) so a stored comment with a `javascript:`/`data:` href
cannot XSS — the old TipTap read-only path sanitized this; the static renderer
must too. Control-char smuggling (java\tscript:) is stripped before the check.
- MentionContent extracted from MentionView, shared by the TipTap NodeView and the
static renderer (identical user/page-mention behavior).
- keepMounted={false} on the tabs: the inactive tab no longer mounts its editors.
- Lazy reply editor: a stub until click/focus, then the real editor (kept mounted
so the draft survives thread re-renders).
- React.memo(CommentListItem) + a childrenByParent map (replaces the per-thread
O(n^2) filter) + localized reply-send pending state: resolve/apply/reply now
re-render only the touched thread.
- Progressive first paint: useCommentsQuery no longer blocks on hasNextPage.
Gate: client comment+mention suites 22/22 passed, tsc --noEmit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,241 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
// The fallback path renders the full TipTap editor; stub it so we can assert the
|
||||
// safety valve fired without pulling in the editor stack.
|
||||
vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
default: () => <div data-testid="comment-editor-fallback" />,
|
||||
}));
|
||||
|
||||
// Mention rendering hits react-query; stub the page/share queries so the mention
|
||||
// case renders in isolation.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import { CommentContentView } from "./comment-content-view";
|
||||
|
||||
function renderView(content: string | object) {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
<MemoryRouter>
|
||||
<CommentContentView content={content} />
|
||||
</MemoryRouter>
|
||||
</MantineProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
const doc = (content: any[]) => JSON.stringify({ type: "doc", content });
|
||||
const para = (content: any[]) => ({ type: "paragraph", content });
|
||||
const text = (t: string, marks?: any[]) => ({ type: "text", text: t, marks });
|
||||
|
||||
describe("CommentContentView", () => {
|
||||
it("renders paragraphs as <p> with text", () => {
|
||||
const { container } = renderView(doc([para([text("Hello world")])]));
|
||||
expect(screen.getByText("Hello world")).toBeDefined();
|
||||
expect(container.querySelector("p")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("reproduces the read-only CommentEditor DOM nesting for CSS parity", () => {
|
||||
const { container } = renderView(doc([para([text("x")])]));
|
||||
// outer .commentEditor > .ProseMirror (module) > .ProseMirror (global) > p
|
||||
const globalPm = container.querySelector("div.ProseMirror > p");
|
||||
expect(globalPm).not.toBeNull();
|
||||
});
|
||||
|
||||
it("renders the bold mark as <strong>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("bold", [{ type: "bold" }])])]),
|
||||
);
|
||||
const el = container.querySelector("strong");
|
||||
expect(el?.textContent).toBe("bold");
|
||||
});
|
||||
|
||||
it("renders the italic mark as <em>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("it", [{ type: "italic" }])])]),
|
||||
);
|
||||
expect(container.querySelector("em")?.textContent).toBe("it");
|
||||
});
|
||||
|
||||
it("renders the strike mark as <s>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("st", [{ type: "strike" }])])]),
|
||||
);
|
||||
expect(container.querySelector("s")?.textContent).toBe("st");
|
||||
});
|
||||
|
||||
it("renders the code mark as <code>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("co", [{ type: "code" }])])]),
|
||||
);
|
||||
expect(container.querySelector("code")?.textContent).toBe("co");
|
||||
});
|
||||
|
||||
it("renders the link mark as an anchor with safe rel/target", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("click", [
|
||||
{ type: "link", attrs: { href: "https://example.com" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
const a = container.querySelector("a");
|
||||
expect(a?.getAttribute("href")).toBe("https://example.com");
|
||||
expect(a?.getAttribute("target")).toBe("_blank");
|
||||
expect(a?.getAttribute("rel")).toBe("noopener noreferrer nofollow");
|
||||
expect(a?.textContent).toBe("click");
|
||||
});
|
||||
|
||||
it("neutralizes a javascript: link href (stored XSS) while keeping the text", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("click", [
|
||||
{ type: "link", attrs: { href: "javascript:alert(1)" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
const a = container.querySelector("a");
|
||||
expect(a).not.toBeNull();
|
||||
// No navigable javascript: href — attribute is absent (or empty).
|
||||
expect(a?.getAttribute("href")).toBeFalsy();
|
||||
// The link text is still rendered.
|
||||
expect(a?.textContent).toBe("click");
|
||||
});
|
||||
|
||||
it("neutralizes a control-char-obfuscated javascript: href", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("x", [
|
||||
{ type: "link", attrs: { href: "java\tscript:alert(1)" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBeFalsy();
|
||||
});
|
||||
|
||||
it("neutralizes a data: link href", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("x", [
|
||||
{
|
||||
type: "link",
|
||||
attrs: { href: "data:text/html,<script>alert(1)</script>" },
|
||||
},
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBeFalsy();
|
||||
});
|
||||
|
||||
it("preserves a mailto: link href (allowlisted scheme)", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("mail", [
|
||||
{ type: "link", attrs: { href: "mailto:a@b.com" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBe(
|
||||
"mailto:a@b.com",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves a relative link href (no scheme, not a script vector)", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("rel", [{ type: "link", attrs: { href: "/some/path" } }]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBe(
|
||||
"/some/path",
|
||||
);
|
||||
});
|
||||
|
||||
it("nests multiple marks on one text node", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("x", [{ type: "bold" }, { type: "italic" }])])]),
|
||||
);
|
||||
// bold wraps italic (or vice versa) — both elements exist around the text.
|
||||
expect(container.querySelector("strong")).not.toBeNull();
|
||||
expect(container.querySelector("em")).not.toBeNull();
|
||||
expect(screen.getByText("x")).toBeDefined();
|
||||
});
|
||||
|
||||
it("renders hardBreak as <br/>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("a"), { type: "hardBreak" }, text("b")])]),
|
||||
);
|
||||
expect(container.querySelector("br")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("renders a user mention as a styled span", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
{
|
||||
type: "mention",
|
||||
attrs: { label: "Alice", entityType: "user", entityId: "u1" },
|
||||
},
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(screen.getByText("@Alice")).toBeDefined();
|
||||
// No fallback to the editor.
|
||||
expect(screen.queryByTestId("comment-editor-fallback")).toBeNull();
|
||||
});
|
||||
|
||||
it("renders a page mention as a link", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
{
|
||||
type: "mention",
|
||||
attrs: {
|
||||
label: "Some Page",
|
||||
entityType: "page",
|
||||
slugId: "pg1",
|
||||
},
|
||||
},
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")).not.toBeNull();
|
||||
expect(screen.getByText("Some Page")).toBeDefined();
|
||||
});
|
||||
|
||||
it("renders a legacy plain-text (non-JSON) string as plain text", () => {
|
||||
renderView("just a legacy string");
|
||||
expect(screen.getByText("just a legacy string")).toBeDefined();
|
||||
expect(screen.queryByTestId("comment-editor-fallback")).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to CommentEditor for an unknown node type", () => {
|
||||
renderView(doc([{ type: "codeBlock", content: [text("x")] }]));
|
||||
expect(screen.getByTestId("comment-editor-fallback")).toBeDefined();
|
||||
});
|
||||
|
||||
it("falls back to CommentEditor for malformed JSON", () => {
|
||||
renderView('{"type":"doc","content":[');
|
||||
expect(screen.getByTestId("comment-editor-fallback")).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,194 @@
|
||||
import React from "react";
|
||||
import classes from "./comment.module.css";
|
||||
import { MentionContent } from "@/features/editor/components/mention/mention-view";
|
||||
import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
|
||||
// Static, editor-free renderer of a comment body (ProseMirror JSON). It walks the
|
||||
// document and emits plain DOM, avoiding the cost of a full TipTap/ProseMirror
|
||||
// instance per comment (the panel used to spin up 400+ editors on mount).
|
||||
//
|
||||
// The supported node/mark set MUST mirror what CommentEditor enables
|
||||
// (StarterKit + Mention + LinkExtension). Anything outside that set makes the
|
||||
// whole comment degrade to the read-only CommentEditor via the fallback below,
|
||||
// so we never show a half-rendered comment.
|
||||
|
||||
// Sentinel thrown when we hit a node/mark we don't know how to render statically.
|
||||
// Caught at the top level to trigger the CommentEditor fallback for the whole comment.
|
||||
class UnknownNodeError extends Error {}
|
||||
|
||||
// Protocol allowlist mirroring @tiptap/extension-link's default (the read-only
|
||||
// CommentEditor path relies on it to blank javascript:/data: hrefs). The static
|
||||
// renderer must apply the SAME sanitization because the backend stores comment
|
||||
// content verbatim and React does not neutralize javascript: in an href.
|
||||
const ALLOWED_URI_SCHEMES = /^(?:https?|ftps?|mailto|tel|callto|sms|cid|xmpp):/i;
|
||||
|
||||
function safeHref(href: unknown): string | undefined {
|
||||
if (typeof href !== "string") return undefined;
|
||||
// Strip control chars/whitespace that could smuggle a scheme past the test
|
||||
// (e.g. "java\tscript:").
|
||||
const cleaned = href.replace(/[\u0000-\u0020]/g, "").trim();
|
||||
// Allow relative/anchor/protocol-relative links (no scheme) — not script vectors.
|
||||
if (!/^[a-z][a-z0-9+.-]*:/i.test(cleaned)) return href;
|
||||
return ALLOWED_URI_SCHEMES.test(cleaned) ? href : undefined;
|
||||
}
|
||||
|
||||
interface PMMark {
|
||||
type: string;
|
||||
attrs?: Record<string, any>;
|
||||
}
|
||||
|
||||
interface PMNode {
|
||||
type: string;
|
||||
attrs?: Record<string, any>;
|
||||
content?: PMNode[];
|
||||
text?: string;
|
||||
marks?: PMMark[];
|
||||
}
|
||||
|
||||
// Wrap a text node's string in its marks (marks nest, e.g. bold + italic).
|
||||
function renderMarks(
|
||||
text: React.ReactNode,
|
||||
marks: PMMark[] | undefined,
|
||||
keyPrefix: string,
|
||||
): React.ReactNode {
|
||||
if (!marks || marks.length === 0) return text;
|
||||
|
||||
return marks.reduce<React.ReactNode>((acc, mark, i) => {
|
||||
const key = `${keyPrefix}-m${i}`;
|
||||
switch (mark.type) {
|
||||
case "bold":
|
||||
return <strong key={key}>{acc}</strong>;
|
||||
case "italic":
|
||||
return <em key={key}>{acc}</em>;
|
||||
case "strike":
|
||||
return <s key={key}>{acc}</s>;
|
||||
case "code":
|
||||
return <code key={key}>{acc}</code>;
|
||||
case "link": {
|
||||
// LinkExtension (TiptapLink) opens links in a new tab; keep the same
|
||||
// safe rel semantics the editor produces. Sanitize the href against the
|
||||
// extension's protocol allowlist — a disallowed scheme (javascript:,
|
||||
// data:) yields undefined so the anchor is non-navigable but still shows
|
||||
// its text, matching how extension-link blanks a bad href.
|
||||
const href = safeHref(mark.attrs?.href);
|
||||
return (
|
||||
<a
|
||||
key={key}
|
||||
href={href}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer nofollow"
|
||||
>
|
||||
{acc}
|
||||
</a>
|
||||
);
|
||||
}
|
||||
default:
|
||||
throw new UnknownNodeError(`Unknown mark type: ${mark.type}`);
|
||||
}
|
||||
}, text);
|
||||
}
|
||||
|
||||
function renderNode(node: PMNode, key: string): React.ReactNode {
|
||||
switch (node.type) {
|
||||
case "paragraph":
|
||||
return <p key={key}>{renderChildren(node.content, key)}</p>;
|
||||
case "text":
|
||||
return (
|
||||
<React.Fragment key={key}>
|
||||
{renderMarks(node.text ?? "", node.marks, key)}
|
||||
</React.Fragment>
|
||||
);
|
||||
case "hardBreak":
|
||||
return <br key={key} />;
|
||||
case "mention":
|
||||
return (
|
||||
<span key={key} style={{ display: "inline" }}>
|
||||
<MentionContent attrs={node.attrs as any} />
|
||||
</span>
|
||||
);
|
||||
default:
|
||||
throw new UnknownNodeError(`Unknown node type: ${node.type}`);
|
||||
}
|
||||
}
|
||||
|
||||
function renderChildren(
|
||||
content: PMNode[] | undefined,
|
||||
keyPrefix: string,
|
||||
): React.ReactNode {
|
||||
if (!content) return null;
|
||||
return content.map((child, i) => renderNode(child, `${keyPrefix}-${i}`));
|
||||
}
|
||||
|
||||
// Reproduce the exact DOM nesting the read-only CommentEditor renders so the
|
||||
// scoped CSS in comment.module.css (which targets
|
||||
// `.commentEditor .ProseMirror :global(.ProseMirror)` and `.ProseMirror p`)
|
||||
// applies pixel-for-pixel. Read-only => no data-editable / data-surface attrs.
|
||||
function Shell({ children }: { children: React.ReactNode }) {
|
||||
return (
|
||||
<div className={classes.commentEditor}>
|
||||
<div className={classes.ProseMirror}>
|
||||
<div className="ProseMirror">{children}</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
interface CommentContentViewProps {
|
||||
content: string | object;
|
||||
}
|
||||
|
||||
export function CommentContentView({ content }: CommentContentViewProps) {
|
||||
// Degrade this single comment to the old editor-based render (safety valve).
|
||||
const fallback = () => {
|
||||
if (import.meta.env.DEV) {
|
||||
console.warn(
|
||||
"CommentContentView: unsupported comment content, falling back to editor",
|
||||
);
|
||||
}
|
||||
return <CommentEditor defaultContent={content} editable={false} />;
|
||||
};
|
||||
|
||||
let doc: unknown = content;
|
||||
|
||||
if (typeof content === "string") {
|
||||
try {
|
||||
doc = JSON.parse(content);
|
||||
} catch {
|
||||
const trimmed = content.trim();
|
||||
// Looks like it was meant to be JSON but is malformed -> safety-valve fallback.
|
||||
if (trimmed.startsWith("{") || trimmed.startsWith("[")) {
|
||||
return fallback();
|
||||
}
|
||||
// Otherwise it's a legacy plain-text comment: render as a single paragraph.
|
||||
return (
|
||||
<Shell>
|
||||
<p>{content}</p>
|
||||
</Shell>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Double-stringified / legacy plain-text stored as a JSON string.
|
||||
if (typeof doc === "string") {
|
||||
return (
|
||||
<Shell>
|
||||
<p>{doc}</p>
|
||||
</Shell>
|
||||
);
|
||||
}
|
||||
|
||||
try {
|
||||
const pmDoc = doc as PMNode;
|
||||
if (!pmDoc || typeof pmDoc !== "object" || pmDoc.type !== "doc") {
|
||||
throw new UnknownNodeError("Not a ProseMirror doc");
|
||||
}
|
||||
return <Shell>{renderChildren(pmDoc.content, "n")}</Shell>;
|
||||
} catch (err) {
|
||||
if (err instanceof UnknownNodeError) {
|
||||
return fallback();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
export default CommentContentView;
|
||||
@@ -28,6 +28,16 @@ vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
default: () => <div data-testid="comment-editor" />,
|
||||
}));
|
||||
|
||||
// CommentContentView (used for the read-only body) imports the mention view,
|
||||
// which pulls page-query -> main.tsx (createRoot). Stub the queries so the item
|
||||
// renders in isolation without the app entry side-effect.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import CommentListItem from "./comment-list-item";
|
||||
import {
|
||||
canShowApply,
|
||||
@@ -286,3 +296,25 @@ describe("canShowDismiss predicate", () => {
|
||||
expect(canShowDismiss(c({ parentCommentId: "p" }), true, true)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("CommentListItem — read-only body renders statically", () => {
|
||||
it("renders the comment body as static text without a TipTap editor", () => {
|
||||
renderItem(
|
||||
baseComment({
|
||||
content: JSON.stringify({
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "Hello static world" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
// Body text is present...
|
||||
expect(screen.getByText("Hello static world")).toBeDefined();
|
||||
// ...and it did NOT go through the (mocked) CommentEditor instance.
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { Group, Text, Box, Badge, Button } from "@mantine/core";
|
||||
import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx";
|
||||
import React, { useEffect, useMemo, useRef, useState } from "react";
|
||||
import React, { useMemo, useRef, useState } from "react";
|
||||
import classes from "./comment.module.css";
|
||||
import { useAtom, useAtomValue } from "jotai";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago";
|
||||
import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
import CommentContentView from "@/features/comment/components/comment-content-view";
|
||||
import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms";
|
||||
import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import CommentMenu from "@/features/comment/components/comment-menu";
|
||||
@@ -50,7 +51,6 @@ function CommentListItem({
|
||||
const [isEditing, setIsEditing] = useState(false);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
const editor = useAtomValue(pageEditorAtom);
|
||||
const [content, setContent] = useState<string>(comment.content);
|
||||
const editContentRef = useRef<any>(null);
|
||||
const updateCommentMutation = useUpdateCommentMutation();
|
||||
const deleteCommentMutation = useDeleteCommentMutation(comment.pageId);
|
||||
@@ -78,22 +78,16 @@ function CommentListItem({
|
||||
const isOwnerOrAdmin =
|
||||
currentUser?.user?.id === comment.creatorId || userSpaceRole === "admin";
|
||||
|
||||
useEffect(() => {
|
||||
setContent(comment.content);
|
||||
}, [comment]);
|
||||
|
||||
async function handleUpdateComment() {
|
||||
try {
|
||||
setIsLoading(true);
|
||||
const commentToUpdate = {
|
||||
commentId: comment.id,
|
||||
content: JSON.stringify(editContentRef.current ?? content),
|
||||
content: JSON.stringify(editContentRef.current ?? comment.content),
|
||||
};
|
||||
await updateCommentMutation.mutateAsync(commentToUpdate);
|
||||
if (editContentRef.current) {
|
||||
setContent(editContentRef.current);
|
||||
editContentRef.current = null;
|
||||
}
|
||||
editContentRef.current = null;
|
||||
setIsEditing(false);
|
||||
} catch (error) {
|
||||
console.error("Failed to update comment:", error);
|
||||
@@ -350,11 +344,11 @@ function CommentListItem({
|
||||
)}
|
||||
|
||||
{!isEditing ? (
|
||||
<CommentEditor defaultContent={content} editable={false} />
|
||||
<CommentContentView content={comment.content} />
|
||||
) : (
|
||||
<>
|
||||
<CommentEditor
|
||||
defaultContent={content}
|
||||
defaultContent={comment.content}
|
||||
editable={true}
|
||||
onUpdate={(newContent: any) => { editContentRef.current = newContent; }}
|
||||
onSave={handleUpdateComment}
|
||||
@@ -374,4 +368,6 @@ function CommentListItem({
|
||||
);
|
||||
}
|
||||
|
||||
export default CommentListItem;
|
||||
// Memoized so a resolve/apply/reply cache update (which only replaces the touched
|
||||
// comment's object identity) re-renders that one thread, not all ~356 items.
|
||||
export default React.memo(CommentListItem);
|
||||
|
||||
@@ -23,7 +23,6 @@ import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import { useFocusWithin } from "@mantine/hooks";
|
||||
import { IComment } from "@/features/comment/types/comment.types.ts";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { IPagination } from "@/lib/types.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useGetSpaceBySlugQuery } from "@/features/space/queries/space-query.ts";
|
||||
@@ -46,7 +45,9 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
isError,
|
||||
} = useCommentsQuery({ pageId: page?.id });
|
||||
const createCommentMutation = useCreateCommentMutation();
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
// mutateAsync is a stable reference across renders; depend on it (not the
|
||||
// mutation object) so the reply/comment callbacks stay stable.
|
||||
const createCommentAsync = createCommentMutation.mutateAsync;
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
|
||||
const canEdit = page?.permissions?.canEdit ?? false;
|
||||
@@ -75,13 +76,28 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
return { activeComments: active, resolvedComments: resolved };
|
||||
}, [comments]);
|
||||
|
||||
// Index replies by their parent once, instead of an O(n^2) filter per thread.
|
||||
// The map ref changes on any comments update, so MemoizedChildComments re-runs
|
||||
// (cheap) and re-looks-up, while memoized CommentListItems skip unchanged items.
|
||||
const childrenByParent = useMemo(() => {
|
||||
const m = new Map<string, IComment[]>();
|
||||
for (const c of comments?.items ?? []) {
|
||||
if (c.parentCommentId) {
|
||||
const arr = m.get(c.parentCommentId);
|
||||
if (arr) arr.push(c);
|
||||
else m.set(c.parentCommentId, [c]);
|
||||
}
|
||||
}
|
||||
return m;
|
||||
}, [comments?.items]);
|
||||
|
||||
const [isPageCommentLoading, setIsPageCommentLoading] = useState(false);
|
||||
|
||||
const handleAddPageComment = useCallback(
|
||||
async (_commentId: string, content: string) => {
|
||||
try {
|
||||
setIsPageCommentLoading(true);
|
||||
const createdComment = await createCommentMutation.mutateAsync({
|
||||
const createdComment = await createCommentAsync({
|
||||
pageId: page?.id,
|
||||
content: JSON.stringify(content),
|
||||
});
|
||||
@@ -100,27 +116,26 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
setIsPageCommentLoading(false);
|
||||
}
|
||||
},
|
||||
[createCommentMutation, page?.id],
|
||||
[createCommentAsync, page?.id],
|
||||
);
|
||||
|
||||
const handleAddReply = useCallback(
|
||||
async (commentId: string, content: string) => {
|
||||
// Pending state lives inside CommentEditorWithActions so sending a reply
|
||||
// does not churn renderComments and re-render the whole list.
|
||||
try {
|
||||
setIsLoading(true);
|
||||
const commentData = {
|
||||
pageId: page?.id,
|
||||
parentCommentId: commentId,
|
||||
content: JSON.stringify(content),
|
||||
};
|
||||
|
||||
await createCommentMutation.mutateAsync(commentData);
|
||||
await createCommentAsync(commentData);
|
||||
} catch (error) {
|
||||
console.error("Failed to post comment:", error);
|
||||
} finally {
|
||||
setIsLoading(false);
|
||||
}
|
||||
},
|
||||
[createCommentMutation, page?.id],
|
||||
[createCommentAsync, page?.id],
|
||||
);
|
||||
|
||||
const renderComments = useCallback(
|
||||
@@ -143,7 +158,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
userSpaceRole={space?.membership?.role}
|
||||
/>
|
||||
<MemoizedChildComments
|
||||
comments={comments}
|
||||
childrenByParent={childrenByParent}
|
||||
parentId={comment.id}
|
||||
pageId={page?.id}
|
||||
canComment={canComment}
|
||||
@@ -158,16 +173,15 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
<CommentEditorWithActions
|
||||
commentId={comment.id}
|
||||
onSave={handleAddReply}
|
||||
isLoading={isLoading}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
</Paper>
|
||||
),
|
||||
[
|
||||
comments,
|
||||
childrenByParent,
|
||||
handleAddReply,
|
||||
isLoading,
|
||||
page?.id,
|
||||
space?.membership?.role,
|
||||
canComment,
|
||||
canEdit,
|
||||
@@ -203,6 +217,9 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
<Tabs
|
||||
defaultValue="open"
|
||||
variant="default"
|
||||
// Do not mount the inactive tab: the Resolved list (hundreds of items)
|
||||
// stays unmounted while the Open tab is shown, and vice versa.
|
||||
keepMounted={false}
|
||||
style={{
|
||||
flex: "1 1 auto",
|
||||
display: "flex",
|
||||
@@ -307,7 +324,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
}
|
||||
|
||||
interface ChildCommentsProps {
|
||||
comments: IPagination<IComment>;
|
||||
childrenByParent: Map<string, IComment[]>;
|
||||
parentId: string;
|
||||
pageId: string;
|
||||
canComment: boolean;
|
||||
@@ -315,24 +332,18 @@ interface ChildCommentsProps {
|
||||
userSpaceRole?: string;
|
||||
}
|
||||
const ChildComments = ({
|
||||
comments,
|
||||
childrenByParent,
|
||||
parentId,
|
||||
pageId,
|
||||
canComment,
|
||||
canEdit,
|
||||
userSpaceRole,
|
||||
}: ChildCommentsProps) => {
|
||||
const getChildComments = useCallback(
|
||||
(parentId: string) =>
|
||||
comments.items.filter(
|
||||
(comment: IComment) => comment.parentCommentId === parentId,
|
||||
),
|
||||
[comments.items],
|
||||
);
|
||||
const children = childrenByParent.get(parentId) ?? [];
|
||||
|
||||
return (
|
||||
<div>
|
||||
{getChildComments(parentId).map((childComment) => (
|
||||
{children.map((childComment) => (
|
||||
<div key={childComment.id}>
|
||||
<CommentListItem
|
||||
comment={childComment}
|
||||
@@ -342,7 +353,7 @@ const ChildComments = ({
|
||||
userSpaceRole={userSpaceRole}
|
||||
/>
|
||||
<MemoizedChildComments
|
||||
comments={comments}
|
||||
childrenByParent={childrenByParent}
|
||||
parentId={childComment.id}
|
||||
pageId={pageId}
|
||||
canComment={canComment}
|
||||
@@ -360,19 +371,58 @@ const MemoizedChildComments = memo(ChildComments);
|
||||
const CommentEditorWithActions = ({
|
||||
commentId,
|
||||
onSave,
|
||||
isLoading,
|
||||
placeholder = undefined,
|
||||
}) => {
|
||||
const { t } = useTranslation();
|
||||
// Lazily mount the TipTap reply editor: until the user interacts with the
|
||||
// stub, no editor instance is created for this thread. Once mounted it stays
|
||||
// mounted so the draft is preserved.
|
||||
const [mounted, setMounted] = useState(false);
|
||||
const [content, setContent] = useState("");
|
||||
const [isSending, setIsSending] = useState(false);
|
||||
const { ref, focused } = useFocusWithin();
|
||||
const commentEditorRef = useRef(null);
|
||||
|
||||
const handleSave = useCallback(() => {
|
||||
onSave(commentId, content);
|
||||
setContent("");
|
||||
commentEditorRef.current?.clearContent();
|
||||
const activate = useCallback(() => setMounted(true), []);
|
||||
|
||||
const handleSave = useCallback(async () => {
|
||||
try {
|
||||
setIsSending(true);
|
||||
await onSave(commentId, content);
|
||||
setContent("");
|
||||
commentEditorRef.current?.clearContent();
|
||||
} finally {
|
||||
setIsSending(false);
|
||||
}
|
||||
}, [commentId, content, onSave]);
|
||||
|
||||
if (!mounted) {
|
||||
return (
|
||||
<div
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
onClick={activate}
|
||||
onFocus={activate}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" || e.key === " ") {
|
||||
e.preventDefault();
|
||||
activate();
|
||||
}
|
||||
}}
|
||||
style={{
|
||||
padding: "6px",
|
||||
fontSize: "var(--mantine-font-size-sm)",
|
||||
lineHeight: 1.4,
|
||||
color: "var(--mantine-color-placeholder)",
|
||||
cursor: "text",
|
||||
borderRadius: "var(--mantine-radius-sm)",
|
||||
}}
|
||||
>
|
||||
{placeholder || t("Reply...")}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div ref={ref}>
|
||||
<CommentEditor
|
||||
@@ -381,8 +431,9 @@ const CommentEditorWithActions = ({
|
||||
onSave={handleSave}
|
||||
editable={true}
|
||||
placeholder={placeholder}
|
||||
autofocus={true}
|
||||
/>
|
||||
{focused && <CommentActions onSave={handleSave} isLoading={isLoading} />}
|
||||
{focused && <CommentActions onSave={handleSave} isLoading={isSending} />}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user