search

nandorojo / expo-next-react-navigation

⛴ Make Next.js and react-navigation play nicely together with an Expo/React Native Web app.
406 stars 32 forks source link

useRouting() is undefined using React Navigation v5 #34

Closed markmctamney closed 3 years ago

markmctamney commented 4 years ago

Example: https://github.com/markmctamney/react-native-starter/commit/3cfa99c42bdd4b55dda2b83e7e632946dc998816#diff-95cb9be977482c4f4ee17a16ad88ee24R15

Per the console logs, while useNavigation and useRoute work as expected, useRouting() is coming back undefined when using branch v5

I tracked this back to the expo-navigation-core package and was able to fork and fix this. I believe the issue is that package is requiring @react-navigation/native as a direct dependency rather than a peer dependency, so when expo-navigation-core is imported as a dependency by this package, it is importing its own separate copy of @react-navigation/native. Then useRouting is referencing the context created by that copy, which is undefined, rather than the context created by the version of @react-navigation/native imported to the user's project.

Anyway, my fix for this was just to edit expo-navigation-core to:

  1. Remove @react-navigation/native as a dependency
  2. Add @react-navigation/native as both a peer dependency and dev dependency (still needed for internal development I believe)
  3. Rebuild & re-publish expo-navigation-core and update this package's reference to the new one

See my fix + working example after the change: https://github.com/nandorojo/expo-navigation-core/compare/v5...markmctamney:v5#diff-b9cfc7f2cdf78a7f4b91a753d10865a2

I tested that my original example and confirmed it is now working. Didn't do much testing beyond that, but just let me know if you need anything else!

nandorojo commented 4 years ago

Hey thanks for this, appreciate you digging in!

See my fix + working example after the change: nandorojo/expo-navigation-core@v5...markmctamney:v5diff-b9cfc7f2cdf78a7f4b91a753d10865a2

I might be missing something here, but it says there aren't any differences ^

markmctamney commented 4 years ago

Hm, sorry about that. Thought the link was working when I pasted it, but coming back with no changes when I clicked it just now also.

Looks like the "#" between "v5" and "diff-b9..." is getting subtracted out somewhere between me pasting the link and the markdown rendering the link to break it. Link should be:

https://github.com/nandorojo/expo-navigation-core/compare/v5...markmctamney:v5#diff-b9cfc7f2cdf78a7f4b91a753d10865a2

If not, this should show all of the the changes: https://github.com/nandorojo/expo-navigation-core/compare/v5...markmctamney:v5

and you can just scroll down to package.json, which just has the changes that I detailed out regarding React Navigation as a dependency. All of the other changes are just to files in the build/ folder, which I believe ordinarily wouldn't be committed, but I had to commit to my temporary project, since that was the folder referenced by my fork of expo-next-react-navigation

nandorojo commented 3 years ago

Hey sorry I never got back to you here.

I'm trying to upgrade react navigation as you suggested, and move it out of the actual dependencies into devDependencies and peerDependencies. However, after upgrading the version, I'm getting some intense TS errors when I run yarn prepare to build it. Does this happen for you?

markmctamney commented 3 years ago

Gotcha, yeah I don't think I ran yarn prepare since I was just pushing the code to GitHub and doing a temporary reference to verify this fixed the Context issue, rather than publishing the finished package. But I see some errors from the linter.

Pretty sure the type errors have to do with just importing the "base" types from @react-navigation/native, while your API is effectively assuming you're dealing with a Stack Navigator (from @react-navigation/stack) because of the use of navigation actions like "push", "popToTop", and "replace", which are actions that only exist for Stack Navigators (see Stack Actions in the v5 docs) but not for the other navigators (tab/drawer/etc).

There's a different navigation prop type, StackNavigationProp exported from @react-navigation/stack that should probably be used for stack navigators. There's also documentation specifically on Typescript in the Type Checking with Typescript section, which also deals specifically with annotating useNavigation and useRoute.

If I have some time, I'll maybe dig into the docs a bit more and try to take a crack at a fix here. But hopefully this helps as a step in the right direction if not. If I can help out any other way in the meantime, just let me know.

nandorojo commented 3 years ago

Hmm interesting, my errors are actually coming from imports in their library. I'll see what's going on there, appreciate your explanation here.