Skip to content

[HOLD] [Odometer] Fix state restoration after App reload when image preview is open #82407

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
jakubkalinski0 wants to merge 2 commits into from
Closed

[HOLD] [Odometer] Fix state restoration after App reload when image preview is open #82407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c15b86
Fixed navigation after App reload when image preview is open
jakubkalinski0 Feb 13, 2026
6d13464
Dont revoke blob url when editing from confirmation page
jakubkalinski0 Feb 13, 2026
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
51 changes: 47 additions & 4 deletions src/libs/Navigation/helpers/getAdaptedStateFromPath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import {RHP_TO_DOMAIN, RHP_TO_SEARCH, RHP_TO_SETTINGS, RHP_TO_SIDEBAR, RHP_TO_WORKSPACE, RHP_TO_WORKSPACES_LIST} from '@libs/Navigation/linkingConfig/RELATIONS';
import type {NavigationPartialRoute, RootNavigatorParamList} from '@libs/Navigation/types';
import CONST from '@src/CONST';
import type {IOUAction, IOUType} from '@src/CONST';
import {getSearchParamFromPath} from '@src/libs/Url';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -22,7 +23,7 @@
import replacePathInNestedState from './replacePathInNestedState';

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 26 in src/libs/Navigation/helpers/getAdaptedStateFromPath.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function

Check warning on line 26 in src/libs/Navigation/helpers/getAdaptedStateFromPath.ts

View workflow job for this annotation

GitHub Actions / ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -45,6 +46,28 @@
return route.params !== undefined && 'reportID' in route.params && typeof route.params.reportID === 'string';
}

function isValidReportID(reportID: string): boolean {
return !!allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportID;
}

function isRouteWithOdometerReceiptPreviewParams(
route: NavigationPartialRoute,
): route is Route<string, {action: IOUAction; iouType: IOUType; transactionID: string; reportID: string; imageType: string}> {
return (
route.params !== undefined &&
'action' in route.params &&
typeof route.params.action === 'string' &&
'iouType' in route.params &&
typeof route.params.iouType === 'string' &&
'transactionID' in route.params &&
typeof route.params.transactionID === 'string' &&
'reportID' in route.params &&
typeof route.params.reportID === 'string' &&
'imageType' in route.params &&
typeof route.params.imageType === 'string'
);
}

/**
* Get the appropriate screen name for RHP_TO_SEARCH lookup.
* Split tabs (amount, percentage, date) are nested routes within SPLIT_EXPENSE/SPLIT_EXPENSE_SEARCH.
Expand Down Expand Up @@ -180,7 +203,7 @@
if (route && isRouteWithReportID(route)) {
const reportID = route.params.reportID;

if (!allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportID) {
if (!isValidReportID(reportID)) {
return {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR};
}

Expand Down Expand Up @@ -214,7 +237,7 @@
return getRoutesWithIndex(routes);
}

function getAdaptedState(state: PartialState<NavigationState<RootNavigatorParamList>>): GetAdaptedStateReturnType {
function getAdaptedState(state: PartialState<NavigationState<RootNavigatorParamList>>, defaultFullScreenRouteSource?: NavigationPartialRoute): GetAdaptedStateReturnType {
const fullScreenRoute = state.routes.find((route) => isFullScreenName(route.name));
const onboardingNavigator = state.routes.find((route) => route.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR);
const isWorkspaceSplitNavigator = fullScreenRoute?.name === NAVIGATORS.WORKSPACE_SPLIT_NAVIGATOR;
Expand Down Expand Up @@ -252,7 +275,12 @@
return getRoutesWithIndex([{name: SCREENS.HOME}, adaptedOnboardingNavigator]);
}

const defaultFullScreenRoute = getDefaultFullScreenRoute(focusedRoute);
let routeForDefaultFullScreen = focusedRoute as NavigationPartialRoute | undefined;
if ((!routeForDefaultFullScreen || !isRouteWithReportID(routeForDefaultFullScreen)) && defaultFullScreenRouteSource && isRouteWithReportID(defaultFullScreenRouteSource)) {
routeForDefaultFullScreen = defaultFullScreenRouteSource;
}

const defaultFullScreenRoute = getDefaultFullScreenRoute(routeForDefaultFullScreen);

// If not, add the default full screen route.
return getRoutesWithIndex([defaultFullScreenRoute, ...state.routes]);
Expand All @@ -275,6 +303,7 @@
*/
const getAdaptedStateFromPath: GetAdaptedStateFromPath = (path, options, shouldReplacePathInNestedState = true) => {
let normalizedPath = !path.startsWith('/') ? `/${path}` : path;
let defaultFullScreenRouteSource: NavigationPartialRoute | undefined;
normalizedPath = getRedirectedPath(normalizedPath);
normalizedPath = getMatchingNewRoute(normalizedPath) ?? normalizedPath;

Expand All @@ -283,6 +312,20 @@
normalizedPath = '/';
}

