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

nav types: Add and demo RouteProp, to keep route-param types next to props #4430

Merged
merged 3 commits into from
Jan 26, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/__flow-tests__/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Type-tests

This directory contains tests of our types.

This code never gets run, but it does get type-checked. We use it to
test that some of our more complex types behave in the way we expect.

Mostly this means checking that using the types in certain ways that
should give errors, does give errors. (Checking that things that
should be OK, are OK, is generally well covered by where we simply use
the types, in our main app code.)

The key tool for confirming that something does give an error is to
write `$FlowExpectedError`. As far as Flow is concerned (see [Flow
docs][https://flow.org/en/docs/errors/]), this has exactly the same
effect as `$FlowFixMe`: it suppresses the error, and conversely it
means that if we ever stop getting an error at that spot, we start
getting a warning about the unused suppression.

The difference between `$FlowFixMe` and `$FlowExpectedError` is purely
for the human reader: the latter communicates that the error is
intended, not something we want to get rid of, and in particular that
if it ever goes away then that's itself a problem which we should
investigate.
78 changes: 78 additions & 0 deletions src/__flow-tests__/nav-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* Type-tests for navigation.
*
* @flow strict-local
*/

import React, { type ComponentType } from 'react';
import {
createStackNavigator,
type StackNavigationProp,
TransitionPresets,
} from '@react-navigation/stack';

import { type RouteProp, type RouteParamsOf } from '../react-navigation';

/* eslint-disable flowtype/generic-spacing */

// Test that `RouteProp` gives route.params the right type.
function testRouteParamTypes() {
type ProfileProps = {|
// skip navigation
+route: RouteProp<'Profile', {| +userId: string |}>,
|};

function Profile(props: ProfileProps) {
const { params } = props.route;

(params.userId: string);
// $FlowExpectedError
(params.userId: empty);

(('a': string): typeof params.userId);
// $FlowExpectedError
(('a': mixed): typeof params.userId);

// $FlowExpectedError
params.nonsense;
}
}

// Test that `RouteProp` gives type-checking of the route name
// at the navigator.
function testNavigatorTypes() {
// (The setup of this one is a bit cumbersome because we need to set up
// the navigator.)

type ProfileProps = {|
+navigation: NavigationProp<'Profile'>,
+route: RouteProp<'Profile', {| +userId: string |}>,
|};
declare var Profile: ComponentType<ProfileProps>;

declare var Profile12: ComponentType<{|
+navigation: NavigationProp<'Profile1'>,
+route: RouteProp<'Profile2', {| +userId: string |}>,
|}>;

type NavParamList = {|
Profile: RouteParamsOf<typeof Profile>,
Profile1: RouteParamsOf<typeof Profile12>,
Profile2: RouteParamsOf<typeof Profile12>,
|};

// prettier-ignore
type NavigationProp<+RouteName: $Keys<NavParamList> = $Keys<NavParamList>> =
StackNavigationProp<NavParamList, RouteName>;

const Stack = createStackNavigator<NavParamList, NavParamList, NavigationProp<>>();

<Stack.Navigator>
Copy link
Contributor

Choose a reason for hiding this comment

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

JSX just hanging out here, with no return and no render—totally valid, and I wouldn't change it at all, but I did a double-take! 😄

{/* Happy case is happy */}
<Stack.Screen name="Profile" component={Profile} />
{/* $FlowExpectedError - mismatch of name with route prop */}
<Stack.Screen name="Profile1" component={Profile12} />
{/* Should error but doesn't! on mismatch of name with navigation prop */}
<Stack.Screen name="Profile2" component={Profile12} />
</Stack.Navigator>;
}
61 changes: 59 additions & 2 deletions src/react-navigation.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,68 @@
/* @flow strict-local */
/**
* Helpers for using react-navigation and its relatives.
*
* @flow strict-local
*/

import { useNavigation as useNavigationInner } from '@react-navigation/native';
import { type ElementConfig } from 'react';
import {
useNavigation as useNavigationInner,
type LeafRoute,
type ScreenParams,
} from '@react-navigation/native';

import type { GlobalParamList } from './nav/globalTypes';

/* eslint-disable flowtype/generic-spacing */

/**
* A type to use for the `route` prop on a screen component.
*
* Typically used in the component definition, to annotate its `route` prop.
*
* The type this produces is just like the one produced by `RouteProp` in
* the upstream libdef. The difference is that this one takes `RouteParams`
* as an argument directly, making it suitable for expressing the params
* type directly in the component's own source, just like props types. The
* navigator's central param list can then use `RouteParamsOf` to get the
* appropriate type from the component definition, rather than vice versa.
*
* @param {RouteName} - Must equal the route name this screen will be known
* by at the navigator. (This is type-checked at the navigator.)
* @param {RouteParams} - The type to use for `props.route.params`.
*/
export type RouteProp<+RouteName: string, +RouteParams: ScreenParams> = {|
...LeafRoute<RouteName>,
+params: RouteParams,
|};

/**
* The type of the route params on the given screen component.
*
* This is intended for use in the params-list type for a navigator.
* Sample usage, following the same example as in the upstream docs
* at https://reactnavigation.org/docs/typescript/ :
*
* export type RootStackParamList = {|
* 'Profile': RouteParamsOf<typeof Profile>,
* |};
*
* This allows each component's route-params type to be defined within the
* component's own source, just like props types. Continuing the example,
* the definition of the `Profile` component might contain a definition like
* this one:
*
* type Props = {|
* +navigation: StackNavigationProp<RootStackParamList, 'Profile'>,
* +route: RouteProp<'Profile', {| +userId: string |},
* |};
*
* (Or better yet, next to the params-list type we might define an alias
* to simplify the `navigation` prop's type, to something like
* `RootStackNavigationProp<'Profile'>`.)
*/
export type RouteParamsOf<-C> = $PropertyType<$PropertyType<ElementConfig<C>, 'route'>, 'params'>;

/**
* Exactly like `useNavigation` upstream, but more typed.
*
Expand Down
7 changes: 4 additions & 3 deletions src/sharing/ShareToPm.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
import React from 'react';
import { View, Image, ScrollView, Modal, BackHandler } from 'react-native';

import type { SharingNavigationProp, SharingRouteProp } from './SharingScreen';
import type { RouteProp } from '../react-navigation';
import type { SharingNavigationProp } from './SharingScreen';
import * as NavigationService from '../nav/NavigationService';
import type { Dispatch, Auth, GetText, UserId } from '../types';
import type { Dispatch, Auth, GetText, SharedData, UserId } from '../types';
import { createStyleSheet } from '../styles';
import { TranslationContext } from '../boot/TranslationProvider';
import { connect } from '../react-redux';
Expand Down Expand Up @@ -56,7 +57,7 @@ const styles = createStyleSheet({

type Props = $ReadOnly<{|
navigation: SharingNavigationProp<'share-to-pm'>,
route: SharingRouteProp<'share-to-pm'>,
route: RouteProp<'share-to-pm', {| sharedData: SharedData |}>,

dispatch: Dispatch,
auth: Auth,
Expand Down
7 changes: 4 additions & 3 deletions src/sharing/ShareToStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
import React from 'react';
import { View, Image, ScrollView, BackHandler } from 'react-native';

import type { SharingNavigationProp, SharingRouteProp } from './SharingScreen';
import type { SharingNavigationProp } from './SharingScreen';
import type { RouteProp } from '../react-navigation';
import * as NavigationService from '../nav/NavigationService';
import type { Dispatch, Subscription, Auth, GetText } from '../types';
import type { Dispatch, Subscription, Auth, GetText, SharedData } from '../types';
import { createStyleSheet } from '../styles';
import { TranslationContext } from '../boot/TranslationProvider';
import { connect } from '../react-redux';
Expand Down Expand Up @@ -44,7 +45,7 @@ const styles = createStyleSheet({

type Props = $ReadOnly<{|
navigation: SharingNavigationProp<'share-to-stream'>,
route: SharingRouteProp<'share-to-stream'>,
route: RouteProp<'share-to-stream', {| sharedData: SharedData |}>,

dispatch: Dispatch,
subscriptions: Map<number, Subscription>,
Expand Down
12 changes: 4 additions & 8 deletions src/sharing/SharingScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import {
createMaterialTopTabNavigator,
type MaterialTopTabNavigationProp,
} from '@react-navigation/material-top-tabs';
import type { RouteProp } from '@react-navigation/native';
import { FormattedMessage } from 'react-intl';

import type { GlobalParamList } from '../nav/globalTypes';
import type { RouteParamsOf } from '../react-navigation';
import type { AppNavigationProp, AppNavigationRouteProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import type { Dispatch, Auth, SharedData } from '../types';
import type { Dispatch, Auth } from '../types';
import { createStyleSheet } from '../styles';
import { materialTopTabNavigatorConfig } from '../styles/tabs';
import { connect } from '../react-redux';
Expand All @@ -22,18 +22,14 @@ import ShareToStream from './ShareToStream';
import ShareToPm from './ShareToPm';

export type SharingNavigatorParamList = {|
'share-to-stream': {| sharedData: SharedData |},
'share-to-pm': {| sharedData: SharedData |},
'share-to-stream': RouteParamsOf<typeof ShareToStream>,
'share-to-pm': RouteParamsOf<typeof ShareToPm>,
|};

export type SharingNavigationProp<
+RouteName: $Keys<SharingNavigatorParamList> = $Keys<SharingNavigatorParamList>,
> = MaterialTopTabNavigationProp<GlobalParamList, RouteName>;

export type SharingRouteProp<
RouteName: $Keys<SharingNavigatorParamList> = $Keys<SharingNavigatorParamList>,
> = RouteProp<GlobalParamList, RouteName>;

const Tab = createMaterialTopTabNavigator<
GlobalParamList,
SharingNavigatorParamList,
Expand Down