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

fix useRouter error for expo nextjs 13 #230

Merged
merged 1 commit into from
Nov 19, 2022
Merged

Conversation

ebg1223
Copy link
Contributor

@ebg1223 ebg1223 commented Nov 13, 2022

calling useRouter hook when using nextjs 13 branch leads to an error on expo: unable to find Navigation, instead of simply returning undefined. Using same method as use-route for use-router bypasses this issue so that useRouter hook is only called iff on web.

@vercel
Copy link

vercel bot commented Nov 13, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
solito-s9oj ❌ Failed (Inspect) Nov 13, 2022 at 7:06AM (UTC)
with-custom-fonts ✅ Ready (Inspect) Visit Preview Nov 13, 2022 at 7:06AM (UTC)
2 Ignored Deployments
Name Status Preview Updated
solito ⬜️ Ignored (Inspect) Nov 13, 2022 at 7:06AM (UTC)
solito-app ⬜️ Ignored (Inspect) Nov 13, 2022 at 7:06AM (UTC)

@xinha-sh
Copy link

@ebg1223 wouldn't this make useRouter functionality redundant on native ?

@ebg1223
Copy link
Contributor Author

ebg1223 commented Nov 14, 2022

@xinha-sh sorry, not quite sure what you mean. This only changes the behavior of the useParam hook. The problem is, useParam hook calls both useRoute and useRouter, as hooks must not be conditional.

Prior to next13, useRouter would return undefined if next router was not active. With next13, it instead throws an error, which crashes expo.

This change makes it so that if useRouter is called from the useParam hook on native, useRouter will not actually be called. The same functionality exists for useRoute for the native router.

@nandorojo
Copy link
Owner

i think this looks good to me, i’ll include it in v2

@ebg1223
Copy link
Contributor Author

ebg1223 commented Nov 14, 2022

@nandorojo Thank you!

@nandorojo nandorojo merged commit 693227a into nandorojo:next13 Nov 19, 2022
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 this pull request may close these issues.

3 participants