Skip to content

Fix embeds intermittently lost until page refresh #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RobbyV2 wants to merge 1 commit into from
Closed

Fix embeds intermittently lost until page refresh #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions app/lib/__tests__/store_reducer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,21 @@ describe('media / embeds patching via patchMessage', () => {
expect(st().messages.c1[0].embeds.map(e => e.ord)).toEqual([0])
})

test('embeds_resolved before the message is inserted still lands on the later insert', () => {
useStore.setState({ messages: { c1: [msg(10)] } })
dispatch({ type: 'embeds_resolved', ...scoped, message_id: 80, embeds: [embed(0)] })
dispatch({ type: 'message', ...scoped, message: msg(80, { channel_id: 1 }) })
const stored = st().messages.c1.find(m => m.id === 80)
expect(stored?.embeds.map(e => e.ord)).toEqual([0])
})

test('a later duplicate message echo does not clobber resolved embeds', () => {
useStore.setState({ messages: { c1: [msg(80, { channel_id: 1 })] } })
dispatch({ type: 'embeds_resolved', ...scoped, message_id: 80, embeds: [embed(0)] })
dispatch({ type: 'message', ...scoped, message: msg(80, { channel_id: 1, embeds: [] }) })
expect(st().messages.c1[0].embeds.map(e => e.ord)).toEqual([0])
})

test('embeds_removed banner marks banner_removed; full removes the embed', () => {
useStore.setState({ messages: { c1: [msg(90, { embeds: [embed(0), embed(1)] })] } })
dispatch({ type: 'embeds_removed', ...scoped, message_id: 90, ord: 0, banner: true })
Expand Down
37 changes: 30 additions & 7 deletions app/lib/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,22 @@ const OPT_BASE = 1e15
let optSeq = 0
const draining = new Set<string>()

const pendingEmbeds = new Map<number, Embed[]>()
const PENDING_EMBEDS_MAX = 256
const rememberEmbeds = (id: number, embeds: Embed[]) => {
pendingEmbeds.delete(id)
pendingEmbeds.set(id, embeds)
if (pendingEmbeds.size > PENDING_EMBEDS_MAX)
pendingEmbeds.delete(pendingEmbeds.keys().next().value as number)
}
const applyEmbeds = (list: Message[]) =>
pendingEmbeds.size === 0
? list
: list.map(m => {
const e = pendingEmbeds.get(m.id)
return e ? { ...m, embeds: e } : m
})

interface RChatState {
token: string | null
me: Me | null
Expand Down Expand Up @@ -417,7 +433,7 @@ export const useStore = create<RChatState>()((set, get) => {
view.kind === 'channel'
? await api.channelMessages(view.channelId)
: await api.dmMessages(view.dmId)
set(s => ({ messages: { ...s.messages, [viewKey(view)]: [...msgs].sort(byId) } }))
set(s => ({ messages: { ...s.messages, [viewKey(view)]: applyEmbeds([...msgs].sort(byId)) } }))
askP2p(msgs)
if (view.kind === 'channel') {
const detail = await api.getServer(view.server)
Expand Down Expand Up @@ -807,7 +823,7 @@ export const useStore = create<RChatState>()((set, get) => {
}
set(s => ({
outbox: dropOutgoing(s.outbox, key, head.tempId),
messages: { ...s.messages, [key]: merge(s.messages[key] ?? [], [real]) },
messages: { ...s.messages, [key]: applyEmbeds(merge(s.messages[key] ?? [], [real])) },
}))
if (key[0] !== 't') get().markRead(key, real.id)
}
Expand Down Expand Up @@ -1019,7 +1035,9 @@ export const useStore = create<RChatState>()((set, get) => {
const kind = get().servers[server]?.channels.find(c => c.id === channelId)?.kind
if (kind === 'voice') return
const msgs = await api.channelMessages(channelId)
set(s => ({ messages: { ...s.messages, [`c${channelId}`]: [...msgs].sort(byId) } }))
set(s => ({
messages: { ...s.messages, [`c${channelId}`]: applyEmbeds([...msgs].sort(byId)) },
}))
askP2p(msgs)
}),

Expand All @@ -1034,7 +1052,7 @@ export const useStore = create<RChatState>()((set, get) => {
syncUrl(view, nav)
get().setViewing(null)
const msgs = await api.dmMessages(dmId)
set(s => ({ messages: { ...s.messages, [`d${dmId}`]: [...msgs].sort(byId) } }))
set(s => ({ messages: { ...s.messages, [`d${dmId}`]: applyEmbeds([...msgs].sort(byId)) } }))
askP2p(msgs)
}),

Expand Down Expand Up @@ -1151,7 +1169,9 @@ export const useStore = create<RChatState>()((set, get) => {
act(async () => {
set({ panel: { kind: 'thread', root }, threadPending: null })
const msgs = await api.threadMessages(root.id)
set(s => ({ messages: { ...s.messages, [`t${root.id}`]: [...msgs].sort(byId) } }))
set(s => ({
messages: { ...s.messages, [`t${root.id}`]: applyEmbeds([...msgs].sort(byId)) },
}))
askP2p(msgs)
}),

Expand Down Expand Up @@ -1535,7 +1555,7 @@ export const useStore = create<RChatState>()((set, get) => {
set(s => {
const messages = { ...s.messages }
const tkey = `t${rootId}`
if (messages[tkey]) messages[tkey] = merge(messages[tkey], [m])
if (messages[tkey]) messages[tkey] = applyEmbeds(merge(messages[tkey], [m]))
const ckey = m.channel_id !== null ? `c${m.channel_id}` : null
if (ckey && messages[ckey]) {
messages[ckey] = messages[ckey].map(r =>
Expand All @@ -1558,7 +1578,7 @@ export const useStore = create<RChatState>()((set, get) => {
if (key) {
set(s =>
s.messages[key]
? { messages: { ...s.messages, [key]: merge(s.messages[key], [m]) } }
? { messages: { ...s.messages, [key]: applyEmbeds(merge(s.messages[key], [m])) } }
: {}
)
}
Expand Down Expand Up @@ -1652,10 +1672,13 @@ export const useStore = create<RChatState>()((set, get) => {
return
}
case 'embeds_resolved': {
rememberEmbeds(ev.message_id, ev.embeds)
patchMessage(ev.message_id, m => ({ ...m, embeds: ev.embeds }))
return
}
case 'embeds_removed': {
const cur = pendingEmbeds.get(ev.message_id)
if (cur) rememberEmbeds(ev.message_id, withoutEmbed(cur, ev.ord, ev.banner))
patchMessage(ev.message_id, m => ({
...m,
embeds: withoutEmbed(m.embeds, ev.ord, ev.banner),
Expand Down
Loading