Update the pinned message banner when a pinned message is edited (#34631)

* Update the pinned message banner when a pinned message is edited

The pinned events are fetched from a memo keyed on the pinned event ids, and editing a
pinned message leaves those ids untouched. Nothing invalidated the memo, so the banner
and the pinned messages card went on rendering the original text while the timeline
showed the edit.

Edits landing on a pinned event now invalidate it. An edit to any other event is
ignored, so an active room does not refetch the pinned set on every message.

* Address review: apply pinned edits in place instead of refetching

The counter that forced the memo to recompute was never read by the memo, so the
dependency array had to carry a variable that meant nothing to the computation.
The edit event is already in hand when the timeline fires, so holding onto it
rather than a tally gives the memo something it can actually consume.

Applying that edit to the fetched copy also removes a round-trip: the pinned set
was being fetched again purely to pick up content the client already had. It has
to run in an effect rather than in the memo because replacing an event notifies
whatever is rendering it, and that must not happen during a render.

The tests now assert the content the hook hands back rather than the number of
fetches, which is what the banner actually shows.
This commit is contained in:
hayyaksi
2026-08-11 16:38:35 +00:00
committed by GitHub
parent 91e8f3bdae
commit 400bf348ef
2 changed files with 124 additions and 2 deletions
+40 -2
View File
@@ -16,6 +16,7 @@ import {
RelationType,
EventTimeline,
type MatrixClient,
type IRoomTimelineData,
} from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
@@ -180,7 +181,33 @@ async function fetchPinnedEvent(room: Room, pinnedEventId: string, cli: MatrixCl
export function useFetchedPinnedEvents(room: Room, pinnedEventIds: string[]): Array<MatrixEvent> {
const cli = useMatrixClientContext();
const events = useAsyncMemo(
// Editing a pinned message leaves the pinned ids untouched, and a pinned event fetched from
// the server only carries the edits that existed when it was fetched, so a later edit would go
// unnoticed. Collect the edits landing on a pinned event, keyed by the event they replace.
const [edits, setEdits] = useState(new Map<string, MatrixEvent>());
useTypedEventEmitter(
room,
RoomEvent.Timeline,
(
event: MatrixEvent,
_room: Room | undefined,
_toStartOfTimeline: boolean | undefined,
removed: boolean,
data: IRoomTimelineData,
): void => {
// A backfilled edit can be older than the one already applied, so only live ones count.
if (removed || !data.liveEvent) return;
const relation = event.getRelation();
if (relation?.rel_type !== RelationType.Replace) return;
const editedEventId = relation.event_id;
if (!editedEventId || !pinnedEventIds.includes(editedEventId)) return;
setEdits((edits) => new Map(edits).set(editedEventId, event));
},
);
const fetchedEvents = useAsyncMemo(
() => {
const fetchPromises = pinnedEventIds.map((eventId) => () => fetchPinnedEvent(room, eventId, cli));
// Fetch the pinned events in batches of 10
@@ -189,7 +216,18 @@ export function useFetchedPinnedEvents(room: Room, pinnedEventIds: string[]): Ar
[cli, room, pinnedEventIds],
[],
);
return filterBoolean(events);
const events = useMemo(() => filterBoolean(fetchedEvents), [fetchedEvents]);
// Replacing an event notifies whatever is rendering it, which is what refreshes the banner and
// the pinned messages card, so it has to happen after the render rather than during it.
useEffect(() => {
for (const event of events) {
const edit = edits.get(event.getId()!);
if (edit) event.makeReplaced(edit);
}
}, [events, edits]);
return events;
}
/**
@@ -0,0 +1,84 @@
/*
Copyright 2026 hayaksi1
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE files in the repository root for full details.
*/
import React, { type PropsWithChildren } from "react";
import { act, renderHook, waitFor } from "jest-matrix-react";
import { EventType, type MatrixClient, MatrixEvent, RelationType, Room } from "matrix-js-sdk/src/matrix";
import { useFetchedPinnedEvents } from "../../../src/hooks/usePinnedEvents";
import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
import { stubClient } from "../../test-utils";
describe("useFetchedPinnedEvents", () => {
const roomId = "!room:server";
const userId = "@alice:server";
const pinnedEventId = "$pinned:server";
let client: MatrixClient;
let room: Room;
// Stable identity: the hook memoises on this array, so a fresh literal per render would
// refetch forever.
const pinnedIds = [pinnedEventId];
const makeEvent = (id: string, content: object): MatrixEvent =>
new MatrixEvent({
type: EventType.RoomMessage,
sender: userId,
room_id: roomId,
event_id: id,
origin_server_ts: 0,
content,
});
const edit = (targetId: string, id: string): MatrixEvent =>
makeEvent(id, {
"msgtype": "m.text",
"body": "* edited",
"m.new_content": { msgtype: "m.text", body: "edited" },
"m.relates_to": { rel_type: RelationType.Replace, event_id: targetId },
});
const wrapper = ({ children }: PropsWithChildren): React.JSX.Element => (
<MatrixClientContext.Provider value={client}>{children}</MatrixClientContext.Provider>
);
beforeEach(() => {
client = stubClient();
room = new Room(roomId, client, userId);
// The pinned event is deliberately not in the local timeline, so the hook holds a copy of
// its own that nothing else keeps up to date.
jest.spyOn(client, "fetchRoomEvent").mockResolvedValue(
makeEvent(pinnedEventId, { msgtype: "m.text", body: "original" }).event as never,
);
jest.spyOn(client, "relations").mockResolvedValue({ events: [] });
});
it("gives the edited content for a pinned event that is edited", async () => {
const { result } = renderHook(() => useFetchedPinnedEvents(room, pinnedIds), { wrapper });
await waitFor(() => expect(result.current).toHaveLength(1));
expect(result.current[0].getContent().body).toBe("original");
act(() => {
room.addLiveEvents([edit(pinnedEventId, "$edit:server")], { addToState: false });
});
await waitFor(() => expect(result.current[0].getContent().body).toBe("edited"));
});
it("leaves a pinned event alone when a different event is edited", async () => {
const { result } = renderHook(() => useFetchedPinnedEvents(room, pinnedIds), { wrapper });
await waitFor(() => expect(result.current).toHaveLength(1));
act(() => {
room.addLiveEvents([edit("$somethingElse:server", "$edit2:server")], { addToState: false });
});
// Wait long enough that an edit would have been applied, then confirm none was.
await new Promise((resolve) => setTimeout(resolve, 50));
expect(result.current[0].getContent().body).toBe("original");
});
});