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

flow: Enable Types-First mode, on the path to RN v0.64 upgrade. #4987

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented Sep 3, 2021

From Flow's perspective, we're fine to delay types-first until we're
on v0.143, which we'd normally take along with the RN v0.65 upgrade.

But, from experimentation, React Native v0.64's internal code seems
to have been written with Types-First, such that a few Flow errors
would show up if we tried to use RN v0.64 without Types-First
enabled. So, enable it, now that we can, and now that the RN v0.64
upgrade is on the horizon (#4426).

Fixes: #4907

As it says in the comment, Flow has some trouble with the more
complex type in types-first mode. It would give some wrong-looking
errors:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ flow-typed/@react-navigation/stack_v5.x.x.js:844:22

object type [1] is incompatible with undefined [2] in the first argument. [incompatible-type]

     flow-typed/@react-navigation/stack_v5.x.x.js
 [2] 420│   declare type $IsUndefined<X> = $IsA<X, void>;
        :
     841│     +setOptions: (options: $Shape<ScreenOptions>) => void,
     842│     +setParams: (
     843│       params: $If<
     844│         $IsUndefined<$ElementType<ParamList, RouteName>>,
     845│         empty,
     846│         $Shape<$NonMaybeType<$ElementType<ParamList, RouteName>>>,
     847│       >,

     src/nav/AppNavigator.js
 [1]  53│   chat: RouteParamsOf<typeof ChatScreen>,

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/chat/ChatScreen.js:113:57

Cannot call navigation.setParams with object literal bound to params because object literal [1] is incompatible with
empty [2]. [incompatible-call]

     src/chat/ChatScreen.js
     110│
     111│   const { narrow, editMessage } = route.params;
     112│   const setEditMessage = useCallback(
 [1] 113│     (value: EditMessage | null) => navigation.setParams({ editMessage: value }),
     114│     [navigation],
     115│   );
     116│

     flow-typed/@react-navigation/stack_v5.x.x.js
 [2] 843│       params: $If<
     844│         $IsUndefined<$ElementType<ParamList, RouteName>>,
     845│         empty,
     846│         $Shape<$NonMaybeType<$ElementType<ParamList, RouteName>>>,
     847│       >,
@chrisbobbe chrisbobbe changed the title flow: Enable Types-First mode! flow: Enable Types-First mode, on the path to RN v0.64 upgrade. Sep 3, 2021
From Flow's perspective, we're fine to delay types-first until we're
on v0.143, which we'd normally take along with the RN v0.65 upgrade.

But, from experimentation, React Native v0.64's internal code seems
to have been written with Types-First, such that a few Flow errors
would show up if we tried to use RN v0.64 without Types-First
enabled. So, enable it, now that we can, and now that the RN v0.64
upgrade is on the horizon (zulip#4426).

Fixes: zulip#4907
@gnprice
Copy link
Member

gnprice commented Sep 3, 2021

🎊 Thanks! Looks good -- merging.

@gnprice gnprice merged commit 4774b91 into zulip:master Sep 3, 2021
@chrisbobbe
Copy link
Contributor Author

Yay, thanks for the review!

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

Successfully merging this pull request may close these issues.

Flow: Switch to new "Types-First" mode.
2 participants