Skip to content

Commit 62fa9e1

Browse files
committed
Cache draft across remounts; address bot review patterns
Two things in one commit since they touch overlapping lines: 1. Module-level draftCache so navigating away and back to a chat doesn't flash the synthetic Concierge bubble away during the remount + Onyx-hydration window. Gate's useState lazy-inits from the cache; every reducer write mirrors to the cache; the reducer's null returns (completed / failed / cleared) evict it. Keyed by reportID so chats stay isolated. 4 unit specs cover the contract. 2. Drop redundant useCallback/useMemo on clearDraft, dispatchLocalDraftEvent, stateValue, actionsValue — React Compiler auto-memoizes; the explicit wrappers just shadow its analysis (clean-react-0-compiler bot review). Inline the resubscribe-clear logic so the effect's deps stay scoped to reportID instead of dragging in a per-render clearDraft closure. 3. Extract the trickle-vs-binary-reveal threshold (100 anchors) to a named MIN_TRICKLE_TOKEN_COUNT constant — matches the pattern of every other timing/sizing constant in this file (consistency-2 bot review).
1 parent 03c1339 commit 62fa9e1

4 files changed

Lines changed: 101 additions & 33 deletions

File tree

src/hooks/usePendingConciergeResponse.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ const TICK_INTERVAL_MS = 80;
1919
const TRICKLE_HARD_CAP_MS = 60_000;
2020
/** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */
2121
const ACCELERATED_REMAINING_MS = 1_500;
22+
/** Minimum char-level anchors before we opt into the trickle reveal. Replies under this fall back to the binary reveal at `displayAfter`. */
23+
const MIN_TRICKLE_TOKEN_COUNT = 100;
2224

2325
function easeOut(t: number): number {
2426
const clamped = Math.max(0, Math.min(1, t));
@@ -88,7 +90,7 @@ function usePendingConciergeResponse(reportID: string | undefined) {
8890
// Anchors are character-level. Short replies (~50–100 chars) keep the
8991
// binary reveal; longer ones (paragraphs / lists) cross the threshold
9092
// and get the smooth trickle.
91-
const shouldTrickle = snapshotTokens.length >= 100 && !!snapshotHtml;
93+
const shouldTrickle = snapshotTokens.length >= MIN_TRICKLE_TOKEN_COUNT && !!snapshotHtml;
9294
if (!shouldTrickle) {
9395
const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay));
9496
return () => clearTimeout(timer);

src/pages/inbox/ConciergeDraftContext.tsx

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {getReportChatType} from '@selectors/Report';
2-
import React, {createContext, useCallback, useContext, useEffect, useMemo, useState} from 'react';
2+
import React, {createContext, useContext, useEffect, useState} from 'react';
33
import useOnyx from '@hooks/useOnyx';
44
import {getReportChannelName} from '@libs/actions/Report';
55
import Log from '@libs/Log';
@@ -9,7 +9,7 @@ import CONST from '@src/CONST';
99
import ONYXKEYS from '@src/ONYXKEYS';
1010
import type {ReportAction} from '@src/types/onyx';
1111
import type {ConciergeDraft} from './conciergeDraftState';
12-
import {applyConciergeDraftEvent} from './conciergeDraftState';
12+
import {applyConciergeDraftEvent, getCachedDraft, setCachedDraft} from './conciergeDraftState';
1313

1414
type ConciergeDraftState = {
1515
draftReportAction: ReportAction | null;
@@ -61,23 +61,34 @@ function ConciergeDraftProvider({reportID, children}: React.PropsWithChildren<{r
6161
}
6262

6363
function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{reportID: string}>) {
64-
const [draft, setDraft] = useState<ConciergeDraft | null>(null);
65-
66-
const clearDraft = useCallback(() => {
64+
// Lazy-init from the module-level cache so a remount (ReportScreen
65+
// unmount/remount on chat-switch) restores the in-progress draft on the
66+
// first paint instead of flashing the synthetic bubble away.
67+
const [draft, setDraft] = useState<ConciergeDraft | null>(() => getCachedDraft(reportID));
68+
69+
// React Compiler auto-memoizes; explicit useCallback/useMemo would just
70+
// shadow the compiler's analysis (clean-react-0-compiler).
71+
const clearDraft = () => {
72+
setCachedDraft(reportID, null);
6773
setDraft(null);
68-
}, []);
74+
};
6975

70-
const dispatchLocalDraftEvent = useCallback(
71-
(event: ConciergeDraftEvent) => {
72-
setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, event, reportID));
73-
},
74-
[reportID],
75-
);
76+
const dispatchLocalDraftEvent = (event: ConciergeDraftEvent) => {
77+
setDraft((currentDraft) => {
78+
const next = applyConciergeDraftEvent(currentDraft, event, reportID);
79+
setCachedDraft(reportID, next);
80+
return next;
81+
});
82+
};
7683

7784
useEffect(() => {
7885
const channelName = getReportChannelName(reportID);
86+
// Inline the clear so the effect's deps stay scoped to reportID; closing
87+
// over `clearDraft` would either drag it into deps (re-subscribing on
88+
// every render) or trip exhaustive-deps.
7989
const handleResubscribe = () => {
80-
clearDraft();
90+
setCachedDraft(reportID, null);
91+
setDraft(null);
8192
};
8293
const eventTypes = [
8394
Pusher.TYPE.CONCIERGE_DRAFT_STARTED,
@@ -93,7 +104,11 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor
93104
eventType,
94105
(eventData) => {
95106
const conciergeDraftEvent = eventData as ConciergeDraftEvent;
96-
setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, conciergeDraftEvent, reportID));
107+
setDraft((currentDraft) => {
108+
const next = applyConciergeDraftEvent(currentDraft, conciergeDraftEvent, reportID);
109+
setCachedDraft(reportID, next);
110+
return next;
111+
});
97112
},
98113
handleResubscribe,
99114
);
@@ -110,23 +125,17 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor
110125
subscription.unsubscribe();
111126
}
112127
};
113-
}, [clearDraft, reportID]);
114-
115-
const stateValue = useMemo<ConciergeDraftState>(
116-
() => ({
117-
draftReportAction: draft?.reportAction ?? null,
118-
hasActiveDraft: !!draft?.reportAction,
119-
}),
120-
[draft?.reportAction],
121-
);
128+
}, [reportID]);
122129

