microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.24k stars 1.14k forks source link

Undesired re-renders in react-navigation #11094

Closed MacKenzieHnC closed 1 year ago

MacKenzieHnC commented 1 year ago

Problem Description

I already posted this in react-navigation and react-native-screens, but the more I dig, the less I think it is directly because of code in those repos. Admittedly, this is all extremely over-my-head, so be skeptical of my assessment.

The core issue is screens unmounting/remounting on every blur event in react-navigation, fully ignoring unmountOnBlur, which causes any local states associated with that screen to be lost. This most-frustratingly includes even opening and closing drawers.

I'm still digging, but so far all I've managed to figure out is that the root issue is probably in this repo. It is similar to this issue, but the associated fix has no effect.

I have confirmed that the issue affects drawer, stack, and native-stack implementations of react-navigation, so I suspect it affects all of them.

EDIT: Oh wait, is this a full duplicate? I sincerely cannot tell.

Maybe this is a new clue: drawers opening gets added to the navigation history the same as when you are changing screens. So the core could be that when the navigation prop changes, RNW thinks that that counts as a root change, and updates the whole tree as a result. But if that was all it is, then using

const history = useNavigationState(state => state.history);
history.push({key: 'route', name: 'someName''});

would trigger it and it doesn't. So it might have something to do with emit.

Steps To Reproduce

  1. New react native project
  2. install RNW and navigation
  3. Create a screen with a useState and blur events (navigation.push(), navigation.openDrawer, etc.)
  4. Watch your local state disappear

Expected Results

State should be maintained the same way it is on Android

CLI version

9.3.2

Environment

System:
    OS: Windows 10 10.0.19045
    CPU: (12) x64 Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
    Memory: 7.25 GB / 15.93 GB
  Binaries:
    Node: 18.12.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 9.2.0 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.17763.0, 10.0.18362.0, 10.0.19041.0
  IDEs:
    Android Studio: AI-221.6008.13.2211.9477386
    Visual Studio: 16.11.33214.272 (Visual Studio Community 2019)
  Languages:
    Java: 19.0.1 - C:\Program Files\Common Files\Oracle\Java\javapath\javac.EXE
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.1.0 => 18.1.0
    react-native: 0.70.6 => 0.70.6
    react-native-windows: 0.70.10 => 0.70.10
  npmGlobalPackages:
    *react-native*: Not Found

Target Platform Version

10.0.19041

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2019

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

https://github.com/MacKenzieHnC/ReactNavigationWindows (Note: there is a hack required to even run Gesture Handler on my machine. Clearly labeled in git commit history, unrelated to this issue, but drawer shows this issue most glaringly)

chrisglein commented 1 year ago

@AgneLukoseviciute @chiaramooney is this related to the accessibility tree noHideDescendents/screens issues that have been looked at? This may be part of what can only be fixed by a native implementation and doesn't work with the JS implementation. Can you link or duplicate this issue as appropriate?

MacKenzieHnC commented 1 year ago

Possibly related, I just tried this drawer example and absolutely none of the text inputs persist data for even a moment (without even opening the drawer). If that's not true for everyone, there might be something weird about my setup.

MacKenzieHnC commented 1 year ago

I understand this isn't anyone's priority, and that's fair, but could I just get an update:

On a scale from baby's first programming project to journey through the Forest of the Nightmare King, how hard is this to solve?

It's probably the latter, right?

hefberk commented 1 year ago

We are also facing same issue here is a simple reproductible demo :

const EntityListScreen = () => {
    const [count, setCount] = useState(0);

    return (
        <SafeAreaView style={{ flex: 1}}>
            <View style={{flex: 1}}>
              <Text>test {count}</Text>
                <Button title={"+1"} onPress={() => setCount(count+1)}/>
            </View>
        </SafeAreaView>
    );
};

const Drawer = createBottomTabNavigator(); //or createDrawerNavigator()

<Drawer.Navigator>
  <Drawer.Screen
                        key={'Drawer'}
                        name={'DrawerTitle'}
                        component={EntityListScreen}
                        initialParams={{ entityMetas: x}}
                    />
</Drawer.Navigator>

On iOS, web and android, screen change let the count at value previously set, whereas on windows count is reset each time a screen change. We should not have different behavior between platforms.

here is my package.json :

"dependencies": { "@babel/plugin-proposal-export-namespace-from": "^7.18.9", "@googlemaps/js-api-loader": "^1.15.1", "@masao/dynamics-web-api": "^1.7.6", "@masao/react-native-get-random-values": "^1.2.0", "@masao/react-native-maps": "^1.3.2", "@masao/react-native-msal": "^4.0.4", "@masao/react-native-splash-screen": "^3.3.0", "@react-native-async-storage/async-storage": "^1.17.11", "@react-native-community/datetimepicker": "^6.7.3", "@react-native-community/netinfo": "^9.3.7", "@react-native-community/slider": "^4.4.2", "@react-native-picker/picker": "^2.4.8", "@react-navigation/bottom-tabs": "^6.5.3", "@react-navigation/drawer": "^6.5.7", "@react-navigation/material-top-tabs": "^6.5.1", "@react-navigation/native": "^6.1.2", "@react-navigation/native-stack": "^6.9.8", "@react-navigation/stack": "^6.3.11", "@reduxjs/toolkit": "^1.9.2", "babel-polyfill": "^6.26.0", "core-js": "^3.6.5", "fast-xml-parser": "^4.0.12", "install": "^0.13.0", "patch-package": "^6.5.0", "react": "18.2.0", "react-dom": "^18.2.0", "react-native": "0.71.0", "react-native-calendars": "^1.1275.0", "react-native-gesture-handler": "^2.9.0", "react-native-offline": "^6.0.2", "react-native-pager-view": "^6.1.2", "react-native-paper": "^5.1.4", "react-native-ratings": "^8.1.0", "react-native-reanimated": "^3.0.2", "react-native-safe-area-context": "^4.5.0", "react-native-screens": "^3.19.0", "react-native-svg": "^13.7.0", "react-native-tab-view": "^3.3.4", "react-native-vector-icons": "^9.2.0", "react-native-web": "^0.18.12", "react-native-windows": "0.71.0", "react-redux": "^8.0.5", "yargs": "^17.6.2" }, "devDependencies": { "@babel/core": "^7.16.7", "@babel/preset-env": "^7.14.0", "@babel/runtime": "^7.16.7", "@react-native-community/eslint-config": "^3.0.0", "@svgr/webpack": "^6.5.1", "@tsconfig/react-native": "^2.0.2", "@types/jest": "^29.2.1", "@types/lodash": "^4.14.191", "@types/node": "^18.11.18", "@types/react": "^18.0.24", "@types/react-native": "^0.69.16", "@types/react-test-renderer": "^18.0.0", "@types/webpack": "^5.28.0", "@typescript-eslint/eslint-plugin": "^5.37.0", "@typescript-eslint/parser": "^5.37.0", "babel-jest": "^29.2.1", "babel-loader": "^9.1.2", "babel-plugin-module-resolver": "^5.0.0", "babel-plugin-react-native-web": "^0.18.10", "eslint": "^8.19.0", "html-webpack-plugin": "^5.5.0", "jest": "^29.2.1", "metro-config": "^0.72.3", "metro-react-native-babel-preset": "0.73.5", "prettier": "^2.4.1", "react-native-svg-transformer": "^1.0.0", "react-native-web-image-loader": "^0.1.1", "react-test-renderer": "18.2.0", "ts-loader": "^9.4.2", "ts-node": "^10.9.1", "typescript": "^4.9.5", "url-loader": "^4.1.1", "webpack": "^5.75.0", "webpack-cli": "^5.0.1", "webpack-dev-server": "^4.11.1" },

And here info: System: OS: Windows 10 10.0.22621 CPU: (16) x64 Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz Memory: 18.23 GB / 31.94 GB Binaries: Node: 16.5.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - C:\Program Files (x86)\Yarn\bin\yarn.CMD npm: 9.4.2 - C:\Program Files\nodejs\npm.CMD Watchman: Not Found SDKs: Android SDK: Not Found Windows SDK: AllowAllTrustedApps: Enabled AllowDevelopmentWithoutDevLicense: Enabled Versions: 10.0.17763.0, 10.0.19041.0, 10.0.22000.0, 10.0.22621.0 IDEs: Android Studio: Version 4.2.0.0 AI-202.7660.26.42.7486908 Visual Studio: 17.4.33213.308 (Visual Studio Community 2022) Languages: Java: 1.8.0_292 npmPackages: @react-native-community/cli: Not Found react: 18.2.0 => 18.2.0 react-native: 0.71.0 => 0.71.0 react-native-windows: 0.71.0 => 0.71.0 npmGlobalPackages: react-native: Not Found

acaininet commented 1 year ago

I am having this same issue in RNW .71 Is there a solution or workaround for this?

MacKenzieHnC commented 1 year ago

I am having this same issue in RNW .71 Is there a solution or workaround for this?

None that I could find, and I don't think one's coming soon. I ended up having to build navigation myself.

acaininet commented 1 year ago

I am having this same issue in RNW .71 Is there a solution or workaround for this?

None that I could find, and I don't think one's coming soon. I ended up having to build navigation myself.

How deep was the well to re-write navigation? How does it avoid the re-rendering issues?

MacKenzieHnC commented 1 year ago

I am having this same issue in RNW .71 Is there a solution or workaround for this?

None that I could find, and I don't think one's coming soon. I ended up having to build navigation myself.

How deep was the well to re-write navigation? How does it avoid the re-rendering issues?

I went with really minimal functionality so not awful. Had to get good at forwardRefs for my version of onNavigateAway, but that was the worst of it.

Getting the header bar to look right was pretty annoying, I guess.

I also made no attempt to implement any kind of history, which I'm sure would be an absolute nightmare, and I suspect that's where the bad interaction with RNW is. I don't know, but I think the problem is that the history object changes on every navigation event (and then emits that change), which makes RNW re-render the tree. I have no idea why it's only RNW tho

No idea how helpful it will be, but here is my code

chiaramooney commented 1 year ago

Closing issue for now. This is related to the Paper implementation on no-hide-descendants. The team is currently focused on implementing Fabric (the new architecture). Fabric will bring a re-write to this functionality. If issue persists on new architecture, new issue can be created.