if (normalizedPath.includes('/receipt/')) {

@github-actions github-actions Bot Feb 13, 2026

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.

❌ PERF-2 (docs)

Expensive work performed before validation

The code calls getStateFromPath(normalizedPath, options) before validating whether focusedRouteAsPartial meets all required conditions. If the route name check or parameter validation fails, the expensive state parsing was unnecessary.

Suggested fix:

Move the validation logic before calling getStateFromPath. Since you need the state to get the focused route, consider restructuring to avoid the second getStateFromPath call:

// First check if we need special handling
if (normalizedPath.includes('/receipt/')) {
    const stateWithOdometerReceiptPreview = getStateFromPath(normalizedPath, options) as PartialState<NavigationState<RootNavigatorParamList>>;
    const focusedRoute = stateWithOdometerReceiptPreview ? findFocusedRoute(stateWithOdometerReceiptPreview) : undefined;
    const focusedRouteAsPartial = focusedRoute as NavigationPartialRoute | undefined;

    if (focusedRouteAsPartial?.name === SCREENS.MONEY_REQUEST.RECEIPT_PREVIEW && isRouteWithOdometerReceiptPreviewParams(focusedRouteAsPartial)) {
        const {reportID} = focusedRouteAsPartial.params;
        if (isValidReportID(reportID)) {
            defaultFullScreenRouteSource = focusedRouteAsPartial;
            const {action, iouType, transactionID} = focusedRouteAsPartial.params;
            normalizedPath = `/${ROUTES.DISTANCE_REQUEST_CREATE_TAB_ODOMETER.getRoute(action, iouType, transactionID, reportID)}`;
        }
    }
}

const state = getStateFromPath(normalizedPath, options) as PartialState<NavigationState<RootNavigatorParamList>>;

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

const stateWithOdometerReceiptPreview = getStateFromPath(normalizedPath, options) as PartialState<NavigationState<RootNavigatorParamList>>;
const focusedRoute = stateWithOdometerReceiptPreview ? findFocusedRoute(stateWithOdometerReceiptPreview) : undefined;
const focusedRouteAsPartial = focusedRoute as NavigationPartialRoute | undefined;

if (focusedRouteAsPartial?.name === SCREENS.MONEY_REQUEST.RECEIPT_PREVIEW && isRouteWithOdometerReceiptPreviewParams(focusedRouteAsPartial)) {
const {action, iouType, transactionID, reportID} = focusedRouteAsPartial.params;
if (isValidReportID(reportID)) {
defaultFullScreenRouteSource = focusedRouteAsPartial;
}
normalizedPath = `/${ROUTES.DISTANCE_REQUEST_CREATE_TAB_ODOMETER.getRoute(action, iouType, transactionID, reportID)}`;
}
}

const state = getStateFromPath(normalizedPath, options) as PartialState<NavigationState<RootNavigatorParamList>>;
if (shouldReplacePathInNestedState) {
replacePathInNestedState(state, normalizedPath);
Expand All @@ -292,7 +335,7 @@
throw new Error(`[getAdaptedStateFromPath] Unable to get state from path: ${path}`);
}

return getAdaptedState(state);
return getAdaptedState(state, defaultFullScreenRouteSource);
};

