-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[NO QA] Create DynamicVerifyAccountPage Component (BATCH-4) #81392
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
Merged
mjasikowski
merged 15 commits into
Expensify:main
from
software-mansion-labs:collectioneur/dynamic-routes-batch-4
Feb 11, 2026
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
443ab2d
add intercept and validate dynamic suffixes
collectioneur 0ceeb40
no need to add misspelled route name, there's no entry screens
collectioneur 4ea2761
add all neccessary files to create first dynamic page
collectioneur 1a2c199
fix test
collectioneur e1c6fa7
add a guard to prevent path = undefined
collectioneur 7417586
add filename to the error text
collectioneur 045a070
Merge branch 'collectioneur/dynamic-routes-batch-3' into collectioneu…
collectioneur ba436dc
Merge branch 'main' into collectioneur/dynamic-routes-batch-3
collectioneur 4dc280a
Merge branch 'collectioneur/dynamic-routes-batch-3' into collectioneu…
collectioneur bad2c69
Merge branch 'main' into collectioneur/dynamic-routes-batch-4
collectioneur d2822ca
add and fix all suggestions left by bots
collectioneur 52f74d3
fix getStateFromPath handling dynamic routes and change imports for g…
collectioneur 0ec218a
add comment that expains why we should left options arg and remove op…
collectioneur 53e6d0f
Merge branch 'main' into collectioneur/dynamic-routes-batch-4
collectioneur 1735d03
change string screen name to const from SCREENS
collectioneur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import {useNavigationState} from '@react-navigation/native'; | ||
| import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; | ||
| import splitPathAndQuery from '@libs/Navigation/helpers/splitPathAndQuery'; | ||
| import type {DynamicRouteSuffix, Route} from '@src/ROUTES'; | ||
| import ROUTES from '@src/ROUTES'; | ||
|
|
||
| /** | ||
| * Returns the back path for a dynamic route by removing the dynamic suffix from the current URL. | ||
| * Only removes the suffix if it's the last segment of the path (ignoring trailing slashes and query parameters). | ||
| * @param dynamicRouteSuffix The dynamic route suffix to remove from the current path | ||
| * @returns The back path without the dynamic route suffix, or HOME if path is null/undefined | ||
| */ | ||
| function useDynamicBackPath(dynamicRouteSuffix: DynamicRouteSuffix): Route { | ||
| const path = useNavigationState((state) => getPathFromState(state)); | ||
|
|
||
| if (!path) { | ||
| return ROUTES.HOME; | ||
| } | ||
|
|
||
| // Remove leading slashes for consistent processing | ||
| const pathWithoutLeadingSlash = path.replace(/^\/+/, ''); | ||
|
|
||
| const [normalizedPath, query] = splitPathAndQuery(pathWithoutLeadingSlash); | ||
|
|
||
| if (normalizedPath?.endsWith(`/${dynamicRouteSuffix}`)) { | ||
| const backPathWithoutQuery = normalizedPath.slice(0, -(dynamicRouteSuffix.length + 1)); | ||
| const backPath = `${backPathWithoutQuery}${query ? `?${query}` : ''}`; | ||
|
|
||
| return backPath as Route; | ||
| } | ||
|
|
||
| // If suffix is not the last segment, return the original path | ||
| return pathWithoutLeadingSlash as Route; | ||
| } | ||
|
|
||
| export default useDynamicBackPath; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 changes: 2 additions & 3 deletions
5
src/libs/Navigation/AppNavigator/createRootStackNavigator/syncBrowserHistory/index.web.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,10 @@ | ||
| import type {ParamListBase, StackNavigationState} from '@react-navigation/native'; | ||
| import {getPathFromState} from '@react-navigation/native'; | ||
| import {linkingConfig} from '@libs/Navigation/linkingConfig'; | ||
| import getPathFromState from '@libs/Navigation/helpers/getPathFromState'; | ||
|
|
||
| function syncBrowserHistory(state: StackNavigationState<ParamListBase>) { | ||
| // We reset the URL as the browser sets it in a way that doesn't match the navigation state | ||
| // eslint-disable-next-line no-restricted-globals | ||
| history.replaceState({}, '', getPathFromState(state, linkingConfig.config)); | ||
| history.replaceState({}, '', getPathFromState(state)); | ||
| } | ||
|
|
||
| export default syncBrowserHistory; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * Extracts the last segment from a URL path, removing query parameters and trailing slashes. | ||
| * | ||
| * @param path - The URL path to extract the suffix from (can be undefined) | ||
| * @returns The last segment of the path as a string | ||
| */ | ||
| function getLastSuffixFromPath(path: string | undefined): string { | ||
| const pathWithoutParams = path?.split('?').at(0); | ||
|
|
||
| if (!pathWithoutParams) { | ||
| throw new Error('[getLastSuffixFromPath.ts] Failed to parse the path, path is empty'); | ||
| } | ||
|
|
||
| const pathWithoutTrailingSlash = pathWithoutParams.endsWith('/') ? pathWithoutParams.slice(0, -1) : pathWithoutParams; | ||
|
|
||
| const lastSuffix = pathWithoutTrailingSlash.split('/').pop() ?? ''; | ||
|
|
||
| return lastSuffix; | ||
| } | ||
|
|
||
| export default getLastSuffixFromPath; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import {findFocusedRoute, getPathFromState as RNGetPathFromState} from '@react-navigation/native'; | ||
| import type {NavigationState, PartialState} from '@react-navigation/routers'; | ||
| import {linkingConfig} from '@libs/Navigation/linkingConfig'; | ||
| import {normalizedConfigs} from '@libs/Navigation/linkingConfig/config'; | ||
| import {DYNAMIC_ROUTES} from '@src/ROUTES'; | ||
| import type {Screen} from '@src/SCREENS'; | ||
|
|
||
| type State = NavigationState | Omit<PartialState<NavigationState>, 'stale'>; | ||
|
|
||
| /** | ||
| * Checks if a screen name is a dynamic route screen | ||
| */ | ||
| function isDynamicRouteScreen(screenName: Screen): boolean { | ||
| const dynamicRouteEntries = Object.values(DYNAMIC_ROUTES); | ||
| const screenPath = normalizedConfigs[screenName]?.path; | ||
|
|
||
| if (!screenPath) { | ||
| return false; | ||
| } | ||
|
|
||
| for (const {path} of dynamicRouteEntries) { | ||
| if (screenPath === path) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| const getPathFromState = (state: State): string => { | ||
| const focusedRoute = findFocusedRoute(state); | ||
| const screenName = focusedRoute?.name ?? ''; | ||
|
|
||
| // Handle dynamic route screens that require special path that is placed in state | ||
| if (isDynamicRouteScreen(screenName as Screen) && focusedRoute?.path) { | ||
| return focusedRoute.path; | ||
| } | ||
|
|
||
| // For regular routes, use React Navigation's default path generation | ||
| const path = RNGetPathFromState(state, linkingConfig.config); | ||
|
|
||
| return path; | ||
| }; | ||
|
|
||
| export default getPathFromState; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that you’re using type assertions like
as Routeoras RoutePathbecause we’re callinggetStateFromPathwith a value typed asRoute. However, thegetStateFromPathAPI from React Navigation actually accepts a plain string.What’s your opinion on this approach? Do you think we should avoid these casts and instead handle the path as a string with proper validation before passing it in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should keep the
Routetype. It makes type assertions explicit and gives developers a hint about which routes are valid for state generation.I kept it as
Routeto maintain consistency, since our customgetStateFromPathhas acceptedRoutefrom the start. I still need to research whether switching to string is strictly better. If it turns out thatRouteisn't necessary, I suggest making that change in a follow-up PR.