Skip to content

Commit

Permalink
Merge pull request #44531 from software-mansion-labs/fix-too-many-par…
Browse files Browse the repository at this point in the history
…ams-passed-to-screens

Pick only params from route before passing them to screen
  • Loading branch information
mountiny authored Jul 19, 2024
2 parents 54fdb24 + 6c69b9e commit f1b29ee
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const ROUTES = {

ALL_SETTINGS: 'all-settings',

SEARCH: {
SEARCH_CENTRAL_PANE: {
route: '/search/:query',
getRoute: (searchQuery: SearchQuery, queryParams?: AuthScreensParamList['Search_Central_Pane']) => {
const {sortBy, sortOrder} = queryParams ?? {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
if (selectedTab === SCREENS.SEARCH.BOTTOM_TAB) {
return;
}
interceptAnonymousUser(() => Navigation.navigate(ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL)));
interceptAnonymousUser(() => Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL)));
}}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.search')}
Expand Down
16 changes: 7 additions & 9 deletions src/libs/Navigation/getTopmostFullScreenRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,16 @@ function getTopmostFullScreenRoute(state: State<RootStackParamList>): Navigation
return;
}

if (!!topmostFullScreenRoute.params && 'screen' in topmostFullScreenRoute.params) {
return {name: topmostFullScreenRoute.params.screen as FullScreenName, params: topmostFullScreenRoute.params.params};
}
if (topmostFullScreenRoute.state) {
// There will be at least one route in the fullscreen navigator.
const {name, params} = topmostFullScreenRoute.state.routes.at(-1) as NavigationPartialRoute<FullScreenName>;

if (!topmostFullScreenRoute.state) {
return;
return {name, params};
}

// There will be at least one route in the fullscreen navigator.
const {name, params} = topmostFullScreenRoute.state.routes.at(-1) as NavigationPartialRoute<FullScreenName>;

return {name, params};
if (!!topmostFullScreenRoute.params && 'screen' in topmostFullScreenRoute.params) {
return {name: topmostFullScreenRoute.params.screen as FullScreenName, params: topmostFullScreenRoute.params.params};
}
}

export default getTopmostFullScreenRoute;
30 changes: 28 additions & 2 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
/* eslint-disable @typescript-eslint/naming-convention */
import type {LinkingOptions} from '@react-navigation/native';
import type {RootStackParamList} from '@navigation/types';
import NAVIGATORS from '@src/NAVIGATORS';
import ROUTES from '@src/ROUTES';
import type {Screen} from '@src/SCREENS';
import SCREENS from '@src/SCREENS';
import type {RouteConfig} from './createNormalizedConfigs';
import createNormalizedConfigs from './createNormalizedConfigs';

// Moved to a separate file to avoid cyclic dependencies.
const config: LinkingOptions<RootStackParamList>['config'] = {
Expand Down Expand Up @@ -51,7 +53,7 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
exact: true,
},
[SCREENS.SETTINGS.WORKSPACES]: ROUTES.SETTINGS_WORKSPACES,
[SCREENS.SEARCH.CENTRAL_PANE]: ROUTES.SEARCH.route,
[SCREENS.SEARCH.CENTRAL_PANE]: ROUTES.SEARCH_CENTRAL_PANE.route,
[SCREENS.SETTINGS.SAVE_THE_WORLD]: ROUTES.SETTINGS_SAVE_THE_WORLD,
[SCREENS.SETTINGS.SUBSCRIPTION.ROOT]: ROUTES.SETTINGS_SUBSCRIPTION,

Expand Down Expand Up @@ -1045,4 +1047,28 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
},
};

const normalizedConfigs = Object.keys(config.screens)
.map((key) =>
createNormalizedConfigs(
key,
config.screens,
[],
config.initialRouteName
? [
{
initialRouteName: config.initialRouteName,
parentScreens: [],
},
]
: [],
[],
),
)
.flat()
.reduce((acc, route) => {
acc[route.screen as Screen] = route;
return acc;
}, {} as Record<Screen, RouteConfig>);

export {normalizedConfigs};
export default config;
135 changes: 135 additions & 0 deletions src/libs/Navigation/linkingConfig/createNormalizedConfigs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment */

/* eslint-disable @typescript-eslint/default-param-last */

/* eslint-disable @typescript-eslint/prefer-nullish-coalescing */

/* eslint-disable no-param-reassign */

/* eslint-disable @typescript-eslint/no-unsafe-argument */

/* eslint-disable @typescript-eslint/no-non-null-assertion */

/* eslint-disable @typescript-eslint/no-unsafe-member-access */

/* eslint-disable @typescript-eslint/no-explicit-any */