export default getAdaptedStateFromPath;
Expand Down
64 changes: 52 additions & 12 deletions src/pages/iou/request/step/IOURequestStepDistanceOdometer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import {
createDistanceRequest,
getMoneyRequestParticipantsFromReport,
removeMoneyRequestOdometerImage,
setCustomUnitRateID,
setMoneyRequestDistance,
setMoneyRequestMerchant,
Expand Down Expand Up @@ -251,24 +252,60 @@ function IOURequestStepDistanceOdometer({

const startImageSource = useMemo(() => getImageSource(odometerStartImage), [getImageSource, odometerStartImage]);
const endImageSource = useMemo(() => getImageSource(odometerEndImage), [getImageSource, odometerEndImage]);
const hasStartOdometerImage = !!startImageSource;

@github-actions github-actions Bot Feb 13, 2026

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.

❌ PERF-6 (docs)

Redundant state derived from memo

hasStartOdometerImage and hasEndOdometerImage are stored as constants but could be computed inline where used, or these are already derivable from startImageSource and endImageSource which are memoized.

Suggested fix:

Since these are only used once each in the handleOdometerImagePress calls, compute them inline:

onPress={() => {
    handleOdometerImagePress(CONST.IOU.ODOMETER_IMAGE_TYPE.START, !!startImageSource);
}}

// and

onPress={() => {
    handleOdometerImagePress(CONST.IOU.ODOMETER_IMAGE_TYPE.END, !!endImageSource);
}}

This eliminates the intermediate constants that don't add clarity.

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

const hasEndOdometerImage = !!endImageSource;

useEffect(() => {
return () => {
if (isEditingConfirmation) {
return;
}
if (!startImageSource?.startsWith('blob:')) {
return;
}
URL.revokeObjectURL(startImageSource);
};
}, [startImageSource]);
}, [startImageSource, isEditingConfirmation]);

useEffect(() => {
return () => {
if (isEditingConfirmation) {
return;
}
if (!endImageSource?.startsWith('blob:')) {

@github-actions github-actions Bot Feb 13, 2026

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.

❌ PERF-12 (docs)

Missing cleanup for blob validation fetch

The validateOdometerBlobSource function initiates a fetch request but doesn't provide a cleanup mechanism. If the component unmounts while the fetch is in progress, the promise continues executing and may attempt to call removeMoneyRequestOdometerImage after unmount, potentially causing memory leaks or state updates on unmounted components.

Suggested fix:

const validateOdometerBlobSource = useCallback(
    (imageSource: string | undefined, imageType: OdometerImageType) => {
        if (\!imageSource?.startsWith('blob:')) {
            return;
        }

        const controller = new AbortController();
        
        fetch(imageSource, { signal: controller.signal })
            .catch((error) => {
                if (error.name === 'AbortError') {
                    return;
                }
                if (imageType === CONST.IOU.ODOMETER_IMAGE_TYPE.START) {
                    initialStartImageRef.current = undefined;
                } else {
                    initialEndImageRef.current = undefined;
                }
                removeMoneyRequestOdometerImage(transactionID, imageType, isTransactionDraft);
            });
        
        return controller;
    },
    [isTransactionDraft, transactionID],
);

useEffect(() => {
    const controller = validateOdometerBlobSource(startImageSource, CONST.IOU.ODOMETER_IMAGE_TYPE.START);
    return () => controller?.abort();
}, [startImageSource, validateOdometerBlobSource]);

useEffect(() => {
    const controller = validateOdometerBlobSource(endImageSource, CONST.IOU.ODOMETER_IMAGE_TYPE.END);
    return () => controller?.abort();
}, [endImageSource, validateOdometerBlobSource]);

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

return;
}
URL.revokeObjectURL(endImageSource);
};
}, [endImageSource]);
}, [endImageSource, isEditingConfirmation]);

const validateOdometerBlobSource = useCallback(
(imageSource: string | undefined, imageType: OdometerImageType) => {
if (!imageSource?.startsWith('blob:')) {
return;
}

// Blob URLs are process-local and can become stale after refresh.
// If the blob cannot be fetched anymore, clear it from transaction state.
fetch(imageSource).catch(() => {
if (imageType === CONST.IOU.ODOMETER_IMAGE_TYPE.START) {
initialStartImageRef.current = undefined;
} else {
initialEndImageRef.current = undefined;
}
removeMoneyRequestOdometerImage(transactionID, imageType, isTransactionDraft);
});
},
[isTransactionDraft, transactionID],
);

useEffect(() => {
validateOdometerBlobSource(startImageSource, CONST.IOU.ODOMETER_IMAGE_TYPE.START);
}, [startImageSource, validateOdometerBlobSource]);

useEffect(() => {
validateOdometerBlobSource(endImageSource, CONST.IOU.ODOMETER_IMAGE_TYPE.END);
}, [endImageSource, validateOdometerBlobSource]);

