Skip to content

Fix infinite skeleton when deleting expense and returning to self DM #83156

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

Open
marufsharifi wants to merge 1 commit into
base: main
Choose a base branch
from
+52 −9
Open

Fix infinite skeleton when deleting expense and returning to self DM #83156

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
55 changes: 47 additions & 8 deletions src/libs/Navigation/Navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,21 +808,60 @@ function removeScreenByKey(key: string) {
function removeReportScreen(reportIDSet: Set<string>) {
isNavigationReady().then(() => {
navigationRef.current?.dispatch((state) => {
const routes = state?.routes.filter((route) => {
if (route.name === SCREENS.REPORT && route.params && 'reportID' in route.params) {
return !reportIDSet.has(route.params?.reportID as string);
}
return true;
});
const removalResult = removeReportRoutesFromState(state, reportIDSet);
if (!removalResult) {
return state;
}
const {updatedState, didRemoveRouteAtRoot} = removalResult;
const routes = updatedState.routes ?? [];
const nextIndex = didRemoveRouteAtRoot ? Math.max(0, Math.min(updatedState.index ?? 0, routes.length - 1)) : updatedState.index;
Copy link
Contributor

@github-actions github-actions bot Feb 22, 2026

Choose a reason for hiding this comment

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

❌ CONSISTENCY-3 (docs)

The index clamping logic here duplicates the same computation already performed inside removeReportRoutesFromState (line 861). The returned updatedState.index is already set to Math.max(0, Math.min(state.index, nextRoutes.length - 1)) when didRemoveRouteAtRoot is true, so re-clamping it here is redundant.

Since removeReportRoutesFromState already returns the correctly computed index in updatedState, you can simplify this to use updatedState.index directly:

const {updatedState, didRemoveRouteAtRoot} = removalResult;
const routes = updatedState.routes ?? [];
return CommonActions.reset({
    ...updatedState,
    routes,
    index: updatedState.index ?? 0,
});

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

return CommonActions.reset({
...state,
...updatedState,
routes,
index: routes.length < state.routes.length ? state.index - 1 : state.index,
index: nextIndex ?? 0,
});
});
});
}

function removeReportRoutesFromState(state: NavigationState, reportIDSet: Set<string>): {updatedState: NavigationState; didRemoveRouteAtRoot: boolean} | null {
if (!state?.routes) {
return null;
}

let didRemoveRouteAtRoot = false;
let didChangeNestedRoutes = false;

const nextRoutes = state.routes
.map((route) => {
if (route.name === SCREENS.REPORT && route.params && 'reportID' in route.params) {
const reportID = route.params?.reportID as string;
if (reportIDSet.has(reportID)) {
didRemoveRouteAtRoot = true;
return null;
}
}

if (route.state && 'routes' in route.state) {
const nestedState = removeReportRoutesFromState(route.state as NavigationState, reportIDSet);
if (nestedState) {
didChangeNestedRoutes = didChangeNestedRoutes || nestedState.didRemoveRouteAtRoot || nestedState.updatedState !== route.state;
return {...route, state: nestedState.updatedState};
}
}

return route;
})
.filter((route): route is NavigationState['routes'][number] => !!route);

if (!didRemoveRouteAtRoot && !didChangeNestedRoutes) {
return null;
}

const nextIndex = didRemoveRouteAtRoot ? Math.max(0, Math.min(state.index, nextRoutes.length - 1)) : state.index;
return {updatedState: {...state, routes: nextRoutes, index: nextIndex}, didRemoveRouteAtRoot};
}

function isOnboardingFlow() {
const state = navigationRef.getRootState();
const currentFocusedRoute = findFocusedRoute(state);
Expand Down
6 changes: 5 additions & 1 deletion src/pages/inbox/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,10 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
return;
}

if (reportIDFromRoute) {
Navigation.removeReportScreen(new Set([reportIDFromRoute]));
}
Comment on lines +831 to +833
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 22, 2026

Choose a reason for hiding this comment

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

P1 Badge Avoid removing the next report route on stale delete state

This effect now runs when reportIDFromRoute changes, but reportWasDeleted is reset asynchronously in useReportWasDeleted (see src/pages/inbox/hooks/useReportWasDeleted.ts), so there is a render where reportWasDeleted is still true for the previous report while reportIDFromRoute already points to the new one. In that window, calling Navigation.removeReportScreen(new Set([reportIDFromRoute])) removes a valid report route and can incorrectly kick users back to fallback navigation when moving away from a deleted report in flows where ReportScreen is reused.

Useful? React with 👍 / 👎.


// Only redirect if focused
if (!isFocused) {
return;
Expand All @@ -845,7 +849,7 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr
Navigation.isNavigationReady().then(() => {
navigateToConciergeChat(conciergeReportID);
});
}, [reportWasDeleted, isFocused, deletedReportParentID, conciergeReportID]);
}, [reportWasDeleted, isFocused, deletedReportParentID, conciergeReportID, reportIDFromRoute]);

useEffect(() => {
if (!isValidReportIDFromPath(reportIDFromRoute)) {
Expand Down
Loading