From b86d2bf4155ec6e3cb3776617b206056a97968ac Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 May 2020 14:38:12 -0700 Subject: [PATCH 1/2] MainTabs [nfc]: Prepare for and explain some FlowFixMes. The next commit's message is quite long, so here's the motivation for the FlowFixMes. Importing `createBottomTabNavigator` from `react-navigation` is good because it means more type checking! Sadly, getting that to really work in all the ways we want won't be easy. To sum up, we could get *part* of the way there by making tweaks to our typed `connect` (and making sure those tweaks didn't break something or lose type-checking elsewhere!). But there's an additional hurdle in something that's not quite right in the `react-navigation` libdef. The same goes for `createMaterialTopTabNavigator`. Greg goes into more detail at the PR thread [1]. See also discussion [2]. [1]: https://github.com/zulip/zulip-mobile/pull/4114#issuecomment-634255590 [2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/react-navigation.20types/near/878259 --- src/main/MainTabs.js | 9 ++++++++- src/main/StreamTabs.js | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/MainTabs.js b/src/main/MainTabs.js index aecb99194bc..754ee15b37c 100644 --- a/src/main/MainTabs.js +++ b/src/main/MainTabs.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import React from 'react'; import { Platform } from 'react-native'; -import { createBottomTabNavigator } from 'react-navigation-tabs'; +import { createBottomTabNavigator } from 'react-navigation'; import type { TabNavigationOptionsPropsType } from '../types'; import tabsOptions from '../styles/tabs'; @@ -15,8 +15,10 @@ import IconUnreadConversations from '../nav/IconUnreadConversations'; import ProfileCard from '../account-info/ProfileCard'; export default createBottomTabNavigator( + // We'll activate these fixmes in the next commit. { home: { + // £FlowFixMe `navigationOptions` property on component type screen: HomeTab, navigationOptions: { tabBarLabel: 'Home', @@ -26,6 +28,8 @@ export default createBottomTabNavigator( }, }, streams: { + // No fixme needed; StreamTabs is a navigator from + // `createMaterialTopTabNavigator`. screen: StreamTabs, navigationOptions: { tabBarLabel: 'Streams', @@ -35,6 +39,7 @@ export default createBottomTabNavigator( }, }, conversations: { + // £FlowFixMe `navigationOptions` property on component type screen: PmConversationsCard, navigationOptions: { tabBarLabel: 'Conversations', @@ -44,6 +49,7 @@ export default createBottomTabNavigator( }, }, settings: { + // £FlowFixMe `navigationOptions` property on component type screen: SettingsCard, navigationOptions: { tabBarLabel: 'Settings', @@ -53,6 +59,7 @@ export default createBottomTabNavigator( }, }, profile: { + // £FlowFixMe `navigationOptions` property on component type screen: ProfileCard, navigationOptions: { tabBarLabel: 'Profile', diff --git a/src/main/StreamTabs.js b/src/main/StreamTabs.js index f28708b986c..de530fb8236 100644 --- a/src/main/StreamTabs.js +++ b/src/main/StreamTabs.js @@ -18,8 +18,10 @@ const styles = createStyleSheet({ }); export default createMaterialTopTabNavigator( + // We'll activate these fixmes in the next commit. { subscribed: { + // £FlowFixMe `navigationOptions` property on component type screen: SubscriptionsCard, navigationOptions: { tabBarLabel: (props: TabNavigationOptionsPropsType) => ( @@ -30,6 +32,7 @@ export default createMaterialTopTabNavigator( }, }, allStreams: { + // £FlowFixMe `navigationOptions` property on component type screen: StreamListCard, navigationOptions: { tabBarLabel: (props: TabNavigationOptionsPropsType) => ( From 31b59f0ceb418de1aa7206c06f1ae27a74a05aaf Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 15 May 2020 14:19:23 -0700 Subject: [PATCH 2/2] deps: Remove `react-navigation-tabs`. See the previous commit for the reason for the $FlowFixMes. In 0d2066f41, we followed a warning to use the `react-native-tabs` package: ``` console.warn node_modules/react-navigation/src/react-navigation.js:180 TabBarBottom is deprecated. Please use the react-navigation-tabs package instead: https://github.com/react-navigation/react-navigation-tabs ``` An important bit of context that's missing from this warning is that `react-navigation-tabs`'s function `createBottomTabNavigator`, specifically, is what replaces what was previously done with `TabBarBottom`. `react-navigation` also exports another function `createBottomTabNavigator`. So, is it so important to use the export from `react-navigation-tabs`? Turns out it's not; that function in `react-navigation` is an extremely thin wrapper on the one in `react-navigation-tabs`: In node_modules/react-navigation/src/react-navigation.js:69: ``` get createBottomTabNavigator() { return require('react-navigation-tabs').createBottomTabNavigator; }, ``` So, `react-navigation` depends on `react-navigation-tabs`, so we can remove the latter as a direct dependency in our project. The `react-navigation` v2 doc [1] also doesn't say anything about needing to depend directly on `react-navigation-tabs`. See also further discussion [2]. It would also have been possible to remove the direct dependency on `react-native-screens`, but we're planning to use that later; that's issue #4111. The same story applies to `createMaterialTopTabNavigator`. [1]: https://reactnavigation.org/docs/2.x/tab-based-navigation/ [2]: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/iOS.3A.20r-n-screens.20.2F.20r-n-document-picker/near/877185 --- .../npm/react-navigation-tabs_vx.x.x.js | 178 ------------------ package.json | 1 - src/main/MainTabs.js | 9 +- src/main/StreamTabs.js | 7 +- 4 files changed, 7 insertions(+), 188 deletions(-) delete mode 100644 flow-typed/npm/react-navigation-tabs_vx.x.x.js diff --git a/flow-typed/npm/react-navigation-tabs_vx.x.x.js b/flow-typed/npm/react-navigation-tabs_vx.x.x.js deleted file mode 100644 index b33140d4bd1..00000000000 --- a/flow-typed/npm/react-navigation-tabs_vx.x.x.js +++ /dev/null @@ -1,178 +0,0 @@ -// flow-typed signature: 66582a0e4da4613bd0cd2f73b6f3df6c -// flow-typed version: <>/react-navigation-tabs_v0.8.4/flow_v0.92.0 - -/** - * This is an autogenerated libdef stub for: - * - * 'react-navigation-tabs' - * - * Fill this stub out by replacing all the `any` types. - * - * Once filled out, we encourage you to share your work with the - * community by sending a pull request to: - * https://github.com/flowtype/flow-typed - */ - -declare module 'react-navigation-tabs' { - declare module.exports: any; -} - -/** - * We include stubs for each file inside this npm package in case you need to - * require those files directly. Feel free to delete any files that aren't - * needed. - */ -declare module 'react-navigation-tabs/createBottomTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/createMaterialTopTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/navigators/createBottomTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/navigators/createMaterialBottomTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/navigators/createMaterialTopTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/utils/createTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/utils/withDimensions' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/views/BottomTabBar' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/views/CrossFadeIcon' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/views/MaterialTopTabBar' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/dist/views/ResourceSavingScene' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/navigators/createBottomTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/navigators/createMaterialTopTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/utils/createTabNavigator' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/utils/withDimensions' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/views/BottomTabBar' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/views/CrossFadeIcon' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/views/MaterialTopTabBar' { - declare module.exports: any; -} - -declare module 'react-navigation-tabs/src/views/ResourceSavingScene' { - declare module.exports: any; -} - -// Filename aliases -declare module 'react-navigation-tabs/createBottomTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/createBottomTabNavigator'>; -} -declare module 'react-navigation-tabs/createMaterialTopTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/createMaterialTopTabNavigator'>; -} -declare module 'react-navigation-tabs/dist/index' { - declare module.exports: $Exports<'react-navigation-tabs/dist'>; -} -declare module 'react-navigation-tabs/dist/index.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist'>; -} -declare module 'react-navigation-tabs/dist/navigators/createBottomTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/navigators/createBottomTabNavigator'>; -} -declare module 'react-navigation-tabs/dist/navigators/createMaterialBottomTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/navigators/createMaterialBottomTabNavigator'>; -} -declare module 'react-navigation-tabs/dist/navigators/createMaterialTopTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/navigators/createMaterialTopTabNavigator'>; -} -declare module 'react-navigation-tabs/dist/utils/createTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/utils/createTabNavigator'>; -} -declare module 'react-navigation-tabs/dist/utils/withDimensions.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/utils/withDimensions'>; -} -declare module 'react-navigation-tabs/dist/views/BottomTabBar.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/views/BottomTabBar'>; -} -declare module 'react-navigation-tabs/dist/views/CrossFadeIcon.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/views/CrossFadeIcon'>; -} -declare module 'react-navigation-tabs/dist/views/MaterialTopTabBar.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/views/MaterialTopTabBar'>; -} -declare module 'react-navigation-tabs/dist/views/ResourceSavingScene.js' { - declare module.exports: $Exports<'react-navigation-tabs/dist/views/ResourceSavingScene'>; -} -declare module 'react-navigation-tabs/src/index' { - declare module.exports: $Exports<'react-navigation-tabs/src'>; -} -declare module 'react-navigation-tabs/src/index.js' { - declare module.exports: $Exports<'react-navigation-tabs/src'>; -} -declare module 'react-navigation-tabs/src/navigators/createBottomTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/navigators/createBottomTabNavigator'>; -} -declare module 'react-navigation-tabs/src/navigators/createMaterialTopTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/navigators/createMaterialTopTabNavigator'>; -} -declare module 'react-navigation-tabs/src/utils/createTabNavigator.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/utils/createTabNavigator'>; -} -declare module 'react-navigation-tabs/src/utils/withDimensions.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/utils/withDimensions'>; -} -declare module 'react-navigation-tabs/src/views/BottomTabBar.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/views/BottomTabBar'>; -} -declare module 'react-navigation-tabs/src/views/CrossFadeIcon.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/views/CrossFadeIcon'>; -} -declare module 'react-navigation-tabs/src/views/MaterialTopTabBar.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/views/MaterialTopTabBar'>; -} -declare module 'react-navigation-tabs/src/views/ResourceSavingScene.js' { - declare module.exports: $Exports<'react-navigation-tabs/src/views/ResourceSavingScene'>; -} diff --git a/package.json b/package.json index 27a04b5780b..a91d7d8a548 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,6 @@ "react-native-webview": "~10.0.0", "react-navigation": "^2.18.3", "react-navigation-redux-helpers": "^2.0.9", - "react-navigation-tabs": "0.8.4", "react-redux": "^5.0.7", "redux": "^4.0.0", "redux-action-buffer": "^1.2.0", diff --git a/src/main/MainTabs.js b/src/main/MainTabs.js index 754ee15b37c..a33dfb26f42 100644 --- a/src/main/MainTabs.js +++ b/src/main/MainTabs.js @@ -15,10 +15,9 @@ import IconUnreadConversations from '../nav/IconUnreadConversations'; import ProfileCard from '../account-info/ProfileCard'; export default createBottomTabNavigator( - // We'll activate these fixmes in the next commit. { home: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: HomeTab, navigationOptions: { tabBarLabel: 'Home', @@ -39,7 +38,7 @@ export default createBottomTabNavigator( }, }, conversations: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: PmConversationsCard, navigationOptions: { tabBarLabel: 'Conversations', @@ -49,7 +48,7 @@ export default createBottomTabNavigator( }, }, settings: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: SettingsCard, navigationOptions: { tabBarLabel: 'Settings', @@ -59,7 +58,7 @@ export default createBottomTabNavigator( }, }, profile: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: ProfileCard, navigationOptions: { tabBarLabel: 'Profile', diff --git a/src/main/StreamTabs.js b/src/main/StreamTabs.js index de530fb8236..0c5d537b39f 100644 --- a/src/main/StreamTabs.js +++ b/src/main/StreamTabs.js @@ -2,7 +2,7 @@ import React from 'react'; import { Text } from 'react-native'; import { FormattedMessage } from 'react-intl'; -import { createMaterialTopTabNavigator } from 'react-navigation-tabs'; +import { createMaterialTopTabNavigator } from 'react-navigation'; import type { TabNavigationOptionsPropsType } from '../types'; import { createStyleSheet } from '../styles'; @@ -18,10 +18,9 @@ const styles = createStyleSheet({ }); export default createMaterialTopTabNavigator( - // We'll activate these fixmes in the next commit. { subscribed: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: SubscriptionsCard, navigationOptions: { tabBarLabel: (props: TabNavigationOptionsPropsType) => ( @@ -32,7 +31,7 @@ export default createMaterialTopTabNavigator( }, }, allStreams: { - // £FlowFixMe `navigationOptions` property on component type + // $FlowFixMe `navigationOptions` property on component type screen: StreamListCard, navigationOptions: { tabBarLabel: (props: TabNavigationOptionsPropsType) => (