Skip to content
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.

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

fix: unsub userIsTyping event on report screen unmount #43542

Merged
merged 4 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
19 changes: 9 additions & 10 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import Navigation from '@libs/Navigation/Navigation';
import clearReportNotifications from '@libs/Notification/clearReportNotifications';
import Performance from '@libs/Performance';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import * as Pusher from '@libs/Pusher/pusher';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import shouldFetchReport from '@libs/shouldFetchReport';
Expand All @@ -52,6 +53,7 @@ import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import HeaderView from './HeaderView';
import ReportActionsView from './report/ReportActionsView';
import ReportFooter from './report/ReportFooter';
import useReportPusherEventSubscription from './report/useReportPusherEventSubscription';
import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext';
import {ActionListContext, ReactionListContext} from './ReportScreenContext';

Expand Down Expand Up @@ -304,7 +306,6 @@ function ReportScreen({
const isSingleTransactionView = ReportUtils.isMoneyRequest(report) || ReportUtils.isTrackExpenseReport(report);
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] ?? null;
const isTopMostReportId = currentReportID === reportIDFromRoute;
const didSubscribeToReportLeavingEvents = useRef(false);

useEffect(() => {
if (!report.reportID || shouldHideReport) {
Expand Down Expand Up @@ -453,6 +454,8 @@ function ReportScreen({
useEffect(clearNotifications, [clearNotifications]);
useAppFocusEvent(clearNotifications);

const subscriptionManagerMap = useReportPusherEventSubscription();

useEffect(() => {
Timing.end(CONST.TIMING.CHAT_RENDER);
Performance.markEnd(CONST.TIMING.CHAT_RENDER);
Expand All @@ -462,11 +465,7 @@ function ReportScreen({
});
return () => {
interactionTask.cancel();
if (!didSubscribeToReportLeavingEvents.current) {
return;
}

Report.unsubscribeFromLeavingRoomReportChannel(report.reportID);
subscriptionManagerMap.forEach((manager) => manager.unsubscribe(report.reportID));
};

// I'm disabling the warning, as it expects to use exhaustive deps, even though we want this useEffect to run only on the first render.
Expand Down Expand Up @@ -582,10 +581,9 @@ function ReportScreen({
// Existing reports created will have empty fields for `pendingFields`.
const didCreateReportSuccessfully = !report.pendingFields || (!report.pendingFields.addWorkspaceRoom && !report.pendingFields.createChat);
let interactionTask: ReturnType<typeof InteractionManager.runAfterInteractions> | null = null;
if (!didSubscribeToReportLeavingEvents.current && didCreateReportSuccessfully) {
if (didCreateReportSuccessfully) {
interactionTask = InteractionManager.runAfterInteractions(() => {
Report.subscribeToReportLeavingEvents(reportIDFromRoute);
didSubscribeToReportLeavingEvents.current = true;
subscriptionManagerMap.get(Pusher.TYPE.USER_IS_LEAVING_ROOM)?.subscribe(reportIDFromRoute);
});
}
return () => {
Expand All @@ -594,7 +592,7 @@ function ReportScreen({
}
interactionTask.cancel();
};
}, [report, didSubscribeToReportLeavingEvents, reportIDFromRoute]);
}, [report, reportIDFromRoute, subscriptionManagerMap]);

const onListLayout = useCallback((event: LayoutChangeEvent) => {
setListHeight((prev) => event.nativeEvent?.layout?.height ?? prev);
Expand Down Expand Up @@ -717,6 +715,7 @@ function ReportScreen({
hasLoadingOlderReportActionsError={reportMetadata?.hasLoadingOlderReportActionsError}
isReadyForCommentLinking={!shouldShowSkeleton}
transactionThreadReportID={transactionThreadReportID}
userTypingEventSubscriptionManager={subscriptionManagerMap.get(Pusher.TYPE.USER_IS_TYPING)}
/>
)}

Expand Down
9 changes: 8 additions & 1 deletion src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject';
import getInitialPaginationSize from './getInitialPaginationSize';
import PopoverReactionList from './ReactionList/PopoverReactionList';
import ReportActionsList from './ReportActionsList';
import type {ReportPusherSubscriptionManager} from './useReportPusherEventSubscription/types';
import UserTypingEventListener from './UserTypingEventListener';

type ReportActionsViewOnyxProps = {
Expand Down Expand Up @@ -76,6 +77,8 @@ type ReportActionsViewProps = ReportActionsViewOnyxProps & {
/** The reportID of the transaction thread report associated with this current report, if any */
// eslint-disable-next-line react/no-unused-prop-types
transactionThreadReportID?: string | null;

userTypingEventSubscriptionManager?: ReportPusherSubscriptionManager;
};

const DIFF_BETWEEN_SCREEN_HEIGHT_AND_LIST = 120;
Expand All @@ -96,6 +99,7 @@ function ReportActionsView({
isLoadingNewerReportActions = false,
hasLoadingNewerReportActionsError = false,
isReadyForCommentLinking = false,
userTypingEventSubscriptionManager,
}: ReportActionsViewProps) {
useCopySelectionHelper();
const reactionListRef = useContext(ReactionListContext);
Expand Down Expand Up @@ -547,7 +551,10 @@ function ReportActionsView({
onContentSizeChange={onContentSizeChange}
shouldEnableAutoScrollToTopThreshold={shouldEnableAutoScroll}
/>
<UserTypingEventListener report={report} />
<UserTypingEventListener
report={report}
userTypingEventSubscriptionManager={userTypingEventSubscriptionManager}
/>
<PopoverReactionList ref={reactionListRef} />
</>
);
Expand Down
21 changes: 10 additions & 11 deletions src/pages/home/report/UserTypingEventListener.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import type {RouteProp} from '@react-navigation/native';
import {useIsFocused, useRoute} from '@react-navigation/native';
import {useEffect, useRef} from 'react';
import {useEffect} from 'react';
import {InteractionManager} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import Navigation from '@libs/Navigation/Navigation';
import type {CentralPaneNavigatorParamList} from '@libs/Navigation/types';
import * as Report from '@userActions/Report';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type {ReportPusherSubscriptionManager} from './useReportPusherEventSubscription/types';

type UserTypingEventListenerOnyxProps = {
/** Stores last visited path */
Expand All @@ -18,9 +18,10 @@ type UserTypingEventListenerOnyxProps = {
type UserTypingEventListenerProps = UserTypingEventListenerOnyxProps & {
/** The report currently being looked at */
report: OnyxTypes.Report;

userTypingEventSubscriptionManager?: ReportPusherSubscriptionManager;
};
function UserTypingEventListener({report, lastVisitedPath}: UserTypingEventListenerProps) {
const didSubscribeToReportTypingEvents = useRef(false);
function UserTypingEventListener({report, lastVisitedPath, userTypingEventSubscriptionManager}: UserTypingEventListenerProps) {
const reportID = report.reportID;
const isFocused = useIsFocused();
const route = useRoute<RouteProp<CentralPaneNavigatorParamList, typeof SCREENS.REPORT>>();
Expand All @@ -37,19 +38,17 @@ function UserTypingEventListener({report, lastVisitedPath}: UserTypingEventListe
// Existing reports created will have empty fields for `pendingFields`.
const didCreateReportSuccessfully = !report.pendingFields || (!report.pendingFields.addWorkspaceRoom && !report.pendingFields.createChat);

if (!didSubscribeToReportTypingEvents.current && didCreateReportSuccessfully) {
if (didCreateReportSuccessfully) {
interactionTask = InteractionManager.runAfterInteractions(() => {
Report.subscribeToReportTypingEvents(reportID);
didSubscribeToReportTypingEvents.current = true;
userTypingEventSubscriptionManager?.subscribe(reportID);
});
}
} else {
const topmostReportId = Navigation.getTopmostReportId();

if (topmostReportId !== reportID && didSubscribeToReportTypingEvents.current) {
didSubscribeToReportTypingEvents.current = false;
if (topmostReportId !== reportID) {
InteractionManager.runAfterInteractions(() => {
Report.unsubscribeFromReportChannel(reportID);
userTypingEventSubscriptionManager?.unsubscribe(reportID);
});
}
}
Expand All @@ -59,7 +58,7 @@ function UserTypingEventListener({report, lastVisitedPath}: UserTypingEventListe
}
interactionTask.cancel();
};
}, [isFocused, report.pendingFields, didSubscribeToReportTypingEvents, lastVisitedPath, reportID, route]);
}, [isFocused, report.pendingFields, lastVisitedPath, reportID, route, userTypingEventSubscriptionManager]);

return null;
}
Expand Down
49 changes: 49 additions & 0 deletions src/pages/home/report/useReportPusherEventSubscription/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {useRef} from 'react';
import * as Pusher from '@libs/Pusher/pusher';
import * as Report from '@userActions/Report';
import type {ReportPusherSubscriptionManager, ReportSubscribeFunction} from './types';

const reportScreenPusherEventSubscriptionMap: Record<string, Omit<ReportPusherSubscriptionManager, 'eventName'>> = {
[Pusher.TYPE.USER_IS_TYPING]: {
subscribe: Report.subscribeToReportTypingEvents,
unsubscribe: Report.unsubscribeFromReportChannel,
},
[Pusher.TYPE.USER_IS_LEAVING_ROOM]: {
subscribe: Report.subscribeToReportLeavingEvents,
unsubscribe: Report.unsubscribeFromLeavingRoomReportChannel,
},
};

const useReportPusherEventSubscription = (): Map<string, ReportPusherSubscriptionManager> => {
const didSubscribedMapRef = useRef<Record<string, boolean>>(Object.fromEntries(Object.keys(reportScreenPusherEventSubscriptionMap).map((eventName) => [eventName, false])));

const createSubscriptionFunctionRef = useRef(
(eventName: string, fn: ReportSubscribeFunction, isForSubscribed = true): ReportSubscribeFunction =>
(reportID: string, ...args: unknown[]) => {
// if the subscription state is already in the desired state (i.e try to subscribe or resubscribe again => return early)
if (!!didSubscribedMapRef.current[eventName] === !!isForSubscribed) {
return;
}

didSubscribedMapRef.current[eventName] = !!isForSubscribed;
fn(reportID, ...args);
},
);

const reportSubscriptionManagerMapRef = useRef<Map<string, ReportPusherSubscriptionManager>>(
new Map(
Object.entries(reportScreenPusherEventSubscriptionMap).map(([eventName, {subscribe, unsubscribe}]) => [
eventName,
{
eventName,
subscribe: createSubscriptionFunctionRef.current(eventName, subscribe, true),
unsubscribe: createSubscriptionFunctionRef.current(eventName, unsubscribe, false),
},
]),
),
);

return reportSubscriptionManagerMapRef.current;
};

export default useReportPusherEventSubscription;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type ReportSubscribeFunction = (reportID: string, ...args: unknown[]) => void;

type ReportPusherSubscriptionManager = {
eventName: string;
subscribe: ReportSubscribeFunction;
unsubscribe: ReportSubscribeFunction;
};

export type {ReportPusherSubscriptionManager, ReportSubscribeFunction};
Loading