/* eslint-disable @typescript-eslint/ban-types */
// THOSE FUNCTIONS ARE COPIED FROM react-navigation/core IN ORDER TO AVOID PATCHING
// THAT'S THE REASON WHY ESLINT IS DISABLED
import type {PathConfigMap} from '@react-navigation/native';

type ParseConfig = Record<string, (value: string) => any>;

type RouteConfig = {
screen: string;
regex?: RegExp;
path: string;
pattern: string;
routeNames: string[];
parse?: ParseConfig;
};

type InitialRouteConfig = {
initialRouteName: string;
parentScreens: string[];
};

const joinPaths = (...paths: string[]): string =>
([] as string[])
.concat(...paths.map((p) => p.split('/')))
.filter(Boolean)
.join('/');

const createConfigItem = (screen: string, routeNames: string[], pattern: string, path: string, parse?: ParseConfig): RouteConfig => {
// Normalize pattern to remove any leading, trailing slashes, duplicate slashes etc.
pattern = pattern.split('/').filter(Boolean).join('/');

const regex = pattern
? new RegExp(
`^(${pattern
.split('/')
.map((it) => {
if (it.startsWith(':')) {
return `(([^/]+\\/)${it.endsWith('?') ? '?' : ''})`;
}
return `${it === '*' ? '.*' : escape(it)}\\/`;
})
.join('')})`,
)
: undefined;

return {
screen,
regex,
pattern,
path,
// The routeNames array is mutated, so copy it to keep the current state
routeNames: [...routeNames],
parse,
};
};

const createNormalizedConfigs = (
screen: string,
routeConfig: PathConfigMap<object>,
routeNames: string[] = [],
initials: InitialRouteConfig[],
parentScreens: string[],
parentPattern?: string,
): RouteConfig[] => {
const configs: RouteConfig[] = [];

routeNames.push(screen);

parentScreens.push(screen);

// @ts-expect-error: we can't strongly typecheck this for now
const config = routeConfig[screen];

if (typeof config === 'string') {
// If a string is specified as the value of the key(e.g. Foo: '/path'), use it as the pattern
const pattern = parentPattern ? joinPaths(parentPattern, config) : config;

configs.push(createConfigItem(screen, routeNames, pattern, config));
} else if (typeof config === 'object') {
let pattern: string | undefined;

// if an object is specified as the value (e.g. Foo: { ... }),
// it can have `path` property and
// it could have `screens` prop which has nested configs
if (typeof config.path === 'string') {
if (config.exact && config.path === undefined) {
throw new Error("A 'path' needs to be specified when specifying 'exact: true'. If you don't want this screen in the URL, specify it as empty string, e.g. `path: ''`.");
}

pattern = config.exact !== true ? joinPaths(parentPattern || '', config.path || '') : config.path || '';

configs.push(createConfigItem(screen, routeNames, pattern!, config.path, config.parse));
}

if (config.screens) {
// property `initialRouteName` without `screens` has no purpose
if (config.initialRouteName) {
initials.push({
initialRouteName: config.initialRouteName,
parentScreens,
});
}

Object.keys(config.screens).forEach((nestedConfig) => {
const result = createNormalizedConfigs(nestedConfig, config.screens as PathConfigMap<object>, routeNames, initials, [...parentScreens], pattern ?? parentPattern);

configs.push(...result);
});
}
}

routeNames.pop();

return configs;
};

export type {RouteConfig};
export default createNormalizedConfigs;
24 changes: 17 additions & 7 deletions src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {NavigationState, PartialState, Route} from '@react-navigation/native';
import {findFocusedRoute, getStateFromPath} from '@react-navigation/native';
import pick from 'lodash/pick';
import type {TupleToUnion} from 'type-fest';
import {isAnonymousUser} from '@libs/actions/Session';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
Expand All @@ -10,9 +11,10 @@ import * as ReportConnection from '@libs/ReportConnection';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Screen} from '@src/SCREENS';
import SCREENS from '@src/SCREENS';
import CENTRAL_PANE_TO_RHP_MAPPING from './CENTRAL_PANE_TO_RHP_MAPPING';
import config from './config';
import config, {normalizedConfigs} from './config';
import extractPolicyIDsFromState from './extractPolicyIDsFromState';
import FULL_SCREEN_TO_RHP_MAPPING from './FULL_SCREEN_TO_RHP_MAPPING';
import getMatchingBottomTabRouteForState from './getMatchingBottomTabRouteForState';
Expand Down Expand Up @@ -94,6 +96,14 @@ function createFullScreenNavigator(route?: NavigationPartialRoute<FullScreenName
};
}

