Skip to content

[Performance] Fix global create scan misidentifying Search context via fallback chain #93718

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

Merged
mountiny merged 14 commits into from
Jun 18, 2026
Merged

[Performance] Fix global create scan misidentifying Search context via fallback chain #93718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f5d3620
fix: global create scan misidentifying Search context via fallback chain
JakubKorytko Jun 16, 2026
8bcd72e
address codex comment
JakubKorytko Jun 16, 2026
bbe24fc
fix: failing tests
JakubKorytko Jun 16, 2026
00dbfda
fix: nested Search query fallback for unmounted tab state
JakubKorytko Jun 17, 2026
e6999a2
fix: Search context detection from preserved tab state
JakubKorytko Jun 17, 2026
9c0f2a1
read Search params from tab params.state
JakubKorytko Jun 17, 2026
154f899
Merge branch 'main' into korytko/fix-global-create-scan
JakubKorytko Jun 17, 2026
d714b58
Merge branch 'main' into korytko/fix-global-create-scan
JakubKorytko Jun 17, 2026
d426b52
Move isRecord to ObjectUtils.ts
mhawryluk Jun 18, 2026
e85fa35
Use the new improved logic in getNavigateAfterCreateSearchNavigatorState
mhawryluk Jun 18, 2026
24ff2be
Modify NavigationRouteLike type and remove unnecessary accessor utils
mhawryluk Jun 18, 2026
223695f
Make tabState a factory function
mhawryluk Jun 18, 2026
d7e0f34
Update params type
mhawryluk Jun 18, 2026
9491ef3
Add jsdoc and more tests
mhawryluk Jun 18, 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
5 changes: 1 addition & 4 deletions src/libs/ImportOnyxStateUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type OnyxState from '@src/types/onyx/OnyxState';
import type CollectionDataSet from '@src/types/utils/CollectionDataSet';
import {clearOnyxStateBeforeImport, importOnyxCollectionState, importOnyxRegularState} from './actions/ImportOnyxState';
import {isRecord} from './ObjectUtils';

// List of Onyx keys from the .txt file we want to keep for the local override
const keysToOmit = [ONYXKEYS.ACTIVE_CLIENTS, ONYXKEYS.FREQUENTLY_USED_EMOJIS, ONYXKEYS.NETWORK, ONYXKEYS.CREDENTIALS, ONYXKEYS.PREFERRED_THEME, ...Object.values(ONYXKEYS.DERIVED)];

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && !Array.isArray(value) && value !== null;
}

