Skip to content

Commit

Permalink
[router] fix relative navigation on hoisted routes (#27778)
Browse files Browse the repository at this point in the history
# Why

Fix #27753

# How

This `isIndexPath` function was not taking in account hoisted index
routes that may not have the `param` structure

Take the following structure.
```
_layout // Stack
parent/
└── child/
    ├── index // name: parent/child/index
    └── page  // name: parent/child/page 
```

Because there are no route params or nested `_layout`, all routes are
hoisted to the root `_layout`. The existing logic for only checking for
`route.params` is insufficient as it will not exist. Additionally we
cannot check for `name: 'index'` as hoisted routes have complex names.


# Test Plan

I think our existing tests are fine, just added one for this scenario

# 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 1e5f916 commit 5fc443e
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/expo-router/CHANGELOG.md
Expand Up @@ -31,6 +31,7 @@
- Export `toHaveRouterState` and other matcher types from `expo-router/testing-library` ([#27646](https://github.com/expo/expo/pull/27646) by [@marklawlor](https://github.com/marklawlor))
- Fix missing types from typed routes ([#27412](https://github.com/expo/expo/pull/27412) by [@marklawlor](https://github.com/marklawlor))
- Fork NavigationContainer on web to use custom linking context ([#27712](https://github.com/expo/expo/pull/27712) by [@marklawlor](https://github.com/marklawlor))
- Fix relative navigation on hoisted routes ([#27778](https://github.com/expo/expo/pull/27778) by [@marklawlor](https://github.com/marklawlor))

### 💡 Others

Expand Down
2 changes: 1 addition & 1 deletion packages/expo-router/build/LocationProvider.d.ts.map

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

10 changes: 9 additions & 1 deletion packages/expo-router/build/LocationProvider.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/LocationProvider.js.map

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

13 changes: 12 additions & 1 deletion packages/expo-router/src/LocationProvider.tsx
Expand Up @@ -33,10 +33,21 @@ function isIndexPath(state: State) {
if (route.state) {
return isIndexPath(route.state);
}
// router.params is typed as 'object', so this usual syntax is to please TypeScript

// Index routes on the same level as a layout do not have `index` in their name
if (route.params && 'screen' in route.params) {
return route.params.screen === 'index';
}

// The `params` key will not exist if there are no params
// So we need to do a positive lookahead to check if the route ends with /index
// Nested routes that are hoisted will have a name ending with /index
// e.g name could be /user/[id]/index
if (route.name.match(/.+\/index$/)) return true;

// The state will either have params (because there are multiple _layout) or it will be hoisted with a name
// If we don't match the above cases, then it's not an index route

return false;
}

Expand Down
20 changes: 19 additions & 1 deletion packages/expo-router/src/__tests__/navigation.test.ios.tsx
@@ -1,5 +1,5 @@
/* eslint-disable react-hooks/rules-of-hooks */
import React, { Text } from 'react-native';
import React, { Text, View } from 'react-native';

import {
useRouter,
Expand All @@ -9,6 +9,7 @@ import {
Redirect,
Slot,
usePathname,
Link,
} from '../exports';
import { Stack } from '../layouts/Stack';
import { Tabs } from '../layouts/Tabs';
Expand Down Expand Up @@ -821,6 +822,23 @@ it('can push relative links from index routes', async () => {
expect(screen).toHavePathname('/test/bar');
});

it('can push relative links from hoisted routes', () => {
renderRouter(
{
_layout: () => <Stack />,
'parent/index': () => <Link testID="link" href="./child" />,
'parent/child': () => <View testID="child" />,
},
{
initialUrl: '/parent',
}
);

expect(screen.getByTestId('link')).toBeOnTheScreen();
fireEvent(screen.getByTestId('link'), 'press');
expect(screen.getByTestId('child')).toBeOnTheScreen();
});

it('can navigation to a relative route without losing path params', async () => {
renderRouter(
{
Expand Down

0 comments on commit 5fc443e

Please sign in to comment.