Skip to content
This repository has been archived by the owner on Dec 3, 2022. It is now read-only.

Types are missing and causing errors #42

Open
shamilovtim opened this issue Sep 21, 2019 · 18 comments · May be fixed by #71
Open

Types are missing and causing errors #42

shamilovtim opened this issue Sep 21, 2019 · 18 comments · May be fixed by #71

Comments

@shamilovtim
Copy link

Using react-navigation v4

Hook:
const navigation = useNavigation<any>();

Use hook:
navigation.popToTop();

Typescript error:
Property 'popToTop' does not exist on type 'NavigationScreenProp<any, NavigationParams>'.ts(2339)

@satya164
Copy link
Member

satya164 commented Sep 22, 2019

IMO useNavigation should take the type of navigation prop as the generic instead of S (?):

const navigation = useNavigation<NavigationStackProp>();

cc @slorber

@slorber
Copy link
Member

slorber commented Sep 24, 2019

yeah that's probably right, will try to see how to expose custom nav actions more easily

@Grohden
Copy link

Grohden commented Sep 24, 2019

Edit: that was a suggestion? I thought that you said it could be used that way in the current version XD

@satya164 the return type of the hook is NavigationScreenProp<S & NavigationRoute>

and the type is defined as (in react-navigation package):

  export interface NavigationScreenProp<S, P = NavigationParams> {
    state: S & { params?: P };
    dispatch: NavigationDispatch;
    goBack: (routeKey?: string | null) => boolean;
    dismiss: () => boolean;
    navigate(options: {
      routeName:
        | string
        | {
            routeName: string;
            params?: NavigationParams;
            action?: NavigationNavigateAction;
            key?: string;
          };
      params?: NavigationParams;
      action?: NavigationAction;
      key?: string;
    }): boolean;
    navigate(
      routeNameOrOptions: string,
      params?: NavigationParams,
      action?: NavigationAction
    ): boolean;
    openDrawer: () => any;
    closeDrawer: () => any;
    toggleDrawer: () => any;
    getParam<T extends keyof P>(
      param: T,
      fallback: NonNullable<P[T]>
    ): NonNullable<P[T]>;
    getParam<T extends keyof P>(param: T): P[T];
    setParams: (newParams: Partial<P>) => boolean;
    emit: (eventName: 'refocus') => void;
    addListener: (
      eventName: string,
      callback: NavigationEventCallback
    ) => NavigationEventSubscription;
    isFocused: () => boolean;
    isFirstRouteInParent: () => boolean;
    router?: NavigationRouter;
    dangerouslyGetParent: () => NavigationScreenProp<S> | undefined;
  }

The S is required, but it's used for state, so using:
const navigation = useNavigation<NavigationStackProp>(); doesn't solve for me :/

Is there anything new to release on other packages?
I'm using these versions:

 "react-navigation": "4.0.9",
 "react-navigation-hooks": "1.0.3",
 "react-navigation-stack": "1.9.0"

Btw I think that this could be solved using:

export function useNavigation<NavigatorProps = any, S = any>(): NavigationScreenProp<
    S & NavigationRoute
> & NavigatorProps

@satya164
Copy link
Member

Just a suggestion sorry 😛

@Wintus
Copy link

Wintus commented Nov 19, 2019

I agree with the idea that useNavigation should take a prop in addition. #42 (comment)

At this time to avoid the error, I figured out no workaround on this problem but a (poor) downcasting as below:

import {useNavigation} from 'react-navigation-hooks';
import {NavigationStackProp} from 'react-navigation-stack';

// FIXME: workaround
export const useStackNavigation = () => useNavigation() as NavigationStackProp;

And this works fine so far.

Here is a more sophisticated downcast.
https://gist.github.com/Wintus/c7fcfda1a735e6f7f89ce4c54c1d5d8e#file-usestacknavigation-ts

@slorber
Copy link
Member

slorber commented Nov 19, 2019

Hey, just thinking out loud here.

For you what's the difference between useNavigation<NavigationStackProp>() and useNavigation() as NavigationStackProp and useStackNavigation?

For me all those are different syntaxes to downcast to a stack navigation object. Is one really better than the other conceptually if we don't take into account the verboseness?

If you call useStackNavigation and it returns a switch navigation, that could be misleading as well. Maybe we should have a custom hook for each type of navigator to encapsulate the casting + a runtime assertion to ensure we don't return a switch navigator when user call useStackNavigation?