function transformNumericKeysToArray(data: UnknownRecord): UnknownRecord | unknown[] {
const dataCopy = cloneDeep(data);
if (!isRecord(dataCopy)) {
Expand Down
25 changes: 21 additions & 4 deletions src/libs/Navigation/helpers/getTopmostFullScreenRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {navigationRef} from '@libs/Navigation/Navigation';
import {getPreservedNavigatorState} from '@libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState';
import navigationRef from '@libs/Navigation/navigationRef';
import type {NavigationRoute, RootNavigatorParamList, State} from '@libs/Navigation/types';
import NAVIGATORS from '@src/NAVIGATORS';

/**
* Returns the active tab route of the topmost TAB_NAVIGATOR in the root navigation state.
* Use this to determine which full-screen tab (Search, Inbox, etc.) is currently focused.
*
* Fallback chain: live tab state → preserved state.
*/
function getTopmostFullScreenRoute(): NavigationRoute | undefined {
const rootState = navigationRef.getRootState() as State<RootNavigatorParamList>;
Expand All @@ -14,11 +17,25 @@ function getTopmostFullScreenRoute(): NavigationRoute | undefined {
}

const topmostTabNavigatorRoute = rootState.routes.findLast((route) => route.name === NAVIGATORS.TAB_NAVIGATOR);
if (!topmostTabNavigatorRoute?.state) {
if (!topmostTabNavigatorRoute) {
return undefined;
}
const index = topmostTabNavigatorRoute.state.index ?? 0;
return topmostTabNavigatorRoute.state.routes?.at(index);

const liveState = topmostTabNavigatorRoute.state;
const liveRoute = liveState ? liveState.routes?.at(liveState.index ?? 0) : undefined;
if (liveRoute) {
return liveRoute;
}

const preservedState = topmostTabNavigatorRoute.key ? getPreservedNavigatorState(topmostTabNavigatorRoute.key) : undefined;
if (preservedState) {
const preservedRoute = preservedState.routes?.at(preservedState.index ?? 0);
if (preservedRoute) {
return preservedRoute;
Comment thread
JakubKorytko marked this conversation as resolved.
}
}

return undefined;
}

export default getTopmostFullScreenRoute;
22 changes: 15 additions & 7 deletions src/libs/Navigation/helpers/navigateAfterExpenseCreate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {addPendingNewTransactionIDs} from '@libs/actions/IOU/PendingNewTransactions';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import Log from '@libs/Log';
import {getPreservedNavigatorState} from '@libs/Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import {buildCannedSearchQuery, getCurrentSearchQueryJSON} from '@libs/SearchQueryUtils';
import {setPendingSubmitFollowUpAction} from '@libs/telemetry/submitFollowUpAction';
Expand All @@ -22,6 +23,14 @@ type NavigateAfterExpenseCreateParams = {
shouldNavigate?: boolean;
};

function getNavigateAfterCreateSearchNavigatorState() {
const rootState = navigationRef.getRootState();
const tabNavigatorRoute = rootState?.routes?.findLast((route) => route.name === NAVIGATORS.TAB_NAVIGATOR);
const tabState = tabNavigatorRoute?.state ?? (tabNavigatorRoute?.key ? getPreservedNavigatorState(tabNavigatorRoute.key) : undefined);
const searchNavigatorRoute = tabState?.routes?.findLast((route) => route.name === NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR);
return searchNavigatorRoute?.state ?? (searchNavigatorRoute?.key ? getPreservedNavigatorState(searchNavigatorRoute.key) : undefined);
}

Comment on lines +26 to +33

@ikevin127 ikevin127 Jun 17, 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.

⚠️ Code Quality

Strength: Extraction of getNavigateAfterCreateSearchNavigatorState into a named function improves readability and testability.

Issue: This new helper does not use the improved getCurrentSearchQueryJSON / getTopmostFullScreenRoute logic. It directly accesses navigationRef.getRootState().

This function skips the new fallback chain entirely. If the SEARCH_FULLSCREEN_NAVIGATOR is inside a TAB_NAVIGATOR but its .state is missing (the scenario this PR fixes), this function returns undefined, and alreadyOnSearchRoot becomes false. This means the telemetry will record NAVIGATE_TO_SEARCH instead of DISMISS_MODAL_ONLY, which may affect performance metrics.

Bug potential: If the SEARCH_FULLSCREEN_NAVIGATOR is not at the root level but nested inside TAB_NAVIGATOR, rootState.routes.findLast(...) will not find it. This is the pre-existing behavior, but the PR doesn't address it. The helper should probably use the same state-finding logic as getCurrentSearchQueryJSON.

/**
* Helper to navigate after an expense is created in order to standardize the post‑creation experience
* when creating an expense from the global create button.
Expand Down Expand Up @@ -61,15 +70,14 @@ function navigateAfterExpenseCreate({

// When already on Search ROOT with the same type (expense vs invoice), we navigate to the same screen (no-op or refresh); record as dismiss_modal_only.
// When on another Search sub-tab (e.g. Chats), or on Search with a different type (e.g. on Invoice, submitting expense), record as navigate_to_search.
const rootState = navigationRef.getRootState();
const searchNavigatorRoute = rootState?.routes?.findLast((route) => route.name === NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR);
const lastSearchRoute = searchNavigatorRoute?.state?.routes?.at(-1);
const alreadyOnSearchRoot = isSearchTopmostFullScreenRoute() && lastSearchRoute?.name === SCREENS.SEARCH.ROOT;
const searchNavigatorState = getNavigateAfterCreateSearchNavigatorState();
const lastSearchRoute = searchNavigatorState?.routes?.at(-1);
const isSearchTopmost = isSearchTopmostFullScreenRoute();
const alreadyOnSearchRoot = isSearchTopmost && lastSearchRoute?.name === SCREENS.SEARCH.ROOT;
const currentSearchQueryJSON = alreadyOnSearchRoot ? getCurrentSearchQueryJSON() : undefined;
const isSameSearchType = currentSearchQueryJSON?.type === type;
setPendingSubmitFollowUpAction(
alreadyOnSearchRoot && isSameSearchType ? CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.DISMISS_MODAL_ONLY : CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.NAVIGATE_TO_SEARCH,
);
const followUpAction = alreadyOnSearchRoot && isSameSearchType ? CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.DISMISS_MODAL_ONLY : CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.NAVIGATE_TO_SEARCH;
setPendingSubmitFollowUpAction(followUpAction);

const queryString = buildCannedSearchQuery({type});
const navigateToSearch = () => {
Expand Down
6 changes: 5 additions & 1 deletion src/libs/ObjectUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ function filterObject<TObject extends Record<string, unknown>>(obj: TObject, pre
}, {} as TObject);
}

export {shallowCompare, filterObject};
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}

export {shallowCompare, filterObject, isRecord};
112 changes: 101 additions & 11 deletions src/libs/SearchQueryUtils.ts
View file
Open in desktop

@mountiny mountiny Jun 18, 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.

Even if these methods are simple, lets create unit tests for them to make sure we gate from some unexpected changes to them

@mhawryluk mhawryluk Jun 18, 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.

I added tests for getLastRouteByName, getRoutes, getParamsState and isSearchRootParams. I think the rest should be covered by the getCurrentSearchQueryJSON tests already

Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {validateAmount} from './MoneyRequestUtils';
import {getPreservedNavigatorState} from './Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState';
import navigationRef from './Navigation/navigationRef';
import type {SearchFullscreenNavigatorParamList} from './Navigation/types';
import {isRecord} from './ObjectUtils';
import {getDisplayNameOrDefault, getPersonalDetailByEmail} from './PersonalDetailsUtils';
import {getCleanedTagName} from './PolicyUtils';
import {getReportName} from './ReportNameUtils';
Expand All @@ -56,6 +57,17 @@ import {hashText} from './UserUtils';
import {isValidDate} from './ValidationUtils';

type FilterKeys = keyof typeof CONST.SEARCH.SYNTAX_FILTER_KEYS;
type SearchRootParams = SearchFullscreenNavigatorParamList[typeof SCREENS.SEARCH.ROOT];
type NavigationRouteLike = {
/** Unique React Navigation route identifier. */
key?: string;
/** Screen name as registered in the navigator. */
name?: string;
/** Screen-specific params passed to the route. */
params?: Record<string, unknown>;
/** Nested navigator state, if this route is itself a navigator. */
state?: unknown;
};
Comment on lines +61 to +70

@ikevin127 ikevin127 Jun 17, 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.

🟡 Issue: NavigationRouteLike type uses unknown where string would be more precise

Impact: Type safety is weakened. getRouteKey has to do typeof route.key === 'string' because the type allows unknown which defeats the purpose of having a typed structure.

Recommendation:

type NavigationRouteLike = {
    key?: string;
    name?: string;
    params?: Record<string, unknown>;
    state?: unknown;
};

Then getRouteKey simplifies to:

function getRouteKey(route: NavigationRouteLike | unknown): string | undefined {
    return isRecord(route) && typeof route.key === 'string' ? route.key : undefined;
}

(Kept the runtime check since route is unknown at call site.)

@mhawryluk mhawryluk Jun 18, 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.

I updated the type and since we always pass the route which is NavigationRouteLike | undefined to these accessor functions, there's no point in checking for isRecord and typeof anymore, so I just removed them and inlined the access instead

Comment on lines +61 to +70

@mountiny mountiny Jun 18, 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.

docs

@mhawryluk mhawryluk Jun 18, 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.

added!


// This map contains chars that match each operator
const operatorToCharMap = {
Expand Down Expand Up @@ -1916,29 +1928,103 @@ function getQueryWithUpdatedValues(query: string, shouldSkipAmountConversion = f
return buildSearchQueryString(standardizedQuery);
}

function isSearchRootParams(params: unknown): params is SearchRootParams {
return (
!!params &&
typeof params === 'object' &&
'q' in params &&
typeof params.q === 'string' &&
(!('rawQuery' in params) || params.rawQuery === undefined || typeof params.rawQuery === 'string')
);
}

function isUnknownArray(value: unknown): value is unknown[] {
return Array.isArray(value);
}
Comment on lines +1941 to +1943

@ikevin127 ikevin127 Jun 17, 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.

🟡 Issue: isUnknownArray is just a type-guard wrapper around Array.isArray

This adds function call overhead for minimal value. TypeScript's Array.isArray already narrows to unknown[] when called on unknown. Consider inlining it or keeping it only if strict lint rules require the guard.

@mhawryluk mhawryluk Jun 18, 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.

doesn't work, Array.isArray makes the type any[], not unknown[]


function getParamsState(params: unknown): unknown {
return isRecord(params) ? params.state : undefined;
}

function getRoutes(state: unknown): unknown[] | undefined {
if (!isRecord(state) || !isUnknownArray(state.routes)) {
return undefined;
}
return state.routes;
}

function getLastRouteByName(state: unknown, routeName: string): NavigationRouteLike | undefined {
const routes = getRoutes(state);
const route = routes?.findLast((candidate) => isRecord(candidate) && candidate.name === routeName);
return isRecord(route) ? route : undefined;
}

function getSearchRootParamsFromNestedNavigatorParams(params: unknown): SearchRootParams | undefined {
if (!params || typeof params !== 'object') {
return undefined;
}

const screen = 'screen' in params ? params.screen : undefined;
const nestedParams = 'params' in params ? params.params : undefined;
if (screen === SCREENS.SEARCH.ROOT) {
return isSearchRootParams(nestedParams) ? nestedParams : undefined;
}

if (screen === NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR) {
return getSearchRootParamsFromNestedNavigatorParams(nestedParams);
}

return undefined;
}

function getSearchRootParamsFromSearchNavigatorState(state: unknown): SearchRootParams | undefined {
const searchRootRoute = getLastRouteByName(state, SCREENS.SEARCH.ROOT);
const searchRootParams = searchRootRoute?.params;
return isSearchRootParams(searchRootParams) ? searchRootParams : undefined;
}

function getSearchRootParamsFromTabState(state: unknown): SearchRootParams | undefined {
const searchNavigatorRoute = getLastRouteByName(state, NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR);
return getSearchRootParamsFromNestedNavigatorParams(searchNavigatorRoute?.params) ?? getSearchRootParamsFromSearchNavigatorState(searchNavigatorRoute?.state);
}

function getSearchQueryJSONFromRouteParams(params: unknown) {
if (!isSearchRootParams(params)) {
return undefined;
}

return buildSearchQueryJSON(params.q, params.rawQuery);
}

function getCurrentSearchQueryJSON() {
const rootState = navigationRef.getRootState();
const lastTabNavigator = rootState?.routes?.findLast((route) => route.name === NAVIGATORS.TAB_NAVIGATOR);
const lastSearchNavigator = lastTabNavigator?.state?.routes?.findLast((route) => route.name === NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR);
const tabStateFromParams = getParamsState(lastTabNavigator?.params);
const tabState = lastTabNavigator?.state ?? (lastTabNavigator?.key ? getPreservedNavigatorState(lastTabNavigator.key) : undefined) ?? tabStateFromParams;
const lastSearchNavigator = getLastRouteByName(tabState, NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR);
let lastSearchNavigatorState = lastSearchNavigator?.state;
if (!lastSearchNavigatorState) {
lastSearchNavigatorState = lastSearchNavigator?.key ? getPreservedNavigatorState(lastSearchNavigator?.key) : undefined;
lastSearchNavigatorState = lastSearchNavigator?.key ? getPreservedNavigatorState(lastSearchNavigator.key) : undefined;
}

const nestedSearchRootParams =
getSearchRootParamsFromNestedNavigatorParams(lastSearchNavigator?.params) ??
getSearchRootParamsFromNestedNavigatorParams(lastTabNavigator?.params) ??
getSearchRootParamsFromTabState(tabStateFromParams);

// When the SearchFullscreenNavigator has never been mounted (e.g. lazy tab not yet visited),
// neither .state nor the preserved state map will have an entry. Fall back to the default
// query that the navigator would use as its initialParams.
// neither .state nor the preserved state map will have an entry. Use nested route params when
// React Navigation provided them, otherwise fall back to the default initialParams query.
if (!lastSearchNavigatorState) {
const nestedQueryJSON = getSearchQueryJSONFromRouteParams(nestedSearchRootParams);
if (nestedQueryJSON) {
return nestedQueryJSON;
}
return buildSearchQueryJSON(buildSearchQueryString());
}

const lastSearchRoute = lastSearchNavigatorState.routes.findLast((route) => route.name === SCREENS.SEARCH.ROOT);
if (!lastSearchRoute?.params) {
return;
}

const {q: searchParams, rawQuery} = lastSearchRoute.params as SearchFullscreenNavigatorParamList[typeof SCREENS.SEARCH.ROOT];
const queryJSON = buildSearchQueryJSON(searchParams, rawQuery);
const lastSearchRoute = getLastRouteByName(lastSearchNavigatorState, SCREENS.SEARCH.ROOT);
const queryJSON = getSearchQueryJSONFromRouteParams(lastSearchRoute?.params);
if (!queryJSON) {
return;
}
Expand Down Expand Up @@ -2240,6 +2326,10 @@ export {
getDateModifierTitle,
applyContainsOperatorToTextFields,
serializeQueryJSONForBackend,
getLastRouteByName,
getParamsState,
getRoutes,
isSearchRootParams,
};

export type {BuildUserReadableQueryStringParams};
Expand Down
14 changes: 9 additions & 5 deletions tests/ui/TimeExpenseConfirmationTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,15 @@ jest.mock('@components/ProductTrainingContext', () => ({
jest.mock('@src/hooks/useResponsiveLayout');

jest.mock('@libs/Navigation/navigationRef', () => ({
getCurrentRoute: jest.fn(() => ({
name: 'Money_Request_Step_Confirmation',
params: {},
})),
getState: jest.fn(() => ({})),
__esModule: true,
default: {
getCurrentRoute: jest.fn(() => ({
name: 'Money_Request_Step_Confirmation',
params: {},
})),
getState: jest.fn(() => ({})),
getRootState: jest.fn(() => ({routes: []})),
},
}));

jest.mock('@libs/Navigation/Navigation', () => {
Expand Down
14 changes: 9 additions & 5 deletions tests/ui/components/IOURequestStepConfirmationPageTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,15 @@ jest.mock('@libs/getCurrentPosition');
jest.mock('@libs/getIsNarrowLayout', () => jest.fn(() => false));

jest.mock('@libs/Navigation/navigationRef', () => ({
getCurrentRoute: jest.fn(() => ({
name: 'Money_Request_Step_Confirmation',
params: {},
})),
getState: jest.fn(() => ({})),
__esModule: true,
default: {
getCurrentRoute: jest.fn(() => ({
name: 'Money_Request_Step_Confirmation',
params: {},
})),
getState: jest.fn(() => ({})),
getRootState: jest.fn(() => ({routes: []})),
},
}));

jest.mock('@libs/Navigation/Navigation', () => {
Expand Down
17 changes: 15 additions & 2 deletions tests/unit/Navigation/getTopmostFullScreenRouteTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import NAVIGATORS from '@src/NAVIGATORS';

const mockGetRootState = jest.fn();

jest.mock('@libs/Navigation/Navigation', () => ({
navigationRef: {
jest.mock('@libs/Navigation/navigationRef', () => ({
__esModule: true,
default: {
getRootState: () => mockGetRootState() as unknown,
},
}));
Expand Down Expand Up @@ -33,6 +34,18 @@ describe('getTopmostFullScreenRoute', () => {
expect(getTopmostFullScreenRoute()).toBeUndefined();
});

it('does not use tab screen params as focused state', () => {
mockGetRootState.mockReturnValue({
routes: [
{
name: NAVIGATORS.TAB_NAVIGATOR,
params: {screen: NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR},
},
],
});
expect(getTopmostFullScreenRoute()).toBeUndefined();
});

it('returns the focused tab route based on state.index', () => {
const reportsRoute = {name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR};
const searchRoute = {name: NAVIGATORS.SEARCH_FULLSCREEN_NAVIGATOR};
Expand Down
Loading
Loading