From d126a86c5d64fbd73a060364a02ab05ca977512c Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 24 Mar 2026 20:38:23 +0200 Subject: [PATCH] refactor(site/src/pages/AgentsPage): remove redundant memo and Context.Provider (#23507) The React Compiler (babel-plugin-react-compiler@1.0.0) handles memoization automatically for all components in the AgentsPage compiled path. Three memo() wrappers were redundant: - ChatMessageItem in ConversationTimeline.tsx - LazyFileDiff in DiffViewer.tsx - ChatTreeNode in AgentsSidebar.tsx Also migrate three Context.Provider usages to the React 19 shorthand () and simplify the EmbedContext export to use the context directly instead of re-exporting .Provider as an alias. --- site/src/pages/AgentsPage/AgentEmbedPage.tsx | 6 +- .../AgentDetail/ConversationTimeline.tsx | 475 +++++++++--------- .../components/DiffViewer/DiffViewer.tsx | 119 +++-- .../AgentsPage/components/EmbedContext.tsx | 2 +- .../components/Sidebar/AgentsSidebar.tsx | 9 +- 5 files changed, 302 insertions(+), 309 deletions(-) diff --git a/site/src/pages/AgentsPage/AgentEmbedPage.tsx b/site/src/pages/AgentsPage/AgentEmbedPage.tsx index fa05ad10d5..82eb62a042 100644 --- a/site/src/pages/AgentsPage/AgentEmbedPage.tsx +++ b/site/src/pages/AgentsPage/AgentEmbedPage.tsx @@ -11,7 +11,7 @@ import { Outlet, useParams } from "react-router"; import type { AgentsOutletContext } from "./AgentsPage"; import { bootstrapChatEmbedSession, - EmbedProvider, + EmbedContext, } from "./components/EmbedContext"; import type { ChatDetailError } from "./utils/usageLimitMessage"; @@ -178,13 +178,13 @@ const AgentEmbedPage: FC = () => { if (auth.isSignedIn) { return ( - + - + ); } diff --git a/site/src/pages/AgentsPage/components/AgentDetail/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/AgentDetail/ConversationTimeline.tsx index 17dc421d88..daefb8a4ea 100644 --- a/site/src/pages/AgentsPage/components/AgentDetail/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/AgentDetail/ConversationTimeline.tsx @@ -21,7 +21,6 @@ import { FileTextIcon, PencilIcon } from "lucide-react"; import { type FC, Fragment, - memo, type ReactNode, useEffect, useLayoutEffect, @@ -371,7 +370,7 @@ function renderBlockList({ return { elements, renderedToolIDs }; } -const ChatMessageItem = memo<{ +interface ChatMessageItemProps { message: TypesGen.ChatMessage; parsed: ParsedMessageContent; onEditUserMessage?: ( @@ -387,250 +386,246 @@ const ChatMessageItem = memo<{ // overlay to indicate truncated content. fadeFromBottom?: boolean; urlTransform?: UrlTransform; -}>( - ({ - message, - parsed, - onEditUserMessage, - editingMessageId, - savingMessageId, - isAfterEditingMessage = false, - fadeFromBottom = false, +} + +const ChatMessageItem: FC = ({ + message, + parsed, + onEditUserMessage, + editingMessageId, + savingMessageId, + isAfterEditingMessage = false, + fadeFromBottom = false, + urlTransform, +}) => { + const isUser = message.role === "user"; + const isSavingMessage = savingMessageId === message.id; + const [previewImage, setPreviewImage] = useState(null); + const [previewText, setPreviewText] = useState(null); + const toolByID = new Map(parsed.tools.map((tool) => [tool.id, tool])); + + if ( + parsed.toolResults.length > 0 && + parsed.toolCalls.length === 0 && + parsed.markdown === "" && + parsed.reasoning === "" + ) { + return null; + } + + // Hide messages that consist entirely of provider-executed + // tool results. The parser skips these parts, so the parsed + // output is empty and would show a "no renderable content" + // fallback. + const parts = message.content ?? []; + if ( + parts.length > 0 && + parts.every((p) => p.type === "tool-result" && p.provider_executed) + ) { + return null; + } + + const hasRenderableContent = + parsed.blocks.length > 0 || + parsed.tools.length > 0 || + parsed.sources.length > 0; + // Pre-compute the inline content for user messages so we + // avoid a filter + map inside the JSX return path. + const userInlineContent = isUser + ? parsed.blocks.filter( + ( + b, + ): b is + | Extract + | Extract => + b.type === "response" || b.type === "file-reference", + ) + : []; + + const userFileBlocks = isUser + ? parsed.blocks.filter( + (b): b is Extract => b.type === "file", + ) + : []; + + const hasUserMessageBody = + userInlineContent.length > 0 || Boolean(parsed.markdown?.trim()); + const hasFileBlocks = userFileBlocks.length > 0; + + const conversationItemProps: { role: "user" | "assistant" } = { + role: isUser ? "user" : "assistant", + }; + const { elements: orderedBlocks, renderedToolIDs } = renderBlockList({ + blocks: parsed.blocks, + toolByID, + keyPrefix: String(message.id), + onImageClick: setPreviewImage, + onTextFileClick: (content) => setPreviewText(content), urlTransform, - }) => { - const isUser = message.role === "user"; - const isSavingMessage = savingMessageId === message.id; - const [previewImage, setPreviewImage] = useState(null); - const [previewText, setPreviewText] = useState(null); - const toolByID = new Map(parsed.tools.map((tool) => [tool.id, tool])); + }); + const remainingTools = parsed.tools.filter( + (tool) => !renderedToolIDs.has(tool.id), + ); - if ( - parsed.toolResults.length > 0 && - parsed.toolCalls.length === 0 && - parsed.markdown === "" && - parsed.reasoning === "" - ) { - return null; - } - - // Hide messages that consist entirely of provider-executed - // tool results. The parser skips these parts, so the parsed - // output is empty and would show a "no renderable content" - // fallback. - const parts = message.content ?? []; - if ( - parts.length > 0 && - parts.every((p) => p.type === "tool-result" && p.provider_executed) - ) { - return null; - } - - const hasRenderableContent = - parsed.blocks.length > 0 || - parsed.tools.length > 0 || - parsed.sources.length > 0; - // Pre-compute the inline content for user messages so we - // avoid a filter + map inside the JSX return path. - const userInlineContent = isUser - ? parsed.blocks.filter( - ( - b, - ): b is - | Extract - | Extract => - b.type === "response" || b.type === "file-reference", - ) - : []; - - const userFileBlocks = isUser - ? parsed.blocks.filter( - (b): b is Extract => b.type === "file", - ) - : []; - - const hasUserMessageBody = - userInlineContent.length > 0 || Boolean(parsed.markdown?.trim()); - const hasFileBlocks = userFileBlocks.length > 0; - - const conversationItemProps: { role: "user" | "assistant" } = { - role: isUser ? "user" : "assistant", - }; - const { elements: orderedBlocks, renderedToolIDs } = renderBlockList({ - blocks: parsed.blocks, - toolByID, - keyPrefix: String(message.id), - onImageClick: setPreviewImage, - onTextFileClick: (content) => setPreviewText(content), - urlTransform, - }); - const remainingTools = parsed.tools.filter( - (tool) => !renderedToolIDs.has(tool.id), - ); - - return ( -
- - {isUser ? ( - - + + {isUser ? ( + + +
+ {(hasUserMessageBody || hasFileBlocks) && ( +
+ {hasUserMessageBody && ( + + {userInlineContent.length > 0 + ? userInlineContent.map((block, i) => + block.type === "response" ? ( + {block.text} + ) : ( + + ), + ) + : parsed.markdown || ""} + + )} + {isSavingMessage && ( + + )} + {onEditUserMessage && !isSavingMessage && ( + + + + + Edit message + + )} +
)} - style={ - fadeFromBottom - ? { maxHeight: "var(--clip-h, none)" } - : undefined - } - > -
- {(hasUserMessageBody || hasFileBlocks) && ( -
- {hasUserMessageBody && ( - - {userInlineContent.length > 0 - ? userInlineContent.map((block, i) => - block.type === "response" ? ( - {block.text} - ) : ( - - ), - ) - : parsed.markdown || ""} - - )} - {isSavingMessage && ( - - )} - {onEditUserMessage && !isSavingMessage && ( - - - - - - Edit message - - - )} -
- )} - {(() => { - if (userFileBlocks.length === 0) return null; - return ( -
- {userFileBlocks.map((block, i) => - renderFileBlock({ - block, - key: `user-file-${block.file_id ?? i}`, - onImageClick: setPreviewImage, - onTextFileClick: setPreviewText, - }), - )} -
- ); - })()} - {fadeFromBottom && ( + {(() => { + if (userFileBlocks.length === 0) return null; + return (
- )} -
- - - ) : ( - - -
- {orderedBlocks} - {remainingTools.map((tool) => ( - - ))} - {!hasRenderableContent && ( -
- Message has no renderable content. + className={cn( + hasUserMessageBody && "mt-2", + "flex flex-wrap gap-2", + )} + > + {userFileBlocks.map((block, i) => + renderFileBlock({ + block, + key: `user-file-${block.file_id ?? i}`, + onImageClick: setPreviewImage, + onTextFileClick: setPreviewText, + }), + )}
- )} -
-
-
- )} - - {previewImage && ( - setPreviewImage(null)} - /> + ); + })()} + {fadeFromBottom && ( +
+ )} +
+ + + ) : ( + + +
+ {orderedBlocks} + {remainingTools.map((tool) => ( + + ))} + {!hasRenderableContent && ( +
+ Message has no renderable content. +
+ )} +
+
+
)} - {previewText !== null && ( - setPreviewText(null)} - /> - )} -
- ); - }, -); + + {previewImage && ( + setPreviewImage(null)} + /> + )} + {previewText !== null && ( + setPreviewText(null)} + /> + )} +
+ ); +}; export const StreamingOutput: FC<{ streamState: StreamState | null; diff --git a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx index 900d9a7a2d..78516727d2 100644 --- a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx +++ b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx @@ -19,7 +19,6 @@ import { ChevronRightIcon } from "lucide-react"; import { type ComponentProps, type FC, - memo, type ReactNode, useCallback, useEffect, @@ -408,11 +407,11 @@ const DiffScrollContainer: FC<{ scrollBarClassName="w-1.5" viewportClassName="[&>div]:!block" > - +
{children}
-
+ ); }; @@ -430,72 +429,72 @@ const DiffScrollContainer: FC<{ * FileDiff that the user has already scrolled past, which avoids * layout shifts and repeated highlighting work. */ -const LazyFileDiff = memo<{ +interface LazyFileDiffProps { fileDiff: FileDiffMetadata; options: ComponentProps["options"]; lineAnnotations?: DiffLineAnnotation[]; renderAnnotation?: (annotation: DiffLineAnnotation) => ReactNode; selectedLines?: SelectedLineRange | null; -}>( - ({ - fileDiff, - options, - lineAnnotations, - renderAnnotation: renderAnnotationProp, - selectedLines, - }) => { - const placeholderRef = useRef(null); - const [visible, setVisible] = useState(false); +} - useEffect(() => { - const el = placeholderRef.current; - if (!el || visible) { - return; - } - const observer = new IntersectionObserver( - ([entry]) => { - if (entry.isIntersecting) { - setVisible(true); - observer.disconnect(); - } - }, - // Pre-load files that are within one viewport-height of - // the visible area so they are ready before the user - // scrolls to them. - { rootMargin: "100% 0px" }, - ); - observer.observe(el); - return () => observer.disconnect(); - }, [visible]); +const LazyFileDiff: FC = ({ + fileDiff, + options, + lineAnnotations, + renderAnnotation: renderAnnotationProp, + selectedLines, +}) => { + const placeholderRef = useRef(null); + const [visible, setVisible] = useState(false); - if (!visible) { - return ( -
- - - - -
- ); + useEffect(() => { + const el = placeholderRef.current; + if (!el || visible) { + return; } - - return ( - + const observer = new IntersectionObserver( + ([entry]) => { + if (entry.isIntersecting) { + setVisible(true); + observer.disconnect(); + } + }, + // Pre-load files that are within one viewport-height of + // the visible area so they are ready before the user + // scrolls to them. + { rootMargin: "100% 0px" }, ); - }, -); + observer.observe(el); + return () => observer.disconnect(); + }, [visible]); + + if (!visible) { + return ( +
+ + + + +
+ ); + } + + return ( + + ); +}; // ------------------------------------------------------------------- // Main component diff --git a/site/src/pages/AgentsPage/components/EmbedContext.tsx b/site/src/pages/AgentsPage/components/EmbedContext.tsx index f0aaefda98..f3c329d4cf 100644 --- a/site/src/pages/AgentsPage/components/EmbedContext.tsx +++ b/site/src/pages/AgentsPage/components/EmbedContext.tsx @@ -13,7 +13,7 @@ const EmbedContext = createContext({ isEmbedded: false, }); -export const EmbedProvider = EmbedContext.Provider; +export { EmbedContext }; export const useEmbedContext = () => useContext(EmbedContext); diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx index dcee40d04e..796aba7be9 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx @@ -59,7 +59,6 @@ import { useDashboard } from "modules/dashboard/useDashboard"; import { createContext, type FC, - memo, useContext, useEffect, useRef, @@ -354,7 +353,7 @@ interface ChatTreeNodeProps { readonly isChildNode: boolean; } -const ChatTreeNode = memo(({ chat, isChildNode }) => { +const ChatTreeNode: FC = ({ chat, isChildNode }) => { const { chatTree, chatById, @@ -578,7 +577,7 @@ const ChatTreeNode = memo(({ chat, isChildNode }) => { )}
); -}); +}; export const AgentsSidebar: FC = (props) => { const { @@ -786,7 +785,7 @@ export const AgentsSidebar: FC = (props) => { ) : ( - + {visibleRootIDs.length === 0 ? (

@@ -889,7 +888,7 @@ export const AgentsSidebar: FC = (props) => { isFetchingNextPage={isFetchingNextPage} /> )} - + )}