function getParamsFromRoute(screenName: string): string[] {
const routeConfig = normalizedConfigs[screenName as Screen];

const route = routeConfig.pattern;

return route.match(/(?<=[:?&])(\w+)(?=[/=?&]|$)/g) ?? [];
}

// This function will return CentralPaneNavigator route or FullScreenNavigator route.
function getMatchingRootRouteForRHPRoute(route: NavigationPartialRoute): NavigationPartialRoute<CentralPaneName | typeof NAVIGATORS.FULL_SCREEN_NAVIGATOR> | undefined {
// Check for backTo param. One screen with different backTo value may need diferent screens visible under the overlay.
Expand Down Expand Up @@ -127,18 +137,18 @@ function getMatchingRootRouteForRHPRoute(route: NavigationPartialRoute): Navigat
// Check for CentralPaneNavigator
for (const [centralPaneName, RHPNames] of Object.entries(CENTRAL_PANE_TO_RHP_MAPPING)) {
if (RHPNames.includes(route.name)) {
const params = {...route.params};
if (centralPaneName === SCREENS.SEARCH.CENTRAL_PANE) {
delete (params as Record<string, string | undefined>)?.reportID;
}
return {name: centralPaneName as CentralPaneName, params};
const paramsFromRoute = getParamsFromRoute(centralPaneName);

return {name: centralPaneName as CentralPaneName, params: pick(route.params, paramsFromRoute)};
}
}

// Check for FullScreenNavigator
for (const [fullScreenName, RHPNames] of Object.entries(FULL_SCREEN_TO_RHP_MAPPING)) {
if (RHPNames.includes(route.name)) {
return createFullScreenNavigator({name: fullScreenName as FullScreenName, params: route.params});
const paramsFromRoute = getParamsFromRoute(fullScreenName);

return createFullScreenNavigator({name: fullScreenName as FullScreenName, params: pick(route.params, paramsFromRoute)});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/switchPolicyID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default function switchPolicyID(navigation: NavigationContainerRef<RootSt
// Here's the configuration: src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx
const isOpeningSearchFromBottomTab = !route && topmostCentralPaneRoute?.name === SCREENS.SEARCH.CENTRAL_PANE;
if (isOpeningSearchFromBottomTab) {
newPath = ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL);
newPath = ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL);
}
const stateFromPath = getStateFromPath(newPath as Route) as PartialState<NavigationState<RootStackParamList>>;
const action: StackNavigationAction = getActionFromState(stateFromPath, linkingConfig.config);
Expand Down
8 changes: 4 additions & 4 deletions src/pages/Search/SearchFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,25 @@ function SearchFilters({query}: SearchFiltersProps) {
title: translate('common.expenses'),
query: CONST.SEARCH.TAB.ALL,
icon: Expensicons.Receipt,
route: ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL),
route: ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL),
},
{
title: translate('common.shared'),
query: CONST.SEARCH.TAB.SHARED,
icon: Expensicons.Send,
route: ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.SHARED),
route: ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.SHARED),
},
{
title: translate('common.drafts'),
query: CONST.SEARCH.TAB.DRAFTS,
icon: Expensicons.Pencil,
route: ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.DRAFTS),
route: ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.DRAFTS),
},
{
title: translate('common.finished'),
query: CONST.SEARCH.TAB.FINISHED,
icon: Expensicons.CheckCircle,
route: ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.FINISHED),
route: ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.FINISHED),
},
];
const activeItemIndex = filterItems.findIndex((item) => item.query === query);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function SearchPage({route}: SearchPageProps) {
const query = rawQuery as SearchQuery;
const isValidQuery = Object.values(CONST.SEARCH.TAB).includes(query);

const handleOnBackButtonPress = () => Navigation.goBack(ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL));
const handleOnBackButtonPress = () => Navigation.goBack(ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL));

// On small screens this page is not displayed, the configuration is in the file: src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.tsx
// To avoid calling hooks in the Search component when this page isn't visible, we return null here.
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchPageBottomTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function SearchPageBottomTab() {

const isValidQuery = Object.values(CONST.SEARCH.TAB).includes(query);

const handleOnBackButtonPress = () => Navigation.goBack(ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL));
const handleOnBackButtonPress = () => Navigation.goBack(ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL));

return (
<ScreenWrapper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ function CardSection() {
title={translate('subscription.cardSection.viewPaymentHistory')}
titleStyle={styles.textStrong}
style={styles.mt5}
onPress={() => Navigation.navigate(ROUTES.SEARCH.getRoute(CONST.SEARCH.TAB.ALL))}
onPress={() => Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute(CONST.SEARCH.TAB.ALL))}
hoverAndPressStyle={styles.hoveredComponentBG}
/>
)}
Expand Down

0 comments on commit f1b29ee

Please sign in to comment.