But what about custom navigators/routers (should we ask the user to create his own hook and do the casting/runtime check on his own?)

@satya164 how do you plan to solve this in v5?

@satya164
Copy link
Member

how do you plan to solve this in v5?

In v5, useNavigation accepts an optional type parameter:

const navigation = useNavigation<StackNavigationProp<RootParamList>>()

For me all those are different syntaxes to downcast to a stack navigation object

In this case the end result is same, but type casting and generics have very different semantics.

With type casting, you tell the compiler that the value is a certain type and it won't complain even if it's not (as long as the interface matches vaguely). Typecasting should be avoided unless necessary.

A generic is like a parameter, TypeScript will show type hints that the hook accepts this type parameter, it maybe required or optional, the hook can ensure that supplied type parameter matches the desired constraint.

Consider the following 2 snippets:

useNavigation() as { setParams: Function }; // No error

useNavigation<{ setParams: Function }>(); // Shows error since the supplied type doesn't match constraint

If you call useStackNavigation and it returns a switch navigation, that could be misleading as well. Maybe we should have a custom hook for each type of navigator to encapsulate the casting + a runtime assertion to ensure we don't return a switch navigator when user call useStackNavigation?

The problem here is that we grab the value automatically from the context. There's no way to statically verify that the type/function being used is correct so we need to entirely trust user on this.

A navigation object can be a combination of multiple navigation objects as well (e.g. have both push and openDrawer methods) depending on nesting, so we can't restrict the type to only one navigator either.

A runtime assertion will provide minimal value here imo, at the cost of each navigator needing to export their own hook.

@slorber
Copy link
Member

slorber commented Nov 19, 2019

But in such case, if we want to support custom actions/navigators, we can't do any constraint on the generic params if we want to allow const navigation = useNavigation<MyCustomNavigator<MyParams>>().

useNavigation<T> = () => useUnsafeNavigation() as T is not really safer, it's just encapsulating the downcast so just feels a bit nicer to use for users.
Do we have any constraint here to apply on T that make sense in all cases (including custom navs?)

@satya164
Copy link
Member

satya164 commented Nov 19, 2019

See the linked code for useNavigation. The code I posted gives error because there's a constraint.

@slorber
Copy link
Member

slorber commented Nov 19, 2019

Thanks, will try to see what I can take from v5

Seems like you have built a typesafe route/params system, looks nice. Wonder if this can be ported to v4 without too much work but maybe i'd rather do something more simple

@satya164
Copy link
Member

It can be ported, but it'll break all existing types of v4 so probably not worth it.

@luisnaranjo733
Copy link

So no support for this until react-navigation v5?

@slorber
Copy link
Member

slorber commented Dec 28, 2019

@luisnaranjo733 if you find a good solution for v4, don't hesitate to send a PR

@ammichael
Copy link

no solution for v5 yet?

@luisnaranjo733
Copy link

I do this workaround to get stack navigator props in v5. I opened a canny feature request on canny but it sounds like its not possible with the current implementation :(

https://react-navigation.canny.io/feature-requests/p/improve-type-safety-for-the-usenavigation-hook-in-react-navigation5

import { StackNavigationProp } from "@react-navigation/stack";
import { useNavigation as _useNavigation } from "@react-navigation/native";
/**
 * Type wrapper for the useNavigation hook
 */
export const useNavigation = () => {
  return (_useNavigation() as unknown) as StackNavigationProp<GlobalRouteParamList, RouteName>;
};

@ammichael
Copy link

ammichael commented Jun 16, 2020

I ended up using this:

const { goBack, popToTop } = useNavigation<any>();

Pretty sure there is a better way, like it's mentioned here https://reactnavigation.org/docs/typescript/#type-checking-screens, but it didn't work for me probably because I didn't use it right 🤔

@luisnaranjo733
Copy link

useNavigation is never going to have the stack props on it. If you look at my canny feature request the maintainers explained why. You have to type cast to the StackNavigationProp if you want to "safely" use stack props.

@amerllica
Copy link

amerllica commented Jun 23, 2020

I'm using:

"@react-navigation/native": "^5.5.1",
"@react-navigation/stack": "^5.5.1",

But in the blow code I see TypeScript missing type error:

const { navigate, push } = useNavigation();

Screen Shot 2020-06-23 at 5 31 52 PM

I guess the written types are not complete.

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

Successfully merging a pull request may close this issue.

8 participants