Skip to content
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

Expo Router v2 mounting a route twice #838

Closed
shessafridi opened this issue Aug 14, 2023 · 8 comments · Fixed by expo/expo#24147
Closed

Expo Router v2 mounting a route twice #838

shessafridi opened this issue Aug 14, 2023 · 8 comments · Fixed by expo/expo#24147

Comments

@shessafridi
Copy link

shessafridi commented Aug 14, 2023

Which package manager are you using? (Yarn is recommended)

npm

Summary

Expo router V2 in mounting routes twice when navigating to an outside route from a tabs screen when the tabs screen and the outside screen is in a group, this issue is not present in v1.

In the example when navigating from /my-account to /address the address/index.tsx component is mounted twice as indicated by
useEffect(() => { console.log('Address Mount'); }, []);
In the address/index.tsx file, this is a big issue as it also plays the routing animation twice on Android.

Removing the (first-level) group will fix the issue but in my use case I need it.

Example app folder structure:

app
│   index.tsx
│   _layout.tsx
│
└───(first-level)
    │   _layout.tsx
    │
    ├───(tabs)
    │       home.tsx
    │       my-account.tsx
    │       _layout.tsx
    │
    └───address
            index.tsx
            _layout.tsx

Minimal reproducible example

This example has a commit called "expo 49" which has the issue, and another commit called "No Issue on Expo 48" which has the project downgraded to expo router v1 with no code change

https://github.com/shessafridi/expo-router-bug

@oliviercperrier
Copy link

Same here

@RoyalAdvisor
Copy link

Also experiencing the same issue. Thought I was doing routing incorrectly.

@acordiner92
Copy link

yeah same problem here

@EvanBacon
Copy link
Contributor

This is due to a bug in React Navigation where passing the seemingly correct navigation state will cause the stack to invalidate the second screen and re-mount it. You can work around this by using React Navigation to "push" the destination screen, the caveat being that usePathname and related URL will not be correct. Here's an updated app/(first-level)/(tabs)/my-account.tsx that navigates to the screen without re-rendering.

import { View, Text } from "react-native";
import React from "react";
import { Link, useNavigation } from "expo-router";

type Props = {};

const MyAccountPage = (props: Props) => {
  const navigation = useNavigation();
  return (
    <View>
      <Text>MyAccountPage</Text>
     {/* Correct navigation + URL, but re-renders */}
      <Link href="/address">Address</Link>
     {/* Correct navigation, single render, but incorrect URL and transition. */}
      <Text
        onPress={() => {
          navigation.push("address");
        }}
      >
        Address with React Nav
      </Text>
    </View>
  );
};

export default MyAccountPage;

Also worth mentioning that nesting Stack -> Slot -> Stack may lead to problems if the nested stacks can conform to the same push events. Stacks aren't analogous to the web, if you push a page, the resolution will bubble up and may be handled in a way that makes sense technically but doesn't feel ergonomic to the developer or user.

EvanBacon added a commit to expo/expo that referenced this issue Aug 28, 2023
# Why

Prevent double renders by cloning state to avoid leaking state between
functions.

- fix expo/router#838 
- fix expo/router#733
- fix expo/router#320
- The issue expo/router#320 has multiple
different things linked, but the original case appears to be fixed.
- possibly also addresses expo/router#847

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

# How

- Prevent mutating the input state to avoid invalidating the nested
state.

# Test Plan

- The testing library doesn't seem to support this case. @marklawlor has
been tasked with ensuring the original branch can detect the error
https://github.com/expo/expo/compare/%40evanbacon/router/fix-838
Just in case the testing library isn't fixed, I ran locally with:

- `app/_layout.js`, `app/(a)/_layout.js`, `app/b/_layout.js`
```js
import { Slot } from "expo-router";
export default function RootLayout() {
  return <Slot />
}
```
- `app/(a)/index.js`
```js
import { router, useNavigation } from "expo-router";
import {  View } from "react-native";

export default function App() {
  // const navigation = useNavigation();

  setTimeout(() => {
    router.push("/b");
    // navigation.push("b");
  });
  return (
    <View />
  );
}
```
- `app/b/index.js`
```js
import { usePathname } from 'expo-router';
import { Text, View } from 'react-native';

let i = 0;
export default function Page() {
  const path = usePathname();
  i++;
  return (
    <View style={{ flex: 1, backgroundColor: i > 1 ? "red" : "white" }}>
      <Text>Path: {path}</Text>
    </View>
  );
}
```



<!--
Please describe how you tested this change and how a reviewer could
reproduce your test, especially if this PR does not include automated
tests! If possible, please also provide terminal output and/or
screenshots demonstrating your test/reproduction.
-->

# 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>
@EvanBacon
Copy link
Contributor

EvanBacon commented Aug 28, 2023

A fix has been published in expo-router@2.0.4

@AyoCodess
Copy link

A fix has been published in expo-router@2.0.4

thank you!

@oliviercperrier
Copy link

oliviercperrier commented Oct 26, 2023

@EvanBacon In 2.0.9,

  • Route is mounting twice
  • And also mounting when you navigate to an other route

@marklawlor
Copy link
Contributor

@oliviercperrier Please create a new issue with a reproducible demo. We cannot assist you if you don't provide any information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants