Skip to content

Commit

Permalink
[router]: ensure navigation events are fired at correct target (#27485)
Browse files Browse the repository at this point in the history
# Why

Expo Router was incorrectly firing events with the incorrect navigator
target. This would cause situations where the event was ignored or the
state in correct.

The result of this was a sublty incorrect routing state that was
- causing incorrect back history
- navigation state was at the incorrect level in the hierarchy.

# How

I rewrote `getNavigateAction` to get the action in a single pass.
Previously we were getting the action, and then refining the action
based up on the parent navigator.

This approach could have worked, but the structure of the navigation
action is, well, weird. This made converting an action into the targeted
action error prone.

The new approach is to first identify the target, and then build the
correct navigation action.

# Test Plan

The fixed heirarachy (and incorrect back behaviour) can be demonstrated
by [this
test](https://github.com/expo/expo/blob/0dd5fd0bea34c5d8faeb6b4ada8ea82cfe220c39/packages/expo-router/src/__tests__/push.test.ios.tsx#L12).
Now the routes are correctly pushing to the `<Stack />` inside the
`(group)`

This incorrect back behaviour can be demonstrated by [this
test](https://github.com/expo/expo/blob/0dd5fd0bea34c5d8faeb6b4ada8ea82cfe220c39/packages/expo-router/src/__tests__/stacks.test.ios.tsx#L98).
Notice how some routes were previously not added to the history, causing
the back behaviour to "skip" a route

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
  • Loading branch information
marklawlor and expo-bot committed Mar 26, 2024
1 parent 27a1f35 commit 62b3fbe
Show file tree
Hide file tree
Showing 7 changed files with 421 additions and 185 deletions.
1 change: 1 addition & 0 deletions packages/expo-router/CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@

### 🐛 Bug fixes

- Ensure navigation events target the correct navigator ([#27485](https://github.com/expo/expo/pull/27485) by [@marklawlor](https://github.com/marklawlor))
- Fix using array syntax `(a,b)` with server output. ([#27462](https://github.com/expo/expo/pull/27462) by [@EvanBacon](https://github.com/EvanBacon))
- Fix issue with skipping all imports. ([#27238](https://github.com/expo/expo/pull/27238) by [@EvanBacon](https://github.com/EvanBacon))
- Include search parameters in the default Screen.getId() function. ([#26710](https://github.com/expo/expo/pull/26710) by [@marklawlor](https://github.com/marklawlor))
Expand Down
2 changes: 1 addition & 1 deletion packages/expo-router/build/global-state/routing.d.ts.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

83 changes: 60 additions & 23 deletions packages/expo-router/build/global-state/routing.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/expo-router/build/global-state/routing.js.map

Large diffs are not rendered by default.

0 comments on commit 62b3fbe

Please sign in to comment.