123-
const actionsValue = useMemo<ConciergeDraftActions>(
124-
() => ({
125-
clearDraft,
126-
dispatchLocalDraftEvent,
127-
}),
128-
[clearDraft, dispatchLocalDraftEvent],
129-
);
130+
const stateValue: ConciergeDraftState = {
131+
draftReportAction: draft?.reportAction ?? null,
132+
hasActiveDraft: !!draft?.reportAction,
133+
};
134+
135+
const actionsValue: ConciergeDraftActions = {
136+
clearDraft,
137+
dispatchLocalDraftEvent,
138+
};
130139

131140
return (
132141
<ConciergeDraftActionsContext.Provider value={actionsValue}>

src/pages/inbox/conciergeDraftState.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,26 @@ function buildConciergeDraftReportAction({bodyMarkdown, created, finalRenderedHT
4040
} as ReportAction;
4141
}
4242

43+
// Module-level cache so a chat re-mount (ReportScreen unmount/remount on chat
44+
// switch) preserves the in-progress draft. Without this the gate's local state
45+
// resets to null on every revisit and the synthetic bubble disappears for the
46+
// remount + Onyx-hydration window. Keyed by reportID; entries are evicted by
47+
// `setCachedDraft(reportID, null)` when the reducer returns null
48+
// (completed/failed/cleared).
49+
const draftCache = new Map<string, ConciergeDraft>();
50+
51+
function getCachedDraft(reportID: string): ConciergeDraft | null {
52+
return draftCache.get(reportID) ?? null;
53+
}
54+
55+
function setCachedDraft(reportID: string, draft: ConciergeDraft | null): void {
56+
if (draft) {
57+
draftCache.set(reportID, draft);
58+
} else {
59+
draftCache.delete(reportID);
60+
}
61+
}
62+
4363
function applyConciergeDraftEvent(currentDraft: ConciergeDraft | null, event: ConciergeDraftEvent, reportID: string): ConciergeDraft | null {
4464
if (event.reportID !== reportID) {
4565
return currentDraft;
@@ -81,5 +101,5 @@ function applyConciergeDraftEvent(currentDraft: ConciergeDraft | null, event: Co
81101
};
82102
}
83103

84-
export {applyConciergeDraftEvent, buildConciergeDraftReportAction};
104+
export {applyConciergeDraftEvent, buildConciergeDraftReportAction, getCachedDraft, setCachedDraft};
85105
export type {ConciergeDraft};

tests/unit/pages/inbox/conciergeDraftState.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {applyConciergeDraftEvent} from '@pages/inbox/conciergeDraftState';
1+
import {applyConciergeDraftEvent, getCachedDraft, setCachedDraft} from '@pages/inbox/conciergeDraftState';
22
import CONST from '@src/CONST';
33

44
const REPORT_ID = '123';
@@ -131,4 +131,41 @@ describe('conciergeDraftState', () => {
131131

132132
expect(otherReportDraft).toBe(initialDraft);
133133
});
134+
135+
describe('draftCache', () => {
136+
// Always start clean so tests don't leak state into each other.
137+
beforeEach(() => {
138+
setCachedDraft(REPORT_ID, null);
139+
});
140+
141+
it('returns null for an unseen reportID', () => {
142+
expect(getCachedDraft('never-stored')).toBeNull();
143+
});
144+
145+
it('persists a draft across set/get and survives across calls (the remount survival contract)', () => {
146+
const draft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID);
147+
expect(draft).not.toBeNull();
148+
setCachedDraft(REPORT_ID, draft);
149+
expect(getCachedDraft(REPORT_ID)).toBe(draft);
150+
// Simulating a remount: a fresh getCachedDraft call returns the same instance.
151+
expect(getCachedDraft(REPORT_ID)).toBe(draft);
152+
});
153+
154+
it('evicts when set to null (completed/failed/cleared reducer return)', () => {
155+
const draft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID);
156+
setCachedDraft(REPORT_ID, draft);
157+
expect(getCachedDraft(REPORT_ID)).not.toBeNull();
158+
setCachedDraft(REPORT_ID, null);
159+
expect(getCachedDraft(REPORT_ID)).toBeNull();
160+
});
161+
162+
it('keeps entries scoped per reportID (no cross-talk)', () => {
163+
const draftA = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID);
164+
const draftB = applyConciergeDraftEvent(null, createDraftEvent({reportID: 'other'}), 'other');
165+
setCachedDraft(REPORT_ID, draftA);
166+
setCachedDraft('other', draftB);
167+
expect(getCachedDraft(REPORT_ID)).toBe(draftA);
168+
expect(getCachedDraft('other')).toBe(draftB);
169+
});
170+
});
134171
});

0 commit comments

Comments
 (0)