mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-03 14:27:20 +02:00
🖼️ fix: Message Icon Flickering from Context-Triggered Re-renders (#12489)
* perf: add custom memo comparator to MessageIcon for stable re-render gating MessageIcon receives full `agent` and `assistant` objects as props from useMessageActions, which recomputes them when AgentsMapContext or AssistantsMapContext update (e.g., react-query refetch on window focus). These context-triggered re-renders bypass MessageRender's React.memo, producing new object references for agent/assistant even when the underlying data is unchanged. The default shallow comparison in MessageIcon's memo then fails, causing unnecessary re-renders that manifest as visible icon flickering. Add arePropsEqual comparator that checks only the fields MessageIcon actually uses (name, avatar filepath, metadata avatar) instead of object identity, so the component correctly bails out when icon-relevant data hasn't changed. * refactor: export arePropsEqual, drop redundant useMemos, add JSDoc - Export arePropsEqual so it can be tested in isolation - Add JSDoc documenting which fields are intentionally omitted (id) and why iconData uses reference equality - Replace five trivial useMemo calls (agent?.name ?? '', etc.) with direct computed values — the custom comparator already gates re-renders, so these memos only add closure/dep-array overhead without ever providing cache hits - Remove unused React import * test: add unit tests for MessageIcon arePropsEqual comparator Exercise the custom memo comparator to ensure: - New object references with same display fields are treated as equal - Changed name or avatar filepath triggers re-render - iconData reference change triggers re-render - undefined→defined agent with undefined fields is treated as equal * fix: replace nested ternary with if-else for eslint compliance * test: add comment on subtle equality invariant and defined→undefined case * perf: compare iconData by field values instead of reference iconData is a useMemo'd object from the parent, but comparing by reference still causes unnecessary re-renders when the parent recomputes the memo with identical primitive values. Compare all five fields individually (endpoint, model, iconURL, modelLabel, isCreatedByUser) for consistency with how agent/assistant are handled.
This commit is contained in:
parent
aa7e5ba051
commit
7181174c3b
2 changed files with 239 additions and 57 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useMemo, memo } from 'react';
|
||||
import { useMemo, memo } from 'react';
|
||||
import { getEndpointField } from 'librechat-data-provider';
|
||||
import type { Assistant, Agent } from 'librechat-data-provider';
|
||||
import type { TMessageIcon } from '~/common';
|
||||
|
|
@ -7,73 +7,101 @@ import { useGetEndpointsQuery } from '~/data-provider';
|
|||
import { getIconEndpoint, logger } from '~/utils';
|
||||
import Icon from '~/components/Endpoints/Icon';
|
||||
|
||||
const MessageIcon = memo(
|
||||
({
|
||||
iconData,
|
||||
assistant,
|
||||
agent,
|
||||
}: {
|
||||
iconData?: TMessageIcon;
|
||||
assistant?: Assistant;
|
||||
agent?: Agent;
|
||||
}) => {
|
||||
logger.log('icon_data', iconData, assistant, agent);
|
||||
const { data: endpointsConfig } = useGetEndpointsQuery();
|
||||
type MessageIconProps = {
|
||||
iconData?: TMessageIcon;
|
||||
assistant?: Assistant;
|
||||
agent?: Agent;
|
||||
};
|
||||
|
||||
const agentName = useMemo(() => agent?.name ?? '', [agent]);
|
||||
const agentAvatar = useMemo(() => agent?.avatar?.filepath ?? '', [agent]);
|
||||
const assistantName = useMemo(() => assistant?.name ?? '', [assistant]);
|
||||
const assistantAvatar = useMemo(() => assistant?.metadata?.avatar ?? '', [assistant]);
|
||||
/**
|
||||
* Compares only the fields MessageIcon actually renders.
|
||||
* `agent.id` / `assistant.id` are intentionally omitted because
|
||||
* this component renders display properties only, not identity-derived content.
|
||||
*/
|
||||
export function arePropsEqual(prev: MessageIconProps, next: MessageIconProps): boolean {
|
||||
if (prev.iconData?.endpoint !== next.iconData?.endpoint) {
|
||||
return false;
|
||||
}
|
||||
if (prev.iconData?.model !== next.iconData?.model) {
|
||||
return false;
|
||||
}
|
||||
if (prev.iconData?.iconURL !== next.iconData?.iconURL) {
|
||||
return false;
|
||||
}
|
||||
if (prev.iconData?.modelLabel !== next.iconData?.modelLabel) {
|
||||
return false;
|
||||
}
|
||||
if (prev.iconData?.isCreatedByUser !== next.iconData?.isCreatedByUser) {
|
||||
return false;
|
||||
}
|
||||
if (prev.agent?.name !== next.agent?.name) {
|
||||
return false;
|
||||
}
|
||||
if (prev.agent?.avatar?.filepath !== next.agent?.avatar?.filepath) {
|
||||
return false;
|
||||
}
|
||||
if (prev.assistant?.name !== next.assistant?.name) {
|
||||
return false;
|
||||
}
|
||||
if (prev.assistant?.metadata?.avatar !== next.assistant?.metadata?.avatar) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
const avatarURL = useMemo(() => {
|
||||
let result = '';
|
||||
if (assistant) {
|
||||
result = assistantAvatar;
|
||||
} else if (agent) {
|
||||
result = agentAvatar;
|
||||
}
|
||||
return result;
|
||||
}, [assistant, agent, assistantAvatar, agentAvatar]);
|
||||
const MessageIcon = memo(({ iconData, assistant, agent }: MessageIconProps) => {
|
||||
logger.log('icon_data', iconData, assistant, agent);
|
||||
const { data: endpointsConfig } = useGetEndpointsQuery();
|
||||
|
||||
const iconURL = iconData?.iconURL;
|
||||
const endpoint = useMemo(
|
||||
() => getIconEndpoint({ endpointsConfig, iconURL, endpoint: iconData?.endpoint }),
|
||||
[endpointsConfig, iconURL, iconData?.endpoint],
|
||||
);
|
||||
const agentName = agent?.name ?? '';
|
||||
const agentAvatar = agent?.avatar?.filepath ?? '';
|
||||
const assistantName = assistant?.name ?? '';
|
||||
const assistantAvatar = assistant?.metadata?.avatar ?? '';
|
||||
let avatarURL = '';
|
||||
if (assistant) {
|
||||
avatarURL = assistantAvatar;
|
||||
} else if (agent) {
|
||||
avatarURL = agentAvatar;
|
||||
}
|
||||
|
||||
const endpointIconURL = useMemo(
|
||||
() => getEndpointField(endpointsConfig, endpoint, 'iconURL'),
|
||||
[endpointsConfig, endpoint],
|
||||
);
|
||||
const iconURL = iconData?.iconURL;
|
||||
const endpoint = useMemo(
|
||||
() => getIconEndpoint({ endpointsConfig, iconURL, endpoint: iconData?.endpoint }),
|
||||
[endpointsConfig, iconURL, iconData?.endpoint],
|
||||
);
|
||||
|
||||
if (iconData?.isCreatedByUser !== true && iconURL != null && iconURL.includes('http')) {
|
||||
return (
|
||||
<ConvoIconURL
|
||||
iconURL={iconURL}
|
||||
modelLabel={iconData?.modelLabel}
|
||||
context="message"
|
||||
assistantAvatar={assistantAvatar}
|
||||
agentAvatar={agentAvatar}
|
||||
endpointIconURL={endpointIconURL}
|
||||
assistantName={assistantName}
|
||||
agentName={agentName}
|
||||
/>
|
||||
);
|
||||
}
|
||||
const endpointIconURL = useMemo(
|
||||
() => getEndpointField(endpointsConfig, endpoint, 'iconURL'),
|
||||
[endpointsConfig, endpoint],
|
||||
);
|
||||
|
||||
if (iconData?.isCreatedByUser !== true && iconURL != null && iconURL.includes('http')) {
|
||||
return (
|
||||
<Icon
|
||||
isCreatedByUser={iconData?.isCreatedByUser ?? false}
|
||||
endpoint={endpoint}
|
||||
iconURL={avatarURL || endpointIconURL}
|
||||
model={iconData?.model}
|
||||
<ConvoIconURL
|
||||
iconURL={iconURL}
|
||||
modelLabel={iconData?.modelLabel}
|
||||
context="message"
|
||||
assistantAvatar={assistantAvatar}
|
||||
agentAvatar={agentAvatar}
|
||||
endpointIconURL={endpointIconURL}
|
||||
assistantName={assistantName}
|
||||
agentName={agentName}
|
||||
size={28.8}
|
||||
/>
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<Icon
|
||||
isCreatedByUser={iconData?.isCreatedByUser ?? false}
|
||||
endpoint={endpoint}
|
||||
iconURL={avatarURL || endpointIconURL}
|
||||
model={iconData?.model}
|
||||
assistantName={assistantName}
|
||||
agentName={agentName}
|
||||
size={28.8}
|
||||
/>
|
||||
);
|
||||
}, arePropsEqual);
|
||||
|
||||
MessageIcon.displayName = 'MessageIcon';
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,154 @@
|
|||
import type { Agent, Assistant } from 'librechat-data-provider';
|
||||
import type { TMessageIcon } from '~/common';
|
||||
|
||||
// Mock all module-level imports so we can import the pure arePropsEqual function
|
||||
// without pulling in React component dependencies
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
getEndpointField: jest.fn(),
|
||||
}));
|
||||
jest.mock('~/components/Endpoints/ConvoIconURL', () => jest.fn());
|
||||
jest.mock('~/data-provider', () => ({ useGetEndpointsQuery: jest.fn(() => ({ data: {} })) }));
|
||||
jest.mock('~/utils', () => ({ getIconEndpoint: jest.fn(), logger: { log: jest.fn() } }));
|
||||
jest.mock('~/components/Endpoints/Icon', () => jest.fn());
|
||||
|
||||
import { arePropsEqual } from '../MessageIcon';
|
||||
|
||||
const baseIconData: TMessageIcon = {
|
||||
endpoint: 'agents',
|
||||
model: 'agent_123',
|
||||
iconURL: '/images/avatar.png',
|
||||
modelLabel: 'Test Agent',
|
||||
isCreatedByUser: false,
|
||||
};
|
||||
|
||||
const makeAgent = (overrides?: Partial<Agent>): Agent =>
|
||||
({
|
||||
id: 'agent_123',
|
||||
name: 'Atlas',
|
||||
avatar: { filepath: '/avatars/atlas.png' },
|
||||
...overrides,
|
||||
}) as Agent;
|
||||
|
||||
const makeAssistant = (overrides?: Partial<Assistant>): Assistant =>
|
||||
({
|
||||
id: 'asst_123',
|
||||
name: 'Helper',
|
||||
metadata: { avatar: '/avatars/helper.png' },
|
||||
...overrides,
|
||||
}) as Assistant;
|
||||
|
||||
describe('MessageIcon arePropsEqual', () => {
|
||||
it('returns true when agent reference changes but display fields are identical', () => {
|
||||
const agent1 = makeAgent();
|
||||
const agent2 = makeAgent();
|
||||
expect(agent1).not.toBe(agent2);
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, agent: agent1 },
|
||||
{ iconData: baseIconData, agent: agent2 },
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when agent name changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, agent: makeAgent({ name: 'Atlas' }) },
|
||||
{ iconData: baseIconData, agent: makeAgent({ name: 'Hermes' }) },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when agent avatar filepath changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, agent: makeAgent({ avatar: { filepath: '/a.png' } }) },
|
||||
{ iconData: baseIconData, agent: makeAgent({ avatar: { filepath: '/b.png' } }) },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when assistant reference changes but display fields are identical', () => {
|
||||
const asst1 = makeAssistant();
|
||||
const asst2 = makeAssistant();
|
||||
expect(asst1).not.toBe(asst2);
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, assistant: asst1 },
|
||||
{ iconData: baseIconData, assistant: asst2 },
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when assistant name changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, assistant: makeAssistant({ name: 'Helper' }) },
|
||||
{ iconData: baseIconData, assistant: makeAssistant({ name: 'Wizard' }) },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when assistant avatar changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, assistant: makeAssistant({ metadata: { avatar: '/a.png' } }) },
|
||||
{ iconData: baseIconData, assistant: makeAssistant({ metadata: { avatar: '/b.png' } }) },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when iconData reference changes but fields are identical', () => {
|
||||
const iconData1 = { ...baseIconData };
|
||||
const iconData2 = { ...baseIconData };
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: iconData1, agent: makeAgent() },
|
||||
{ iconData: iconData2, agent: makeAgent() },
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when iconData endpoint changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: { ...baseIconData, endpoint: 'agents' } },
|
||||
{ iconData: { ...baseIconData, endpoint: 'openAI' } },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when iconData iconURL changes', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: { ...baseIconData, iconURL: '/a.png' } },
|
||||
{ iconData: { ...baseIconData, iconURL: '/b.png' } },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when both agent and assistant are undefined', () => {
|
||||
expect(arePropsEqual({ iconData: baseIconData }, { iconData: baseIconData })).toBe(true);
|
||||
});
|
||||
|
||||
// avatarURL and display strings both remain '' in both states — nothing renders differently,
|
||||
// so suppressing the re-render is correct even though the agent prop went from undefined to defined.
|
||||
it('returns true when agent transitions from undefined to object with undefined display fields', () => {
|
||||
const agentNoFields = makeAgent({ name: undefined, avatar: undefined });
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, agent: undefined },
|
||||
{ iconData: baseIconData, agent: agentNoFields },
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when agent transitions from defined to undefined', () => {
|
||||
expect(
|
||||
arePropsEqual(
|
||||
{ iconData: baseIconData, agent: makeAgent() },
|
||||
{ iconData: baseIconData, agent: undefined },
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue