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

.eslintignore

@@ -1,2 +1,18 @@
+# If you run `tools/test` when you've modified a file that's ignored
+# here, you might get a warning like this:
+#   0:0  warning  File ignored because of a matching ignore pattern. […]
+#
+# Don't worry about that warning; it won't appear in CI, and it won't
+# appear on future `tools/test` runs when not editing these files.
+# For more discussion, see:
+#   https://github.com/zulip/zulip-mobile/pull/4430#issuecomment-767297445
+
+# These are purely type definitions, no runtime code.  Most of them
+# are third-party code, too, so naturally don't match our style.
 **/flow-typed/**
-**/__flow-tests__/**
+
+# These are type-tests, made up of code that gets type-checked but
+# never actually run.  They're naturally full of dead code which
+# ESLint would complain about; and because the code never runs, other
+# things it might complain about don't really matter anyway.
+**/__flow-tests__/**

src/__flow-tests__/README.md

@@ -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.

src/__flow-tests__/nav-test.js

@@ -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>
+      {/* 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>;
+}

src/react-navigation.js

@@ -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.
  *

src/sharing/ShareToPm.js

@@ -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';
@@ -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,

src/sharing/ShareToStream.js

@@ -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';
@@ -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>,

src/sharing/SharingScreen.js

@@ -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';
@@ -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,