Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 35 additions & 5 deletions src/injected.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,19 @@ const start = async () => {
// only sees rooms that changed after the listener attached, so it can
// undercount; we prefer the aggregate for the numeric total and use the
// map solely to rebuild the alert-only "•" indicator.
// Server boot floods this path: the webapp fires one
// `unread-changed-by-subscription` event per room when it starts, and
// dispatching a badge update for each one storms the root window's
// Redux store hard enough that React aborts with "Maximum update depth
// exceeded". Recomputes are therefore coalesced into a single
// trailing-edge call, and the dispatch is skipped entirely when the
// resolved badge value did not change.
const BADGE_COALESCE_MS = 100;
let resolveBadgeTimer: ReturnType<typeof setTimeout> | null = null;
let pendingAggregateCount: number | undefined;
let lastSentBadge: number | '•' | undefined;
let hasSentBadge = false;
Comment on lines +540 to +541

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Share badge deduplication state across both producers.

lastSentBadge tracks only values sent by resolveBadge. The pre-7.8 Tracker.autorun at Line [500-503] calls window.RocketChatDesktop.setBadge directly and does not update this state. If the event path sends 3, the Session path sends 0, and the event path computes 3 again, this guard returns and leaves the actual badge at 0.

Route both producers through one shared helper, or remove this second guard and rely on the shared deduplication in src/servers/preload/badge.ts.

Suggested fix
-      let lastSentBadge: number | '•' | undefined;
-      let hasSentBadge = false;
...
-        if (hasSentBadge && badge === lastSentBadge) {
-          return;
-        }
-        hasSentBadge = true;
-        lastSentBadge = badge;
         window.RocketChatDesktop.setBadge(badge);

Also applies to: 573-579

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/injected.ts` around lines 540 - 541, Share badge deduplication across the
event and pre-7.8 Tracker.autorun producers: route both paths through a single
helper that updates the same lastSentBadge state, or remove the local guard and
rely on the shared deduplication in preload/badge.ts. Ensure alternating badge
values from either producer are applied correctly without stale local state
suppressing updates.


const resolveBadge = (aggregateCount?: number): void => {
let unreadCount = 0;
let alertIndicator: '•' | undefined;
Expand Down Expand Up @@ -557,11 +570,28 @@ const start = async () => {
? aggregateCount
: unreadCount;

if (total > 0) {
window.RocketChatDesktop.setBadge(total);
const badge = total > 0 ? total : alertIndicator ?? 0;
if (hasSentBadge && badge === lastSentBadge) {
return;
}
hasSentBadge = true;
lastSentBadge = badge;
window.RocketChatDesktop.setBadge(badge);
};

const scheduleResolveBadge = (aggregateCount?: number): void => {
if (aggregateCount !== undefined) {
pendingAggregateCount = aggregateCount;
}
if (resolveBadgeTimer !== null) {
return;
}
window.RocketChatDesktop.setBadge(alertIndicator ?? 0);
resolveBadgeTimer = setTimeout(() => {
resolveBadgeTimer = null;
const aggregate = pendingAggregateCount;
pendingAggregateCount = undefined;
resolveBadge(aggregate);
}, BADGE_COALESCE_MS);
};

window.addEventListener('unread-changed-by-subscription', (event) => {
Expand All @@ -581,7 +611,7 @@ const start = async () => {
alert: subscription.alert,
unreadAlert: subscription.unreadAlert,
});
resolveBadge();
scheduleResolveBadge();
});

window.addEventListener('unread-changed', (event) => {
Expand All @@ -590,7 +620,7 @@ const start = async () => {
typeof detail === 'number' && Number.isFinite(detail)
? detail
: undefined;
resolveBadge(aggregateCount);
scheduleResolveBadge(aggregateCount);
});

setupFlags.unreadChangedEvent = true;
Expand Down
4 changes: 3 additions & 1 deletion src/servers/bootWatchdog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,11 @@ export const attachBootWatchdog = (
booted: false,
reportedForCurrentLoad: false,
};
// The deadline is armed by the first committed navigation (did-navigate),
// not here: webviews can attach and legitimately never navigate (lazy or
// error-view panes), and reporting those is pure noise.
watchStates.set(serverUrl, state);
record(state, 'attached');
startDeadline(state);

webContents.on('console-message', (event) => {
const message = String(event.message ?? '').slice(0, MESSAGE_LENGTH_LIMIT);
Expand Down
12 changes: 12 additions & 0 deletions src/servers/preload/badge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,19 @@ import { WEBVIEW_UNREAD_CHANGED } from '../../ui/actions';
import type { Server } from '../common';
import { getServerUrl } from './urls';

let hasDispatched = false;
let lastBadge: Server['badge'];

export const setBadge = (badge: Server['badge']): void => {
// The pre-7.8.0 Session autorun and the unread event listeners can both
// re-emit unchanged values in rapid succession; a no-op dispatch still
// re-renders every server-subscribed component in the root window.
if (hasDispatched && Object.is(badge, lastBadge)) {
return;
}
hasDispatched = true;
lastBadge = badge;

dispatch({
type: WEBVIEW_UNREAD_CHANGED,
payload: {
Expand Down
24 changes: 18 additions & 6 deletions src/servers/reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,27 @@ type ServersActionTypes =
| ActionOf<typeof WEBVIEW_PAGE_TITLE_CHANGED>
| ActionOf<typeof SIDE_BAR_SERVER_REMOVE>;

// Returns the original object (preserving identity) when the patch would not
// change any field — a new array identity here re-renders every
// server-subscribed component, so no-op actions must not mint one.
const patchServer = (server: Server, patch: Server): Server => {
const changed = Object.entries(patch).some(
([key, value]) => !Object.is(server[key as keyof Server], value)
);
return changed ? { ...server, ...patch } : server;
};

const upsert = (state: Server[], server: Server): Server[] => {
const index = state.findIndex(({ url }) => url === server.url);

if (index === -1) {
return [...state, server];
}

return state.map((_server, i) =>
i === index ? { ..._server, ...server } : _server
);
const patched = patchServer(state[index], server);
return patched === state[index]
? state
: state.map((_server, i) => (i === index ? patched : _server));
};

const update = (state: Server[], server: Server): Server[] => {
Expand All @@ -96,9 +107,10 @@ const update = (state: Server[], server: Server): Server[] => {
return state;
}

return state.map((_server, i) =>
i === index ? { ..._server, ...server } : _server
);
const patched = patchServer(state[index], server);
return patched === state[index]
? state
: state.map((_server, i) => (i === index ? patched : _server));
};

export const servers: Reducer<Server[], ServersActionTypes> = (
Expand Down
Loading