Skip to content

feat: Export StackNavigator, StackNavigatorProps and TypedStackNavigator type defs#8460

Closed
jtomaszewski wants to merge 1 commit into
react-navigation:mainfrom
ailohq:export-stack-navigator-type
Closed

feat: Export StackNavigator, StackNavigatorProps and TypedStackNavigator type defs#8460
jtomaszewski wants to merge 1 commit into
react-navigation:mainfrom
ailohq:export-stack-navigator-type

Conversation

@jtomaszewski

Copy link
Copy Markdown

(Same as #8233 but rebased from master to main)

Thanks to this, you can easily import and construct a type of a value returned by createStackNavigator<T>() expression.

We would use this because our app is quite complex, and so we extract constructing routes/params/stacks to separate files. We could use this like that:

import React from "react";
import { TypedStackNavigator } from "@react-navigation/stack";
import { RootStackNavigatorParamList, Screens } from "../navigation";
import { ChatScreen, ChatListScreen } from "./chat";

export const getChatStackScreens = (
  Stack: TypedStackNavigator<RootStackNavigatorParamList>
): React.ReactElement => {
  return (
    <>
      <Stack.Screen
        name={Screens.ChatList}
        component={ChatListScreen}
      />
      <Stack.Screen
        name={Screens.Chat}
        component={ChatScreen}
      />
    </>
  );
};

// and then in `Stack.Navigator` just use it like `{getMoreStackScreens(Stack)}`

@satya164 satya164 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Can you also export types for all of the other navigators?

StackNavigationConfig;

function StackNavigator({
export function StackNavigator({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think exposing this is confusing (there's no the difference between this and Stack.Navigator to the user, which one should they use). Why do you need it?

} from '../types';

type Props = DefaultNavigatorOptions<StackNavigationOptions> &
export type Props = DefaultNavigatorOptions<StackNavigationOptions> &

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shouldn't be exported. You should use React.ComponentProps<typeof Stack.Navigator> or React.ComponentProps<TypedStackNavigator<ParamList>['Navigator']> in this case. Otherwise it lacks type information for params.

typeof StackNavigator
>(StackNavigator);

export type TypedStackNavigator<

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename it to StackNavigatorType?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also rename TypedNavigator to NavigatorType? (That's why I named it TypedStackNavigator, because TypedNavigator already existed.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's internal, so it doesn't matter what we name it. I think TypedNavigator is ok for internal use.

default as createStackNavigator,
StackNavigator,
Props as StackNavigatorProps
} from './navigators/createStackNavigator';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move the type export to separate export type statement?

@satya164

satya164 commented Feb 1, 2022

Copy link
Copy Markdown
Member

Closing due to no activity

@satya164 satya164 closed this Feb 1, 2022
@github-actions

github-actions Bot commented Feb 1, 2022

Copy link
Copy Markdown

Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro.

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.

2 participants