const buttonText = (() => {
if (shouldSkipConfirmation) {
Expand Down Expand Up @@ -331,6 +368,17 @@ function IOURequestStepDistanceOdometer({
[reportID, transactionID, action, iouType],
);

const handleOdometerImagePress = useCallback(
(imageType: OdometerImageType, hasImage: boolean) => {
if (!hasImage) {
handleCaptureImage(imageType);
return;
}
handleViewOdometerImage(imageType);
},
[handleCaptureImage, handleViewOdometerImage],
);

// Navigate to confirmation page helper - following Manual tab pattern
const navigateToConfirmationPage = () => {
if (!transactionID || !reportID) {
Expand Down Expand Up @@ -593,11 +641,7 @@ function IOURequestStepDistanceOdometer({
accessibilityRole="button"
sentryLabel={CONST.SENTRY_LABEL.ODOMETER_EXPENSE.CAPTURE_IMAGE_START}
onPress={() => {
if (odometerStartImage) {
handleViewOdometerImage(CONST.IOU.ODOMETER_IMAGE_TYPE.START);
} else {
handleCaptureImage(CONST.IOU.ODOMETER_IMAGE_TYPE.START);
}
handleOdometerImagePress(CONST.IOU.ODOMETER_IMAGE_TYPE.START, hasStartOdometerImage);
}}
style={[
StyleUtils.getWidthAndHeightStyle(variables.inputHeight, variables.inputHeight),
Expand Down Expand Up @@ -639,11 +683,7 @@ function IOURequestStepDistanceOdometer({
accessibilityRole="button"
sentryLabel={CONST.SENTRY_LABEL.ODOMETER_EXPENSE.CAPTURE_IMAGE_END}
onPress={() => {
if (odometerEndImage) {
handleViewOdometerImage(CONST.IOU.ODOMETER_IMAGE_TYPE.END);
} else {
handleCaptureImage(CONST.IOU.ODOMETER_IMAGE_TYPE.END);
}
handleOdometerImagePress(CONST.IOU.ODOMETER_IMAGE_TYPE.END, hasEndOdometerImage);
}}
style={[
StyleUtils.getWidthAndHeightStyle(variables.inputHeight, variables.inputHeight),
Expand Down
9 changes: 3 additions & 6 deletions src/pages/iou/request/step/IOURequestStepOdometerImage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ import variables from '@styles/variables';
import {setMoneyRequestOdometerImage} from '@userActions/IOU';
import CONST from '@src/CONST';
import type {IOUAction, IOUType} from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {FileObject} from '@src/types/utils/Attachment';

type IOURequestStepOdometerImageProps = WithFullTransactionOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.ODOMETER_IMAGE>;

function IOURequestStepOdometerImage({
route: {
params: {action, iouType, transactionID, reportID, imageType},
params: {action, iouType, transactionID, imageType},
},
}: IOURequestStepOdometerImageProps) {
const {translate} = useLocalize();
Expand All @@ -53,11 +52,9 @@ function IOURequestStepOdometerImage({
const icon = imageType === CONST.IOU.ODOMETER_IMAGE_TYPE.START ? lazyIcons.OdometerStart : lazyIcons.OdometerEnd;
const messageHTML = `<centered-text><muted-text-label>${message}</muted-text-label></centered-text>`;

const odometerRoute = ROUTES.DISTANCE_REQUEST_CREATE_TAB_ODOMETER.getRoute(action, iouType, transactionID, reportID);

const navigateBack = useCallback(() => {
Navigation.goBack(odometerRoute);
}, [odometerRoute]);
Navigation.goBack();

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restore explicit back target for odometer image screen

Using Navigation.goBack() without a fallback breaks the odometer flow when this screen is opened from a fresh URL state (for example after browser refresh or direct deep link): there may be no odometer route in history, so back navigation returns to an unrelated root screen (or cannot go back) instead of the odometer step. The previous goBack(ROUTES.DISTANCE_REQUEST_CREATE_TAB_ODOMETER...) behavior guaranteed users returned to the form after selecting/canceling an image.

Useful? React with 👍 / 👎.

}, []);

const revokeDropBlobUrls = useCallback(() => {
for (const url of dropBlobUrlsRef.current) {
Expand Down
Loading