Skip to content

Chore: Remove getStateFromPath patch #65072

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 2 commits into from
Jul 4, 2025
Merged

Chore: Remove getStateFromPath patch #65072

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
c6cfc28
Remove getStateFromPath patch
staszekscp Jun 27, 2025
53600d2
Update details.md for react-navigation patches
staszekscp Jun 27, 2025
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
361 changes: 0 additions & 361 deletions patches/react-navigation/@react-navigation+core+7.10.0+002+getStateFromPath.patch
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions patches/react-navigation/details.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,6 @@
- PR Introducing Patch: [#22437](https://github.com/Expensify/App/pull/22437)
- PR Updating Patch: [#33280](https://github.com/Expensify/App/pull/33280) [#37421](https://github.com/Expensify/App/pull/37421) [#49539](https://github.com/Expensify/App/pull/49539) [#64155](https://github.com/Expensify/App/pull/64155)

### [@react-navigation+core+7.10.0+002+getStateFromPath.patch](@react-navigation+core+7.10.0+002+getStateFromPath.patch)
- Reason: Make sure navigation state props retrieved from the path are available at all nesting levels to avoid undefined state.
- Upstream PR/issue: N/A
- E/App issue: [#48150](https://github.com/Expensify/App/issues/48150)
- PR Introducing Patch: [#48151](https://github.com/Expensify/App/pull/48151)
- PR Updating Patch: [#64155](https://github.com/Expensify/App/pull/64155)

### [@react-navigation+stack+7.3.3+003+fix-invisible-sidebar-safari.patch](@react-navigation+stack+7.3.3+003+fix-invisible-sidebar-safari.patch)
- Reason: Sidebar is invisible on the safari
- Upstream PR/issue: N/A
Expand Down
6 changes: 3 additions & 3 deletions src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,17 +890,17 @@ const ROUTES = {
getRoute: (iouType: IOUType, iouRequestType: IOURequestType) => `start/${iouType as string}/${iouRequestType as string}` as const,
},
MONEY_REQUEST_CREATE_TAB_DISTANCE: {
route: ':action/:iouType/start/:transactionID/:reportID/distance/:backToReport?',
route: 'distance/:backToReport?',
getRoute: (action: IOUAction, iouType: IOUType, transactionID: string, reportID: string, backToReport?: string) =>
`create/${iouType as string}/start/${transactionID}/${reportID}/distance/${backToReport ?? ''}` as const,
},
MONEY_REQUEST_CREATE_TAB_MANUAL: {
route: ':action/:iouType/start/:transactionID/:reportID/manual/:backToReport?',
route: 'manual/:backToReport?',
getRoute: (action: IOUAction, iouType: IOUType, transactionID: string, reportID: string, backToReport?: string) =>
`${action as string}/${iouType as string}/start/${transactionID}/${reportID}/manual/${backToReport ?? ''}` as const,
},
MONEY_REQUEST_CREATE_TAB_SCAN: {
route: ':action/:iouType/start/:transactionID/:reportID/scan/:backToReport?',
route: 'scan/:backToReport?',
Comment on lines 892 to +903

@allgandalf allgandalf Jul 2, 2025

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.

@staszekscp I don't understand this change, This is a major route change imo, did we get this approved? so from now for manual request instead of the route : create/create/start/1/3269280911335422/manual, we will have a simple manual route in the URL? , is this assumption correct?, also is this the same for quick actions, what are the steps exactly to test this url change?

@adamgrzybowski adamgrzybowski Jul 3, 2025

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.

The route field is used in the linking config. The difference now is that we don't use path: exact. We need to do this because some parameters need to be passed to the screen and some to its parent navigator.

You can see that this part :action/:iouType/start/:transactionID/:reportID/distance is common for all screens. We used this path for the parent navigator as path exact and added the rest in the config for its screens, like distance/:backToReport?

You can see that getRoute still has the whole create/${iouType as string}/start/${transactionID}/${reportID}/distance/${backToReport ?? ''}

TLDR: The visible URL in the browser will stay the same. We only changed where the params will be available from useRoute()

@mountiny mountiny Jul 4, 2025

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.

@adamgrzybowski So to confirm, I believe we user the full create/${iouType as string}/start/${transactionID}/${reportID}/distance/${backToReport ?? ''} in some messaging to deeplink to the flow. That will still work?

@mountiny mountiny Jul 4, 2025

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.

Checked and its using /start/submit/distance so should be ok

getRoute: (action: IOUAction, iouType: IOUType, transactionID: string, reportID: string, backToReport?: string) =>
`create/${iouType as string}/start/${transactionID}/${reportID}/scan/${backToReport ?? ''}` as const,
},
Expand Down
Loading
Loading