fix: catch undefined navigation

[jaulz]

In my case I have an onFocus prop in a reusable component which can be used across the app. The onFocus prop will be called in the useNavigationEvents hook whenever the component is focused. However, in some cases the navigation is undefined because the reusable component is used in places where no navigation is present. Since we cannot conditionally use hooks, this will always fail.

In addition to the simple return in the hook we could also print a comment to inform the user about this behaviour.

[satya164]

Wouldn't it be an error in most cases though? I don't imagine lot of people having same component both in Navigation tree and outside

[jaulz]

I forgot to mention that in my case the onFocus prop is optional. As soon as this reusable component is used in a Portal for example the issue will appear. If you are willing to accept the PR, I can also implement similar checks in the other hooks and print a warning.

[slorber]

I also have the case in my apps where sometimes I want a component to behave different conditionally depending if it's on a react-navigation tree (ie there's a provider on top) or not

I know this is a bit hacky and not very idiomatic but sometimes it's more convenient. For example I can make a generic that is automatically wired to the navigation back (with ability to override).

The core (withNavigation etc) does not currently really permit to handle those cases as it fails whenever the context is not found.

But not sure if we need to design apis with this in mind, as long as we allow "useNavigation" to return nothing if there's no navigation in context that could be enough.

@jaulz I suggest this workaround:

const navigation = useNavigation()
const isFocused = navigation ? useNavigationFocus().isFocused : false;

This may seem counterintuitive and it's generally advised to NOT use conditionals with hooks, but for this usecase it's fine because the test will remain true/false for the whole livetime of the comp so the hooks ordering will be preserved. This trick is often used also for conditionally using hooks in node/browser envs differently.

Please tell me if this solves your problem

If we need to merge this PR, I'd rather wait until we find a good setup to actually test the dev. Didn't find time to fix this example app PR which has a metro config issue or something: #28

[jaulz]

@slorber thanks for your response! That's exactly how I workaround the issue at the moment but I thought it would anyway make sense to catch such errors and make it more intuitive for other developers.

[slorber]

Honestly I think we'd rather fail fast in such case, with an explicit error message telling the user he did not set the container/provider correctly.

I think it's safer to make life easier for beginners, and still offer workarounds for more complex usecases. What we really don't want is a beginner that did not set correctly the provider, tries to listen to events for some reasons and receive no event and open an issue about that.

[slorber]

closing because we choose to fail fast, while still providing a way to access navigation conditionnally using useContext(NavigationContext)

See #37