From b93d60c416603f64e7e2c3cf16ef0e35aba96b23 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 6 Mar 2026 19:09:52 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=9E=EF=B8=8F=20refactor:=20Image=20Ren?= =?UTF-8?q?dering=20with=20Preview=20Caching=20and=20Layout=20Reservation?= =?UTF-8?q?=20(#12114)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: Update Image Component to Remove Lazy Loading and Enhance Rendering - Removed the react-lazy-load-image-component dependency from the Image component, simplifying the image loading process. - Updated the Image component to use a standard tag with async decoding for improved performance and user experience. - Adjusted related tests to reflect changes in image rendering behavior and ensure proper functionality without lazy loading. * refactor: Enhance Image Handling and Caching Across Components - Introduced a new previewCache utility for managing local blob preview URLs, improving image loading efficiency. - Updated the Image component and related parts (FileRow, Files, Part, ImageAttachment, LogContent) to utilize cached previews, enhancing rendering performance and user experience. - Added width and height properties to the Image component for better layout management and consistency across different usages. - Improved file handling logic in useFileHandling to cache previews during file uploads, ensuring quick access to image data. - Enhanced overall code clarity and maintainability by streamlining image rendering logic and reducing redundancy. * refactor: Enhance OpenAIImageGen Component with Image Dimensions - Added width and height properties to the OpenAIImageGen component for improved image rendering and layout management. - Updated the Image component usage within OpenAIImageGen to utilize the new dimensions, enhancing visual consistency and performance. - Improved code clarity by destructuring additional properties from the attachment object, streamlining the component's logic. * refactor: Implement Image Size Caching in DialogImage Component - Introduced an imageSizeCache to store and retrieve image sizes, enhancing performance by reducing redundant fetch requests. - Updated the getImageSize function to first check the cache before making network requests, improving efficiency in image handling. - Added decoding attribute to the image element for optimized rendering behavior. * refactor: Enhance UserAvatar Component with Avatar Caching and Error Handling - Introduced avatar caching logic to optimize avatar resolution based on user ID and avatar source, improving performance and reducing redundant image loads. - Implemented error handling for failed image loads, allowing for fallback to a default avatar when necessary. - Updated UserAvatar props to streamline the interface by removing the user object and directly accepting avatar-related properties. - Enhanced overall code clarity and maintainability by refactoring the component structure and logic. * fix: Layout Shift in Message and Placeholder Components for Consistent Height Management - Adjusted the height of the PlaceholderRow and related message components to ensure consistent rendering with a minimum height of 31px. - Updated the MessageParts and ContentRender components to utilize a minimum height for better layout stability. - Enhanced overall code clarity by standardizing the structure of message-related components. * tests: Update FileRow Component to Prefer Cached Previews for Image Rendering - Modified the image URL selection logic in the FileRow component to prioritize cached previews over file paths when uploads are complete, enhancing rendering performance and user experience. - Updated related tests to reflect changes in image URL handling, ensuring accurate assertions for both preview and file path scenarios. - Introduced a fallback mechanism to use file paths when no preview exists, improving robustness in file handling. * fix: Image cache lifecycle and dialog decoding - Add deletePreview/clearPreviewCache to previewCache.ts for blob URL cleanup - Wire deletePreview into useFileDeletion to revoke blobs on file delete - Move dimensionCache.set into useMemo to avoid side effects during render - Extract IMAGE_MAX_W_PX constant (512) to document coupling with max-w-lg - Export _resetImageCaches for test isolation - Change DialogImage decoding from "sync" to "async" to avoid blocking main thread * fix: Avatar cache invalidation and cleanup - Include avatarSrc in cache invalidation to prevent stale avatars - Remove unused username parameter from resolveAvatar - Skip caching when userId is empty to prevent cache key collisions * test: Fix test isolation and type safety - Reset module-level dimensionCache/paintedUrls in beforeEach via _resetImageCaches - Replace any[] with typed mock signature in cn mock for both test files * chore: Code quality improvements from review - Use barrel imports for previewCache in Files.tsx and Part.tsx - Single Map.get with truthy check instead of has+get in useEventHandlers - Add JSDoc comments explaining EmptyText margin removal and PlaceholderRow height - Fix FileRow toast showing "Deleting file" when file isn't actually deleted (progress < 1) * fix: Address remaining review findings (R1-R3) - Add deletePreview calls to deleteFiles batch path to prevent blob URL leaks - Change useFileDeletion import from deep path to barrel (~/utils) - Change useMemo to useEffect for dimensionCache.set (side effect, not derived value) * fix: Address audit comments 2, 5, and 7 - Fix files preservation to distinguish null (missing) from [] (empty) in finalHandler - Add auto-revoke on overwrite in cachePreview to prevent leaked blobs - Add removePreviewEntry for key transfer without revoke - Clean up stale temp_file_id cache entry after promotion to permanent file_id --- client/package.json | 3 +- .../components/Chat/Input/Files/FileRow.tsx | 14 +- .../Input/Files/__tests__/FileRow.spec.tsx | 36 +++- .../Chat/Messages/Content/DialogImage.tsx | 15 +- .../Chat/Messages/Content/Files.tsx | 20 +- .../Chat/Messages/Content/Image.tsx | 90 +++++---- .../components/Chat/Messages/Content/Part.tsx | 9 +- .../Messages/Content/Parts/Attachment.tsx | 4 +- .../Chat/Messages/Content/Parts/EmptyText.tsx | 3 +- .../Messages/Content/Parts/LogContent.tsx | 2 + .../Parts/OpenAIImageGen/OpenAIImageGen.tsx | 11 +- .../Messages/Content/__tests__/Image.test.tsx | 177 ++++++++---------- .../Content/__tests__/OpenAIImageGen.test.tsx | 4 +- .../components/Chat/Messages/MessageParts.tsx | 4 +- .../Chat/Messages/ui/MessageRender.tsx | 2 +- .../Chat/Messages/ui/PlaceholderRow.tsx | 3 +- client/src/components/Endpoints/Icon.tsx | 131 ++++++++----- .../src/components/Messages/ContentRender.tsx | 2 +- client/src/components/Share/Message.tsx | 2 +- .../Files/__tests__/useFileHandling.test.ts | 8 +- client/src/hooks/Files/useFileDeletion.ts | 11 ++ client/src/hooks/Files/useFileHandling.ts | 10 +- client/src/hooks/SSE/useEventHandlers.ts | 17 ++ client/src/utils/index.ts | 1 + client/src/utils/previewCache.ts | 35 ++++ package-lock.json | 23 +-- 26 files changed, 390 insertions(+), 247 deletions(-) create mode 100644 client/src/utils/previewCache.ts diff --git a/client/package.json b/client/package.json index 31e75c4702..c588ccc6d9 100644 --- a/client/package.json +++ b/client/package.json @@ -94,7 +94,6 @@ "react-gtm-module": "^2.0.11", "react-hook-form": "^7.43.9", "react-i18next": "^15.4.0", - "react-lazy-load-image-component": "^1.6.0", "react-markdown": "^9.0.1", "react-resizable-panels": "^3.0.6", "react-router-dom": "^6.30.3", @@ -147,9 +146,9 @@ "jest": "^30.2.0", "jest-canvas-mock": "^2.5.2", "jest-environment-jsdom": "^30.2.0", - "monaco-editor": "^0.55.0", "jest-file-loader": "^1.0.3", "jest-junit": "^16.0.0", + "monaco-editor": "^0.55.0", "postcss": "^8.4.31", "postcss-preset-env": "^11.2.0", "tailwindcss": "^3.4.1", diff --git a/client/src/components/Chat/Input/Files/FileRow.tsx b/client/src/components/Chat/Input/Files/FileRow.tsx index ba8e73a8a2..bf04b16ade 100644 --- a/client/src/components/Chat/Input/Files/FileRow.tsx +++ b/client/src/components/Chat/Input/Files/FileRow.tsx @@ -3,10 +3,10 @@ import { useToastContext } from '@librechat/client'; import { EToolResources } from 'librechat-data-provider'; import type { ExtendedFile } from '~/common'; import { useDeleteFilesMutation } from '~/data-provider'; +import { logger, getCachedPreview } from '~/utils'; import { useFileDeletion } from '~/hooks/Files'; import FileContainer from './FileContainer'; import { useLocalize } from '~/hooks'; -import { logger } from '~/utils'; import Image from './Image'; export default function FileRow({ @@ -112,13 +112,15 @@ export default function FileRow({ ) .uniqueFiles.map((file: ExtendedFile, index: number) => { const handleDelete = () => { - showToast({ - message: localize('com_ui_deleting_file'), - status: 'info', - }); if (abortUpload && file.progress < 1) { abortUpload(); } + if (file.progress >= 1) { + showToast({ + message: localize('com_ui_deleting_file'), + status: 'info', + }); + } deleteFile({ file, setFiles }); }; const isImage = file.type?.startsWith('image') ?? false; @@ -134,7 +136,7 @@ export default function FileRow({ > {isImage ? ( ({ logger: { log: jest.fn(), }, + getCachedPreview: jest.fn(() => undefined), })); jest.mock('../Image', () => { @@ -95,7 +96,7 @@ describe('FileRow', () => { }; describe('Image URL Selection Logic', () => { - it('should use filepath instead of preview when progress is 1 (upload complete)', () => { + it('should prefer cached preview over filepath when upload is complete', () => { const file = createMockFile({ file_id: 'uploaded-file', preview: 'blob:http://localhost:3080/temp-preview', @@ -109,8 +110,7 @@ describe('FileRow', () => { renderFileRow(filesMap); const imageUrl = screen.getByTestId('image-url').textContent; - expect(imageUrl).toBe('/images/user123/uploaded-file__image.png'); - expect(imageUrl).not.toContain('blob:'); + expect(imageUrl).toBe('blob:http://localhost:3080/temp-preview'); }); it('should use preview when progress is less than 1 (uploading)', () => { @@ -147,7 +147,7 @@ describe('FileRow', () => { expect(imageUrl).toBe('/images/user123/file-without-preview__image.png'); }); - it('should use filepath when both preview and filepath exist and progress is exactly 1', () => { + it('should prefer preview over filepath when both exist and progress is 1', () => { const file = createMockFile({ file_id: 'complete-file', preview: 'blob:http://localhost:3080/old-blob', @@ -161,7 +161,7 @@ describe('FileRow', () => { renderFileRow(filesMap); const imageUrl = screen.getByTestId('image-url').textContent; - expect(imageUrl).toBe('/images/user123/complete-file__image.png'); + expect(imageUrl).toBe('blob:http://localhost:3080/old-blob'); }); }); @@ -284,7 +284,7 @@ describe('FileRow', () => { const urls = screen.getAllByTestId('image-url').map((el) => el.textContent); expect(urls).toContain('blob:http://localhost:3080/preview-1'); - expect(urls).toContain('/images/user123/file-2__image.png'); + expect(urls).toContain('blob:http://localhost:3080/preview-2'); }); it('should deduplicate files with the same file_id', () => { @@ -321,10 +321,10 @@ describe('FileRow', () => { }); }); - describe('Regression: Blob URL Bug Fix', () => { - it('should NOT use revoked blob URL after upload completes', () => { + describe('Preview Cache Integration', () => { + it('should prefer preview blob URL over filepath for zero-flicker rendering', () => { const file = createMockFile({ - file_id: 'regression-test', + file_id: 'cache-test', preview: 'blob:http://localhost:3080/d25f730c-152d-41f7-8d79-c9fa448f606b', filepath: '/images/68c98b26901ebe2d87c193a2/c0fe1b93-ba3d-456c-80be-9a492bfd9ed0__image.png', @@ -337,8 +337,24 @@ describe('FileRow', () => { renderFileRow(filesMap); const imageUrl = screen.getByTestId('image-url').textContent; + expect(imageUrl).toBe('blob:http://localhost:3080/d25f730c-152d-41f7-8d79-c9fa448f606b'); + }); - expect(imageUrl).not.toContain('blob:'); + it('should fall back to filepath when no preview exists', () => { + const file = createMockFile({ + file_id: 'no-preview', + preview: undefined, + filepath: + '/images/68c98b26901ebe2d87c193a2/c0fe1b93-ba3d-456c-80be-9a492bfd9ed0__image.png', + progress: 1, + }); + + const filesMap = new Map(); + filesMap.set(file.file_id, file); + + renderFileRow(filesMap); + + const imageUrl = screen.getByTestId('image-url').textContent; expect(imageUrl).toBe( '/images/68c98b26901ebe2d87c193a2/c0fe1b93-ba3d-456c-80be-9a492bfd9ed0__image.png', ); diff --git a/client/src/components/Chat/Messages/Content/DialogImage.tsx b/client/src/components/Chat/Messages/Content/DialogImage.tsx index cb496de646..b9cbe64555 100644 --- a/client/src/components/Chat/Messages/Content/DialogImage.tsx +++ b/client/src/components/Chat/Messages/Content/DialogImage.tsx @@ -4,6 +4,8 @@ import { Button, TooltipAnchor } from '@librechat/client'; import { X, ArrowDownToLine, PanelLeftOpen, PanelLeftClose, RotateCcw } from 'lucide-react'; import { useLocalize } from '~/hooks'; +const imageSizeCache = new Map(); + const getQualityStyles = (quality: string): string => { if (quality === 'high') { return 'bg-green-100 text-green-800'; @@ -50,18 +52,26 @@ export default function DialogImage({ const closeButtonRef = useRef(null); const getImageSize = useCallback(async (url: string) => { + const cached = imageSizeCache.get(url); + if (cached) { + return cached; + } try { const response = await fetch(url, { method: 'HEAD' }); const contentLength = response.headers.get('Content-Length'); if (contentLength) { const bytes = parseInt(contentLength, 10); - return formatFileSize(bytes); + const result = formatFileSize(bytes); + imageSizeCache.set(url, result); + return result; } const fullResponse = await fetch(url); const blob = await fullResponse.blob(); - return formatFileSize(blob.size); + const result = formatFileSize(blob.size); + imageSizeCache.set(url, result); + return result; } catch (error) { console.error('Error getting image size:', error); return null; @@ -355,6 +365,7 @@ export default function DialogImage({ ref={imageRef} src={src} alt="Image" + decoding="async" className="block max-h-[85vh] object-contain" style={{ maxWidth: getImageMaxWidth(), diff --git a/client/src/components/Chat/Messages/Content/Files.tsx b/client/src/components/Chat/Messages/Content/Files.tsx index c55148891a..504e48e883 100644 --- a/client/src/components/Chat/Messages/Content/Files.tsx +++ b/client/src/components/Chat/Messages/Content/Files.tsx @@ -1,6 +1,7 @@ import { useMemo, memo } from 'react'; import type { TFile, TMessage } from 'librechat-data-provider'; import FileContainer from '~/components/Chat/Input/Files/FileContainer'; +import { getCachedPreview } from '~/utils'; import Image from './Image'; const Files = ({ message }: { message?: TMessage }) => { @@ -17,13 +18,18 @@ const Files = ({ message }: { message?: TMessage }) => { {otherFiles.length > 0 && otherFiles.map((file) => )} {imageFiles.length > 0 && - imageFiles.map((file) => ( - - ))} + imageFiles.map((file) => { + const cached = file.file_id ? getCachedPreview(file.file_id) : undefined; + return ( + + ); + })} ); }; diff --git a/client/src/components/Chat/Messages/Content/Image.tsx b/client/src/components/Chat/Messages/Content/Image.tsx index 502a1c8e02..7e3e12e65b 100644 --- a/client/src/components/Chat/Messages/Content/Image.tsx +++ b/client/src/components/Chat/Messages/Content/Image.tsx @@ -1,18 +1,36 @@ -import React, { useState, useRef, useMemo } from 'react'; +import React, { useState, useRef, useMemo, useEffect } from 'react'; import { Skeleton } from '@librechat/client'; -import { LazyLoadImage } from 'react-lazy-load-image-component'; import { apiBaseUrl } from 'librechat-data-provider'; -import { cn } from '~/utils'; import DialogImage from './DialogImage'; +import { cn } from '~/utils'; /** Max display height for chat images (Tailwind JIT class) */ -const IMAGE_MAX_H = 'max-h-[45vh]' as const; +export const IMAGE_MAX_H = 'max-h-[45vh]' as const; +/** Matches the `max-w-lg` Tailwind class on the wrapper button (32rem = 512px at 16px base) */ +const IMAGE_MAX_W_PX = 512; + +/** Caches image dimensions by src so remounts can reserve space */ +const dimensionCache = new Map(); +/** Tracks URLs that have been fully painted — skip skeleton on remount */ +const paintedUrls = new Set(); + +/** Test-only: resets module-level caches */ +export function _resetImageCaches(): void { + dimensionCache.clear(); + paintedUrls.clear(); +} + +function computeHeightStyle(w: number, h: number): React.CSSProperties { + return { height: `min(45vh, ${(h / w) * 100}vw, ${(h / w) * IMAGE_MAX_W_PX}px)` }; +} const Image = ({ imagePath, altText, className, args, + width, + height, }: { imagePath: string; altText: string; @@ -24,18 +42,15 @@ const Image = ({ style?: string; [key: string]: unknown; }; + width?: number; + height?: number; }) => { const [isOpen, setIsOpen] = useState(false); - const [isLoaded, setIsLoaded] = useState(false); const triggerRef = useRef(null); - const handleImageLoad = () => setIsLoaded(true); - - // Fix image path to include base path for subdirectory deployments const absoluteImageUrl = useMemo(() => { if (!imagePath) return imagePath; - // If it's already an absolute URL or doesn't start with /images/, return as is if ( imagePath.startsWith('http') || imagePath.startsWith('data:') || @@ -44,7 +59,6 @@ const Image = ({ return imagePath; } - // Get the base URL and prepend it to the image path const baseURL = apiBaseUrl(); return `${baseURL}${imagePath}`; }, [imagePath]); @@ -78,6 +92,17 @@ const Image = ({ } }; + useEffect(() => { + if (width && height && absoluteImageUrl) { + dimensionCache.set(absoluteImageUrl, { width, height }); + } + }, [absoluteImageUrl, width, height]); + + const dims = width && height ? { width, height } : dimensionCache.get(absoluteImageUrl); + const hasDimensions = !!(dims?.width && dims?.height); + const heightStyle = hasDimensions ? computeHeightStyle(dims.width, dims.height) : undefined; + const showSkeleton = hasDimensions && !paintedUrls.has(absoluteImageUrl); + return (
- {isLoaded && ( - - )} +
); }; diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 55946d64d5..7bce7ac11d 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -11,6 +11,7 @@ import type { TMessageContentParts, TAttachment } from 'librechat-data-provider' import { OpenAIImageGen, EmptyText, Reasoning, ExecuteCode, AgentUpdate, Text } from './Parts'; import { ErrorMessage } from './MessageContent'; import RetrievalCall from './RetrievalCall'; +import { getCachedPreview } from '~/utils'; import AgentHandoff from './AgentHandoff'; import CodeAnalyze from './CodeAnalyze'; import Container from './Container'; @@ -222,8 +223,14 @@ const Part = memo(function Part({ } } else if (part.type === ContentTypes.IMAGE_FILE) { const imageFile = part[ContentTypes.IMAGE_FILE]; + const cached = imageFile.file_id ? getCachedPreview(imageFile.file_id) : undefined; return ( - + ); } diff --git a/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx b/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx index 132bac51ea..31e30772dc 100644 --- a/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/Attachment.tsx @@ -52,7 +52,7 @@ const FileAttachment = memo(({ attachment }: { attachment: Partial const ImageAttachment = memo(({ attachment }: { attachment: TAttachment }) => { const [isLoaded, setIsLoaded] = useState(false); - const { filepath = null } = attachment as TFile & TAttachmentMetadata; + const { width, height, filepath = null } = attachment as TFile & TAttachmentMetadata; useEffect(() => { setIsLoaded(false); @@ -76,6 +76,8 @@ const ImageAttachment = memo(({ attachment }: { attachment: TAttachment }) => { diff --git a/client/src/components/Chat/Messages/Content/Parts/EmptyText.tsx b/client/src/components/Chat/Messages/Content/Parts/EmptyText.tsx index 1b514164df..409461a058 100644 --- a/client/src/components/Chat/Messages/Content/Parts/EmptyText.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/EmptyText.tsx @@ -1,8 +1,9 @@ import { memo } from 'react'; +/** Streaming cursor placeholder — no bottom margin to match Container's structure and prevent CLS */ const EmptyTextPart = memo(() => { return ( -
+

diff --git a/client/src/components/Chat/Messages/Content/Parts/LogContent.tsx b/client/src/components/Chat/Messages/Content/Parts/LogContent.tsx index ce6750b048..a675ff06d8 100644 --- a/client/src/components/Chat/Messages/Content/Parts/LogContent.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/LogContent.tsx @@ -94,6 +94,8 @@ const LogContent: React.FC = ({ output = '', renderImages, atta )} {imageAttachments?.map((attachment) => ( { if (isSubmitting) { @@ -120,9 +125,11 @@ export default function OpenAIImageGen({

{progress < 1 && }
diff --git a/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx index 3654d8e075..e7e0b99f1e 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/Image.test.tsx @@ -1,12 +1,12 @@ import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; -import Image from '../Image'; +import Image, { _resetImageCaches } from '../Image'; jest.mock('~/utils', () => ({ - cn: (...classes: unknown[]) => + cn: (...classes: (string | boolean | undefined | null)[]) => classes .flat(Infinity) - .filter((c) => typeof c === 'string' && c.length > 0) + .filter((c): c is string => typeof c === 'string' && c.length > 0) .join(' '), })); @@ -14,38 +14,6 @@ jest.mock('librechat-data-provider', () => ({ apiBaseUrl: () => '', })); -jest.mock('react-lazy-load-image-component', () => ({ - LazyLoadImage: ({ - alt, - src, - className, - onLoad, - placeholder, - visibleByDefault: _visibleByDefault, - ...rest - }: { - alt: string; - src: string; - className: string; - onLoad: () => void; - placeholder: React.ReactNode; - visibleByDefault?: boolean; - [key: string]: unknown; - }) => ( -
- {alt} -
{placeholder}
-
- ), -})); - jest.mock('@librechat/client', () => ({ Skeleton: ({ className, ...props }: React.HTMLAttributes) => (
@@ -65,45 +33,84 @@ describe('Image', () => { }; beforeEach(() => { + _resetImageCaches(); jest.clearAllMocks(); }); - describe('rendering', () => { + describe('rendering without dimensions', () => { it('renders with max-h-[45vh] height constraint', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img.className).toContain('max-h-[45vh]'); }); it('renders with max-w-full to prevent landscape clipping', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img.className).toContain('max-w-full'); }); it('renders with w-auto and h-auto for natural aspect ratio', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img.className).toContain('w-auto'); expect(img.className).toContain('h-auto'); }); - it('starts with opacity-0 before image loads', () => { + it('does not show skeleton without dimensions', () => { render(); - const img = screen.getByTestId('lazy-image'); - expect(img.className).toContain('opacity-0'); - expect(img.className).not.toContain('opacity-100'); + expect(screen.queryByTestId('skeleton')).not.toBeInTheDocument(); }); - it('transitions to opacity-100 after image loads', () => { + it('does not apply heightStyle without dimensions', () => { render(); - const img = screen.getByTestId('lazy-image'); + const button = screen.getByRole('button'); + expect(button.style.height).toBeFalsy(); + }); + }); + + describe('rendering with dimensions', () => { + it('shows skeleton behind image', () => { + render(); + expect(screen.getByTestId('skeleton')).toBeInTheDocument(); + }); + + it('applies computed heightStyle to button', () => { + render(); + const button = screen.getByRole('button'); + expect(button.style.height).toBeTruthy(); + expect(button.style.height).toContain('min(45vh'); + }); + + it('uses size-full object-contain on image when dimensions provided', () => { + render(); + const img = screen.getByRole('img'); + expect(img.className).toContain('size-full'); + expect(img.className).toContain('object-contain'); + }); + + it('skeleton is absolute inset-0', () => { + render(); + const skeleton = screen.getByTestId('skeleton'); + expect(skeleton.className).toContain('absolute'); + expect(skeleton.className).toContain('inset-0'); + }); + + it('marks URL as painted on load and skips skeleton on rerender', () => { + const { rerender } = render(); + const img = screen.getByRole('img'); + + expect(screen.getByTestId('skeleton')).toBeInTheDocument(); fireEvent.load(img); - expect(img.className).toContain('opacity-100'); + // Rerender same component — skeleton should not show (URL painted) + rerender(); + expect(screen.queryByTestId('skeleton')).not.toBeInTheDocument(); }); + }); + describe('common behavior', () => { it('applies custom className to the button wrapper', () => { render(); const button = screen.getByRole('button'); @@ -112,57 +119,9 @@ describe('Image', () => { it('sets correct alt text', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img).toHaveAttribute('alt', 'Test image'); }); - }); - - describe('skeleton placeholder', () => { - it('renders skeleton with non-zero dimensions', () => { - render(); - const skeleton = screen.getByTestId('skeleton'); - expect(skeleton.className).toContain('h-48'); - expect(skeleton.className).toContain('w-full'); - expect(skeleton.className).toContain('max-w-lg'); - }); - - it('renders skeleton with max-h constraint', () => { - render(); - const skeleton = screen.getByTestId('skeleton'); - expect(skeleton.className).toContain('max-h-[45vh]'); - }); - - it('has accessible loading attributes', () => { - render(); - const skeleton = screen.getByTestId('skeleton'); - expect(skeleton).toHaveAttribute('aria-label', 'Loading image'); - expect(skeleton).toHaveAttribute('aria-busy', 'true'); - }); - }); - - describe('dialog interaction', () => { - it('opens dialog on button click after image loads', () => { - render(); - - const img = screen.getByTestId('lazy-image'); - fireEvent.load(img); - - expect(screen.queryByTestId('dialog-image')).not.toBeInTheDocument(); - - const button = screen.getByRole('button'); - fireEvent.click(button); - - expect(screen.getByTestId('dialog-image')).toBeInTheDocument(); - }); - - it('does not render dialog before image loads', () => { - render(); - - const button = screen.getByRole('button'); - fireEvent.click(button); - - expect(screen.queryByTestId('dialog-image')).not.toBeInTheDocument(); - }); it('has correct accessibility attributes on button', () => { render(); @@ -172,28 +131,48 @@ describe('Image', () => { }); }); + describe('dialog interaction', () => { + it('opens dialog on button click', () => { + render(); + expect(screen.queryByTestId('dialog-image')).not.toBeInTheDocument(); + + const button = screen.getByRole('button'); + fireEvent.click(button); + + expect(screen.getByTestId('dialog-image')).toBeInTheDocument(); + }); + + it('dialog is always mounted (not gated by load state)', () => { + render(); + // DialogImage mock returns null when isOpen=false, but the component is in the tree + // Clicking should immediately show it + fireEvent.click(screen.getByRole('button')); + expect(screen.getByTestId('dialog-image')).toBeInTheDocument(); + }); + }); + describe('image URL resolution', () => { it('passes /images/ paths through with base URL', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img).toHaveAttribute('src', '/images/test.png'); }); it('passes absolute http URLs through unchanged', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img).toHaveAttribute('src', 'https://example.com/photo.jpg'); }); it('passes data URIs through unchanged', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img).toHaveAttribute('src', 'data:image/png;base64,abc'); }); it('passes non-/images/ paths through unchanged', () => { render(); - const img = screen.getByTestId('lazy-image'); + const img = screen.getByRole('img'); expect(img).toHaveAttribute('src', '/other/path.png'); }); }); diff --git a/client/src/components/Chat/Messages/Content/__tests__/OpenAIImageGen.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/OpenAIImageGen.test.tsx index 886b1b6294..ef8ac2807a 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/OpenAIImageGen.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/OpenAIImageGen.test.tsx @@ -3,10 +3,10 @@ import { render, screen } from '@testing-library/react'; import OpenAIImageGen from '../Parts/OpenAIImageGen/OpenAIImageGen'; jest.mock('~/utils', () => ({ - cn: (...classes: unknown[]) => + cn: (...classes: (string | boolean | undefined | null)[]) => classes .flat(Infinity) - .filter((c) => typeof c === 'string' && c.length > 0) + .filter((c): c is string => typeof c === 'string' && c.length > 0) .join(' '), })); diff --git a/client/src/components/Chat/Messages/MessageParts.tsx b/client/src/components/Chat/Messages/MessageParts.tsx index 88162f3287..7aa73a54e6 100644 --- a/client/src/components/Chat/Messages/MessageParts.tsx +++ b/client/src/components/Chat/Messages/MessageParts.tsx @@ -129,7 +129,7 @@ export default function Message(props: TMessageProps) { )}
-
+
{isLast && isSubmitting ? ( -
+
) : ( -
+
; + return
; }); PlaceholderRow.displayName = 'PlaceholderRow'; diff --git a/client/src/components/Endpoints/Icon.tsx b/client/src/components/Endpoints/Icon.tsx index 3256145bfb..fae0f286d3 100644 --- a/client/src/components/Endpoints/Icon.tsx +++ b/client/src/components/Endpoints/Icon.tsx @@ -1,64 +1,102 @@ -import React, { memo, useState } from 'react'; +import React, { memo } from 'react'; import { UserIcon, useAvatar } from '@librechat/client'; -import type { TUser } from 'librechat-data-provider'; import type { IconProps } from '~/common'; import MessageEndpointIcon from './MessageEndpointIcon'; import { useAuthContext } from '~/hooks/AuthContext'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; +type ResolvedAvatar = { type: 'image'; src: string } | { type: 'fallback' }; + +/** + * Caches the resolved avatar decision per user ID. + * Invalidated when `user.avatar` changes (e.g., settings upload). + * Tracks failed image URLs so they fall back to SVG permanently for the session. + */ +const avatarCache = new Map< + string, + { avatar: string; avatarSrc: string; resolved: ResolvedAvatar } +>(); +const failedUrls = new Set(); + +function resolveAvatar(userId: string, userAvatar: string, avatarSrc: string): ResolvedAvatar { + if (!userId) { + const imgSrc = userAvatar || avatarSrc; + return imgSrc && !failedUrls.has(imgSrc) + ? { type: 'image', src: imgSrc } + : { type: 'fallback' }; + } + + const cached = avatarCache.get(userId); + if (cached && cached.avatar === userAvatar && cached.avatarSrc === avatarSrc) { + return cached.resolved; + } + + const imgSrc = userAvatar || avatarSrc; + const resolved: ResolvedAvatar = + imgSrc && !failedUrls.has(imgSrc) ? { type: 'image', src: imgSrc } : { type: 'fallback' }; + + avatarCache.set(userId, { avatar: userAvatar, avatarSrc, resolved }); + return resolved; +} + +function markAvatarFailed(userId: string, src: string): ResolvedAvatar { + failedUrls.add(src); + const fallback: ResolvedAvatar = { type: 'fallback' }; + const cached = avatarCache.get(userId); + if (cached) { + avatarCache.set(userId, { ...cached, resolved: fallback }); + } + return fallback; +} + type UserAvatarProps = { size: number; - user?: TUser; + avatar: string; avatarSrc: string; + userId: string; username: string; className?: string; }; -const UserAvatar = memo(({ size, user, avatarSrc, username, className }: UserAvatarProps) => { - const [imageError, setImageError] = useState(false); +const UserAvatar = memo( + ({ size, avatar, avatarSrc, userId, username, className }: UserAvatarProps) => { + const [resolved, setResolved] = React.useState(() => resolveAvatar(userId, avatar, avatarSrc)); - const handleImageError = () => { - setImageError(true); - }; + React.useEffect(() => { + setResolved(resolveAvatar(userId, avatar, avatarSrc)); + }, [userId, avatar, avatarSrc]); - const renderDefaultAvatar = () => ( -
- -
- ); - - return ( -
- {(!(user?.avatar ?? '') && (!(user?.username ?? '') || user?.username.trim() === '')) || - imageError ? ( - renderDefaultAvatar() - ) : ( - avatar - )} -
- ); -}); + return ( +
+ {resolved.type === 'image' ? ( + avatar setResolved(markAvatarFailed(userId, resolved.src))} + /> + ) : ( +
+ +
+ )} +
+ ); + }, +); UserAvatar.displayName = 'UserAvatar'; @@ -74,9 +112,10 @@ const Icon: React.FC = memo((props) => { return ( ); diff --git a/client/src/components/Messages/ContentRender.tsx b/client/src/components/Messages/ContentRender.tsx index a50b91c071..4114baefe4 100644 --- a/client/src/components/Messages/ContentRender.tsx +++ b/client/src/components/Messages/ContentRender.tsx @@ -144,7 +144,7 @@ const ContentRender = memo(function ContentRender({ )}
-
+
{messageLabel}
-
+
({ })); jest.mock('~/hooks/useLocalize', () => { - const fn = jest.fn((key: string) => key); + const fn = jest.fn((key: string) => key) as jest.Mock & { + TranslationKeys: Record; + }; fn.TranslationKeys = {}; return { __esModule: true, default: fn, TranslationKeys: {} }; }); @@ -87,6 +89,8 @@ jest.mock('../useUpdateFiles', () => ({ jest.mock('~/utils', () => ({ logger: { log: jest.fn() }, validateFiles: jest.fn(() => true), + cachePreview: jest.fn(), + getCachedPreview: jest.fn(() => undefined), })); const mockValidateFiles = jest.requireMock('~/utils').validateFiles; @@ -263,7 +267,7 @@ describe('useFileHandling', () => { it('falls back to "default" when no conversation endpoint and no override', async () => { mockConversation = { - conversationId: Constants.NEW_CONVO, + conversationId: Constants.NEW_CONVO as string, endpoint: null, endpointType: undefined, }; diff --git a/client/src/hooks/Files/useFileDeletion.ts b/client/src/hooks/Files/useFileDeletion.ts index 34d89e313b..c33ac2a50b 100644 --- a/client/src/hooks/Files/useFileDeletion.ts +++ b/client/src/hooks/Files/useFileDeletion.ts @@ -5,6 +5,7 @@ import type * as t from 'librechat-data-provider'; import type { UseMutateAsyncFunction } from '@tanstack/react-query'; import type { ExtendedFile, GenericSetter } from '~/common'; import useSetFilesToDelete from './useSetFilesToDelete'; +import { deletePreview } from '~/utils'; type FileMapSetter = GenericSetter>; @@ -88,6 +89,11 @@ const useFileDeletion = ({ }); } + deletePreview(file_id); + if (temp_file_id) { + deletePreview(temp_file_id); + } + if (attached) { return; } @@ -125,6 +131,11 @@ const useFileDeletion = ({ temp_file_id, embedded: embedded ?? false, }); + + deletePreview(file_id); + if (temp_file_id) { + deletePreview(temp_file_id); + } } if (setFiles) { diff --git a/client/src/hooks/Files/useFileHandling.ts b/client/src/hooks/Files/useFileHandling.ts index 68d56e75ad..be62700651 100644 --- a/client/src/hooks/Files/useFileHandling.ts +++ b/client/src/hooks/Files/useFileHandling.ts @@ -16,13 +16,13 @@ import debounce from 'lodash/debounce'; import type { EModelEndpoint, TEndpointsConfig, TError } from 'librechat-data-provider'; import type { ExtendedFile, FileSetter } from '~/common'; import type { TConversation } from 'librechat-data-provider'; +import { logger, validateFiles, cachePreview, getCachedPreview, removePreviewEntry } from '~/utils'; import { useGetFileConfig, useUploadFileMutation } from '~/data-provider'; import useLocalize, { TranslationKeys } from '~/hooks/useLocalize'; import { useDelayedUploadToast } from './useDelayedUploadToast'; import { processFileForUpload } from '~/utils/heicConverter'; import { useChatContext } from '~/Providers/ChatContext'; import { ephemeralAgentByConvoId } from '~/store'; -import { logger, validateFiles } from '~/utils'; import useClientResize from './useClientResize'; import useUpdateFiles from './useUpdateFiles'; @@ -130,6 +130,11 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil ); setTimeout(() => { + const cachedBlob = getCachedPreview(data.temp_file_id); + if (cachedBlob && data.file_id !== data.temp_file_id) { + cachePreview(data.file_id, cachedBlob); + removePreviewEntry(data.temp_file_id); + } updateFileById( data.temp_file_id, { @@ -260,7 +265,6 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil replaceFile(extendedFile); await startUpload(extendedFile); - URL.revokeObjectURL(preview); }; img.src = preview; }; @@ -301,6 +305,7 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil try { // Create initial preview with original file const initialPreview = URL.createObjectURL(originalFile); + cachePreview(file_id, initialPreview); // Create initial ExtendedFile to show immediately const initialExtendedFile: ExtendedFile = { @@ -378,6 +383,7 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil if (finalProcessedFile !== originalFile) { URL.revokeObjectURL(initialPreview); // Clean up original preview const newPreview = URL.createObjectURL(finalProcessedFile); + cachePreview(file_id, newPreview); const updatedExtendedFile: ExtendedFile = { ...initialExtendedFile, diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index 9f809bd6c1..325ee97315 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -526,6 +526,23 @@ export default function useEventHandlers({ } else if (requestMessage != null && responseMessage != null) { finalMessages = [...messages, requestMessage, responseMessage]; } + + /* Preserve files from current messages when server response lacks them */ + if (finalMessages.length > 0) { + const currentMsgMap = new Map( + currentMessages + .filter((m) => m.files && m.files.length > 0) + .map((m) => [m.messageId, m.files]), + ); + for (let i = 0; i < finalMessages.length; i++) { + const msg = finalMessages[i]; + const preservedFiles = currentMsgMap.get(msg.messageId); + if (msg.files == null && preservedFiles) { + finalMessages[i] = { ...msg, files: preservedFiles }; + } + } + } + if (finalMessages.length > 0) { setFinalMessages(conversation.conversationId, finalMessages); } else if ( diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index 2a7dfc4a88..dae075b471 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -24,6 +24,7 @@ export * from './resources'; export * from './roles'; export * from './localStorage'; export * from './promptGroups'; +export * from './previewCache'; export * from './email'; export * from './share'; export * from './timestamps'; diff --git a/client/src/utils/previewCache.ts b/client/src/utils/previewCache.ts new file mode 100644 index 0000000000..604ce56308 --- /dev/null +++ b/client/src/utils/previewCache.ts @@ -0,0 +1,35 @@ +/** + * Module-level cache for local blob preview URLs keyed by file_id. + * Survives message replacements from SSE but clears on page refresh. + */ +const previewCache = new Map(); + +export function cachePreview(fileId: string, previewUrl: string): void { + const existing = previewCache.get(fileId); + if (existing && existing !== previewUrl) { + URL.revokeObjectURL(existing); + } + previewCache.set(fileId, previewUrl); +} + +export function getCachedPreview(fileId: string): string | undefined { + return previewCache.get(fileId); +} + +/** Removes the cache entry without revoking the blob (used when transferring between keys) */ +export function removePreviewEntry(fileId: string): void { + previewCache.delete(fileId); +} + +export function deletePreview(fileId: string): void { + const url = previewCache.get(fileId); + if (url) { + URL.revokeObjectURL(url); + previewCache.delete(fileId); + } +} + +export function clearPreviewCache(): void { + previewCache.forEach((url) => URL.revokeObjectURL(url)); + previewCache.clear(); +} diff --git a/package-lock.json b/package-lock.json index e634f9f053..f382df6b37 100644 --- a/package-lock.json +++ b/package-lock.json @@ -445,7 +445,6 @@ "react-gtm-module": "^2.0.11", "react-hook-form": "^7.43.9", "react-i18next": "^15.4.0", - "react-lazy-load-image-component": "^1.6.0", "react-markdown": "^9.0.1", "react-resizable-panels": "^3.0.6", "react-router-dom": "^6.30.3", @@ -500,6 +499,7 @@ "jest-environment-jsdom": "^30.2.0", "jest-file-loader": "^1.0.3", "jest-junit": "^16.0.0", + "monaco-editor": "^0.55.0", "postcss": "^8.4.31", "postcss-preset-env": "^11.2.0", "tailwindcss": "^3.4.1", @@ -32359,11 +32359,6 @@ "dev": true, "license": "MIT" }, - "node_modules/lodash.throttle": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/lodash.throttle/-/lodash.throttle-4.1.1.tgz", - "integrity": "sha512-wIkUCfVKpVsWo3JSZlc+8MB5it+2AN5W8J7YVMST30UrvcQNZ1Okbj+rbVniijTWE6FGYy4XJq/rHkas8qJMLQ==" - }, "node_modules/lodash.uniq": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.uniq/-/lodash.uniq-4.5.0.tgz", @@ -34312,7 +34307,6 @@ "resolved": "https://registry.npmjs.org/monaco-editor/-/monaco-editor-0.55.1.tgz", "integrity": "sha512-jz4x+TJNFHwHtwuV9vA9rMujcZRb0CEilTEwG2rRSpe/A7Jdkuj8xPKttCgOh+v/lkHy7HsZ64oj+q3xoAFl9A==", "license": "MIT", - "peer": true, "dependencies": { "dompurify": "3.2.7", "marked": "14.0.0" @@ -34323,7 +34317,6 @@ "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.2.7.tgz", "integrity": "sha512-WhL/YuveyGXJaerVlMYGWhvQswa7myDG17P7Vu65EWC05o8vfeNbvNf4d/BOvH99+ZW+LlQsc1GDKMa1vNK6dw==", "license": "(MPL-2.0 OR Apache-2.0)", - "peer": true, "optionalDependencies": { "@types/trusted-types": "^2.0.7" } @@ -34333,7 +34326,6 @@ "resolved": "https://registry.npmjs.org/marked/-/marked-14.0.0.tgz", "integrity": "sha512-uIj4+faQ+MgHgwUW1l2PsPglZLOLOT1uErt06dAPtx2kjteLAkbsd/0FiYg/MGS+i7ZKLb7w2WClxHkzOOuryQ==", "license": "MIT", - "peer": true, "bin": { "marked": "bin/marked.js" }, @@ -38142,19 +38134,6 @@ "integrity": "sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg==", "license": "MIT" }, - "node_modules/react-lazy-load-image-component": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/react-lazy-load-image-component/-/react-lazy-load-image-component-1.6.0.tgz", - "integrity": "sha512-8KFkDTgjh+0+PVbH+cx0AgxLGbdTsxWMnxXzU5HEUztqewk9ufQAu8cstjZhyvtMIPsdMcPZfA0WAa7HtjQbBQ==", - "dependencies": { - "lodash.debounce": "^4.0.8", - "lodash.throttle": "^4.1.1" - }, - "peerDependencies": { - "react": "^15.x.x || ^16.x.x || ^17.x.x || ^18.x.x", - "react-dom": "^15.x.x || ^16.x.x || ^17.x.x || ^18.x.x" - } - }, "node_modules/react-lifecycles-compat": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz",