types: Switch to TsFlower for react-navigation types #5397

gnprice · 2022-06-01T20:42:15Z

(Marking this as draft only because it's stacked atop #5396.)

This replaces the type definitions we've had for react-navigation,
which derive from the flow-typed project with some changes and fixes
of our own, with new definitions based more directly on the
TypeScript type definitions found in the library itself upstream.

The Flow type definitions are first generated from the upstream TS
type definitions using TsFlower, a tool I recently wrote for this
purpose:
https://www.npmjs.com/package/tsflower

Then a stack of local patches is applied. There are 24 of these,
including one added for react-native-safe-area-context in the
previous PR. They comprise:

9 patches for things that could be fixed upstream, e.g. by adding
readonly. These are places where the TS definitions are less
accurate than they could be, but TS apparently takes the
distinctions less seriously than Flow does.
8 patches for things that could be fixed in TsFlower.
5 "irreducible" patches, where we fundamentally need a local
patch like this.
- 3 patches are because TS doesn't have a way to express a
  distinction that becomes important in Flow: exact object types,
  or covariant type aliases.
- 2 patches are because the types use a TS feature that there's
  no way to translate into Flow: conditional types. As a result,
  our patched-in versions are imperfect approximations, but I
  think good enough.
1 patch that just reformats a few definitions for readability.
This was useful in developing some of these changes.
1 patch (the one at the end) that fixes an issue I haven't fully
debugged.

The best way to read these patches is probably to use the command:

$ tools/tsflower unpack -s

in order to unpack them into a Git branch, and then read that branch
in the usual way. (The -s just makes the command faster, by not
rerunning TsFlower.)

Meanwhile, because these upstream-based type definitions are quite
different from the ones we had based on flow-typed, our own code
that interacts with react-navigation needs a number of small changes.

All of these changes are purely in the types, with no runtime
changes -- after all, we're using the same underlying library JS
code at runtime, just switching from one type-level description of
its API to a different one.

Many such changes are pulled out as prep commits, for changes that work already with the old definitions. Others happen simultaneously with the change of type definitions, in the main commit.

Fixes: #5391

chrisbobbe

Great! This is exciting. See a few comments below.

chrisbobbe · 2022-06-01T22:07:03Z

src/__flow-tests__/nav-test.js

@@ -66,7 +66,9 @@ function testNavigatorTypes() {
      <Stack.Screen name="Profile" component={Profile} />
      {/* $FlowExpectedError[incompatible-type] - mismatch of name with route prop */}
      <Stack.Screen name="Profile1" component={Profile12} />
-      {/* Should error but doesn't! on mismatch of name with navigation prop */}
+      {/* FAIL - Should error but doesn't! on mismatch of name with navigation prop */}


nav type tests: Add another test case for route names

In addition to adding a test case, this also changes the comment on an existing test case. What is that change about; is it just to emphasize the comment / make it more noticeable? Or is it changing the comment's meaning?

Hmm, I'd hoped the comment would be self-explanatory but I guess not. (Perhaps it becomes clearer with the later commit that touches this file?)

The intent isn't to change the meaning of the comment -- just to make it more visually scannable, as saying that this is a failing test / that its current behavior isn't what we would want it to be.

just to make it more visually scannable, as saying that this is a failing test / that its current behavior isn't what we would want it to be.

Cool, yeah; that seemed like the most likely explanation. So I shouldn't read it as coherent with the "Add another test case" part of the commit; it's just a small improvement you made while you were working on that nearby code.

chrisbobbe · 2022-06-01T22:14:17Z

src/__flow-tests__/nav-test.js

@@ -64,11 +64,11 @@ function testNavigatorTypes() {
    <Stack.Navigator>
      {/* Happy case is happy */}
      <Stack.Screen name="Profile" component={Profile} />
-      {/* $FlowExpectedError[incompatible-type] - mismatch of name with route prop */}
+      {/* FAIL - Should error but doesn't! on mismatch of name with route prop */}


types: Switch to TsFlower for react-navigation types

Same comment about changing the comment on an existing test case.

Right, so here we had had a test that successfully showed there was an error here, as there should be.

And on this change to upstream-based type definitions, the test starts failing. So the $FlowExpectedError goes away -- there's no longer an error -- and the comment switches to indicating that the desired behavior is something other than the current behavior.

Hmm, odd; for some reason I thought there was a second commit that just added FAIL - to this line (see previous comment for the first one), and that that second commit was this "Switch to TsFlower" one. I guess not. 🤷‍♂️ I agree that this one is unsurprising and self-explanatory.

chrisbobbe · 2022-06-01T22:35:24Z

src/__flow-tests__/nav-test.js

-      {/* $FlowExpectedError[incompatible-type] - mismatch of name with route prop */}
+      {/* FAIL - Should error but doesn't! on mismatch of name with route prop */}
      <Stack.Screen name="Profile1" component={Profile12} />
      {/* FAIL - Should error but doesn't! on mismatch of name with navigation prop */}
      <Stack.Screen name="Profile2" component={Profile12} />
-      {/* $FlowExpectedError[incompatible-type] - name that isn't even in the list */}
+      {/* $FlowExpectedError[prop-missing] - name that isn't even in the list */}


types: Switch to TsFlower for react-navigation types […] * Regression in a type-test: we no longer get an error in `<Foo.Screen …>` if the `name` prop doesn't agree with the name in the component's nav props. […] * The createFooNavigator functions now take just one type argument, not three; it corresponds to what was the first one.

Hmm, yeah, I see. We also lose type-checking on the part of a screen's interface that says which navigator(s) it can go on. For example, I've just tried transplanting PasswordAuthScreen from AppNavigator to the navigator in StreamTabsScreen (changing Stack.Screen to Tab.Screen), and didn't get an error. But I did get an error when I tried that at main, as expected. (However, it was a bad one that would've baffled me if I didn't already know I was misusing PasswordAuthScreen.)

Separately, I've just noticed that Flow doesn't complain if I make this change, either at the tip of the branch, or on main:

diff --git src/settings/DebugScreen.js src/settings/DebugScreen.js index 9fb9121a8..1361db10e 100644 --- src/settings/DebugScreen.js +++ src/settings/DebugScreen.js @@ -9,7 +9,7 @@ import type { AppNavigationProp } from '../nav/AppNavigator'; import Screen from '../common/Screen'; type Props = $ReadOnly<{| - navigation: AppNavigationProp<'debug'>, + navigation: AppNavigationProp<'asdfjkl;'>, route: RouteProp<'debug', void>, |}>;

It sure seems like it should, since 'asdfjkl;' isn't a subtype of $Keys<AppNavigatorParamList>. But I think that's something else/unrelated. Does that reproduce for you?

We also lose type-checking on the part of a screen's interface that says which navigator(s) it can go on.

I think we said on the call today that this is unfortunate but tolerable. Fixing the TypeScript types would take more time and effort than we can afford.

Separately, I've just noticed that Flow doesn't complain if I make this change, either at the tip of the branch, or on main:
…
It sure seems like it should, since 'asdfjkl;' isn't a subtype of $Keys<AppNavigatorParamList>. But I think that's something else/unrelated.

Yeah. That's this failing type-test in main:

{/* Should error but doesn't! on mismatch of name with navigation prop */} <Stack.Screen name="Profile2" component={Profile12} />

Or as it's worded at the tip of the PR, as discussed above:

{/* FAIL - Should error but doesn't! on mismatch of name with navigation prop */} <Stack.Screen name="Profile2" component={Profile12} />

(However, it was a bad one that would've baffled me if I didn't already know I was misusing PasswordAuthScreen.)

Ha, good to know. So I guess even the type-test that had been passing isn't that big of a regression, in practice.

Yeah. That's this failing type-test in main:

Hmm, really? In my example, I wrote something that shouldn't be considered a valid type at all: AppNavigationProp<'asdfjkl;'>. I don't see that happening in the "Should error but doesn't!" example.

Mmm, I see. Yeah, that's puzzling. I do reproduce (on main); and I don't know why Flow doesn't report an error there, given the bound on the parameter of AppNavigationProp:

export type AppNavigationProp< +RouteName: $Keys<AppNavigatorParamList> = $Keys<AppNavigatorParamList>, > = StackNavigationProp<GlobalParamList, RouteName>;

chrisbobbe · 2022-06-01T23:12:02Z

src/react-navigation.js

+  //     useNavigationInner<NavigationProp<GlobalParamList>>()
+  // But that gives a large number of puzzling errors.  And the one thing it
+  // actually does with the type argument is to determine the return type,
+  // anyway.  We know what that should be, so just handle it ourselves.


We know what that should be

In some sense, this isn't true, right; callers will be assuming some things about the returned object (e.g., it has special controls for the stack nav the caller is on). And even assuming that there will be a navigation object at all. See discussion. But we can't improve things in that direction just by changing how we call useNavigationInner here, so this solution seems fine.

chrisbobbe · 2022-06-01T23:33:37Z

types/patches/0013-irreducible-rnav-Covariant-type-parameters-on-variou.patch

+ export type StackNavigationProp<
+     ParamList: ParamListBase,
+-    RouteName: $Keys<ParamList>=string
+    +RouteName: $Keys<ParamList>=$Keys<ParamList>,


nit: does the change at the end of this line belong in

[upstream] rnav: Fix type-parameter defaults that don't meet bounds

?

Hmm yeah, I guess so -- there are two changes on this line.

chrisbobbe · 2022-06-01T23:43:25Z

types/patches/0020-tsflower-Manually-rewrite-Pick-from-TS-default-libra.patch

+-    stale: false,
+     ...
+-}>, "key" | "index" | "routeNames" | "history" | "type">> & Readonly<{
+}>> & Readonly<{


Did you mean to add the & ReadOnly<…> outside the Partial<…>, like this, or should it go inside? (Same in some other places in this patch.)

I think the & ReadOnly<…> was outside the Partial<…>, and stays that way. Here's a bit more context for reference:

-): { getInitialState: () => PromiseLike<Partial<Pick<Readonly<{ +): { getInitialState: () => PromiseLike<Partial<Readonly<{ // … ... -}>, "key" | "index" | "routeNames" | "history" | "type">> & Readonly<{ +}>> & Readonly<{

So it was:
Partial<Pick<Readonly<{ … }>, keys>> & Readonly<{

It becomes:
Partial<Readonly<{ … }>> & Readonly<{

Am I missing something there?

(It definitely was a bit tricky counting the angle brackets as I was writing this particular patch.)

Oh yeah, no, I think you're right, looking again.

chrisbobbe · 2022-06-01T23:46:42Z

types/patches/0020-tsflower-Manually-rewrite-Pick-from-TS-default-libra.patch

+         stale: false,
+         ...
+-    }>, "stale" | "key" | "index" | "routeNames" | "history" | "type"> & { routes: Pick<Route<string, {...} | void>, "name" | "params">[], ... } | void,
+    }> & { routes: SubsetProperties<Route<string, {...} | void>, {| name: mixed, params?: mixed |}>[], ... } | void,


Should this stuff go inside the Readonly<…>? (Here and in some other places in this patch.)

(Never mind; I think I was wrong about this.)

chrisbobbe · 2022-06-01T23:52:29Z

types/patches/0022-adhoc-rnav-Manually-approximate-conditional-in-Route.patch

+Subject: [adhoc] rnav: Manually approximate conditional in Route
+
+In the upstream definition, this is a conditional type that's basically
+
+    undefined extends T ? { readonly x?: T } : { readonly x: T }
+
+That is, it's like `{ readonly x: T }`, but if T contains undefined,
+then the object could have `x` missing entirely as well as having
+the value `undefined`.
+
+I don't think there's a way to write precisely that type in Flow.
+Here, manually approximate it as `{ +x?: T }` -- that is, make the
+parameter always optional.
+
+Alternatively, we could write `{ +x: T }`.  For code consuming the
+value, I think that's nearly indistinguishable in Flow from the
+upstream type.  But then code supplying such a value would need an
+`x` property.  Here, for the `params` property on a route, that'd
+probably get annoying.


This is reminding me of 49c7eef and its followup ad82c4c ; is there something relevant there?

Hmm, yeah, I guess Flow does have $If (as seen in the old code at the first of those two commits.)

... Or maybe it doesn't, and never did? https://flow.org/try just says "Cannot resolve name $If" if I try, and it's the same story if I try a bunch of older Flow versions there.

And I can't find any mention of it in the Flow source tree, even its history.

In fact, in the flow-typed project, its only appearance is in those react-navigation types.

So I suspect the story is that $If has never been a thing in Flow. And the author of those react-navigation libdefs was just fooled by the fact that until early 2021, Flow had a bug affecting libdefs where if a name couldn't be resolved it would suppress that error.

This replaces the type definitions we've had for react-navigation, which derive from the flow-typed project with some changes and fixes of our own, with new definitions based more directly on the TypeScript type definitions found in the library itself upstream. --- The Flow type definitions are first generated from the upstream TS type definitions using TsFlower, a tool I recently wrote for this purpose: https://www.npmjs.com/package/tsflower Then a stack of local patches is applied. There are 24 of these, including one added for react-native-safe-area-context in the previous PR. They comprise: * 9 patches for things that could be fixed upstream, e.g. by adding `readonly`. These are places where the TS definitions are less accurate than they could be, but TS apparently takes the distinctions less seriously than Flow does. * 8 patches for things that could be fixed in TsFlower. * 5 "irreducible" patches, where we fundamentally need a local patch like this. * 3 patches are because TS doesn't have a way to express a distinction that becomes important in Flow: exact object types, or covariant type aliases. * 2 patches are because the types use a TS feature that there's no way to translate into Flow: conditional types. As a result, our patched-in versions are imperfect approximations, but I think good enough. * 1 patch that just reformats a few definitions for readability. This was useful in developing some of these changes. * 1 patch (the one at the end) that fixes an issue I haven't fully debugged. The best way to read these patches is probably to use the command: $ tools/tsflower unpack -s in order to unpack them into a Git branch, and then read that branch in the usual way. (The `-s` just makes the command faster, by not rerunning TsFlower.) --- Meanwhile, because these upstream-based type definitions are quite different from the ones we had based on flow-typed, our own code that interacts with react-navigation needs a number of small changes. All of these changes are purely in the types, with no runtime changes -- after all, we're using the same underlying library JS code at runtime, just switching from one type-level description of its API to a different one. I pulled out a number of such changes as prep commits in the branch leading up to this one, for changes that worked already with the old definitions. The changes in this commit, happening simultaneously with the change of type definitions, include: * Some fixmes and expected errors get a new error code. * Two fixmes in EditStreamCard go away. * New, quite appropriate, fixmes in AppNavigator. * Regression in a type-test: we no longer get an error in `<Foo.Screen …>` if the `name` prop doesn't agree with the name in the component's nav props. Though perhaps not that big a regression in practice, because the error you'd get if you made this mistake in the app code was a baffling one: zulip#5397 (comment) * The new type of `useNavigation` is pretty useless. But we already have a type-wrapper anyway; just update it. * The type StackAction is now called StackActionType. The old name seems better (values of that type are stack actions, not types of stack actions); but so it goes. * NavigationState now takes type arguments; the defaults are fine, but we need an explicit `<>`. * The createFooNavigator functions now take just one type argument, not three; it corresponds to what was the first one.

gnprice · 2022-06-02T23:30:42Z

Thanks for the review! Replies above, and revision pushed.

gnprice · 2022-06-04T01:43:02Z

And pushed another revision where the type definitions are auto-formatted -- atop #5399, with the patches rebased over the auto-formatting using the technique described there. (As discussed at #5396 (comment) .)

gnprice · 2022-06-06T19:34:40Z

Marking ready, as the preceding PRs are merged.

chrisbobbe · 2022-06-06T21:11:23Z

Thanks! I've just noticed that in some cases, the object type for a React component's props is inexact, meaning Flow lets you pass nonsense props like foo={123}. This is better than letting you pass a wrong type for an expected prop, but I wonder if it's something we should try to fix. I've noticed it on <Stack.Navigator … /> and on <NavigationContainer … /> so far.

For <Stack.Navigator … />, the following gives me the "property foo is missing" error I'd been hoping for:

diff --git src/__flow-tests__/nav-test.js src/__flow-tests__/nav-test.js
index ba7921277..b0f5f5ba4 100644
--- src/__flow-tests__/nav-test.js
+++ src/__flow-tests__/nav-test.js
@@ -61,7 +61,7 @@ function testNavigatorTypes() {
 
   const Stack = createStackNavigator<NavParamList, NavParamList, NavigationProp<>>();
 
-    <Stack.Navigator>
+    <Stack.Navigator foo={123}>
       {/* Happy case is happy */}
       <Stack.Screen name="Profile" component={Profile} />
       {/* FAIL - Should error but doesn't! on mismatch of name with route prop */}
diff --git types/@react-navigation/core/lib/typescript/src/types.js.flow types/@react-navigation/core/lib/typescript/src/types.js.flow
index b1232ee31..e5baf28c0 100644
--- types/@react-navigation/core/lib/typescript/src/types.js.flow
+++ types/@react-navigation/core/lib/typescript/src/types.js.flow
@@ -24,7 +24,7 @@ import {
 export type DefaultNavigatorOptions<
   ScreenOptions: { ... },
   ParamList: ParamListBase = ParamListBase,
-> = DefaultRouterOptions<string> & {
+> = DefaultRouterOptions<string> & {|
   children: $tsflower_subst$React$ReactNode,
   screenOptions?:
     | ScreenOptions
@@ -41,8 +41,7 @@ export type DefaultNavigatorOptions<
         options: ScreenOptions,
         ...
       }) => ScreenOptions),
-  ...
-};
+|};
 
 export type EventMapBase = {
   +[key: string]: {|
diff --git types/@react-navigation/routers/lib/typescript/src/types.js.flow types/@react-navigation/routers/lib/typescript/src/types.js.flow
index 780a9b08f..6ddd25ca6 100644
--- types/@react-navigation/routers/lib/typescript/src/types.js.flow
+++ types/@react-navigation/routers/lib/typescript/src/types.js.flow
@@ -84,10 +84,9 @@ export type ActionCreators<Action: NavigationAction> = {
   [key: string]: (...args: any) => Action,
   ...,
 };
-export type DefaultRouterOptions<RouteName: string = string> = {
+export type DefaultRouterOptions<RouteName: string = string> = {|
   initialRouteName?: RouteName,
-  ...
-};
+|};
 export type RouterFactory<
   State: NavigationState<>,
   Action: NavigationAction,

…but it also gives me 59 other errors.

gnprice · 2022-06-06T22:12:24Z

I've just noticed that in some cases, the object type for a React component's props is inexact, meaning Flow lets you pass nonsense props like foo={123}.

Yeah, that's the TypeScript semantics.

I believe TS goes on to try to mitigate that by attempting some kind of warning or error when you pass extra properties, even though they don't recognize the exact/inexact distinction in the type system and so it's impossible to do that soundly. I'm not sure of the details of that.

This is better than letting you pass a wrong type for an expected prop, but I wonder if it's something we should try to fix. […]

For <Stack.Navigator … />, the following gives me the "property foo is missing" error I'd been hoping for:
[…]
…but it also gives me 59 other errors.

Yeah. You're now intersecting two exact object types that have different sets of properties, so there are no objects that belong to both sides of the intersection, meaning there are none that belong to that intersection type. (In particular, any such object must have a children property to satisfy the second member of the intersection; but by the first member, it can't have any properties other than initialRouteName.) Which isn't the intended result.

The Flow idiom here is to use spreads instead of intersections, and then those work nicely with exact as well as inexact object types. That does make the patch a bit more invasive, though.

I think it could be reasonable to try to add a patch that makes the props types exact. Because these props types are constructed from complicated expressions, the simplest way for that here might be to add an $Exact just inside the React.ComponentType in TypedNavigator, rather than to make all the object-type literals that go into it be exact and convert all the intersections to spreads.

chrisbobbe · 2022-06-06T23:22:14Z

That does make the patch a bit more invasive, though.

Hmm, yeah. I'm excited to use TsFlower's "push button, receive types" service for our other dependencies (particularly those from Expo), and I think this is a fine place to let the output sit as it is. We can always add to the patch in the future if we decide it's worth it.

So, with that in mind, this looks good! Please merge at will.

…ories This smooths the path slightly toward generated versions of these definitions, which will naturally go into directories with the same name as the package.

…onContainer Upstream doesn't give a name to the type of this value it exports. But we can take `typeof` of it. And because the only exported name that refers to is the name of a value -- and the two type definitions must agree about what names refer to values, because that's part of the runtime reality that they're both just seeking to describe -- that approach works already with our existing type definitions.

With the type definitions from upstream, this type parameter becomes mandatory. It's also good to just make this explicit, because it's quite different from the rest of our navigators.

Also where we have a test case that's currently failing -- i.e. the current behavior isn't what we actually want -- make the comment explicit about that in a more visually scannable way, with "FAIL".

These don't seem like they should be necessary. But they're clearly correct; and without them, after we switch to types translated by TsFlower from upstream, we get an error at this navigation.navigate call. Its parameter type is a union of object types, and Flow can't decide (without this help) which branch of the union to try.

These exist because our existing type definitions for react-navigation repeat most of their contents in the file for each package. (Which is a legacy from flow-typed, where for a long time it was believed to be impossible for one libdef to import types from another.) We're about to replace these definitions with versions generated from the upstream libraries' own TS type definitions. Those won't have this redundancy, so this type we're testing will be exported only by the one package that actually defines it. Delete the tests that will stop making sense.

This replaces the type definitions we've had for react-navigation, which derive from the flow-typed project with some changes and fixes of our own, with new definitions based more directly on the TypeScript type definitions found in the library itself upstream. --- The Flow type definitions are first generated from the upstream TS type definitions using TsFlower, a tool I recently wrote for this purpose: https://www.npmjs.com/package/tsflower The output is auto-formatted, and then a stack of local patches is applied. There are 24 of these, including one added for react-native-safe-area-context in the previous PR. They comprise: * 9 patches for things that could be fixed upstream, e.g. by adding `readonly`. These are places where the TS definitions are less accurate than they could be, but TS apparently takes the distinctions less seriously than Flow does. * 8 patches for things that could be fixed in TsFlower. * 5 "irreducible" patches, where we fundamentally need a local patch like this. * 3 patches are because TS doesn't have a way to express a distinction that becomes important in Flow: exact object types, or covariant type aliases. * 2 patches are because the types use a TS feature that there's no way to translate into Flow: conditional types. As a result, our patched-in versions are imperfect approximations, but I think good enough. * 1 patch that just reformats a few definitions for readability. This was useful in developing some of these changes. * 1 patch (the one at the end) that fixes an issue I haven't fully debugged. The best way to read these patches is probably to use the command: $ tools/tsflower unpack -s in order to unpack them into a Git branch, and then read that branch in the usual way. (The `-s` just makes the command faster, by not rerunning TsFlower.) --- Meanwhile, because these upstream-based type definitions are quite different from the ones we had based on flow-typed, our own code that interacts with react-navigation needs a number of small changes. All of these changes are purely in the types, with no runtime changes -- after all, we're using the same underlying library JS code at runtime, just switching from one type-level description of its API to a different one. I pulled out a number of such changes as prep commits in the branch leading up to this one, for changes that worked already with the old definitions. The changes in this commit, happening simultaneously with the change of type definitions, include: * Some fixmes and expected errors get a new error code. * Two fixmes in EditStreamCard go away. * New, quite appropriate, fixmes in AppNavigator. * Regression in a type-test: we no longer get an error in `<Foo.Screen …>` if the `name` prop doesn't agree with the name in the component's nav props. Though perhaps not that big a regression in practice, because the error you'd get if you made this mistake in the app code was a baffling one: zulip#5397 (comment) * The new type of `useNavigation` is pretty useless. But we already have a type-wrapper anyway; just update it. * The type StackAction is now called StackActionType. The old name seems better (values of that type are stack actions, not types of stack actions); but so it goes. * NavigationState now takes type arguments; the defaults are fine, but we need an explicit `<>`. * The createFooNavigator functions now take just one type argument, not three; it corresponds to what was the first one.

gnprice · 2022-06-07T01:46:16Z

Thanks for the review! Done.

gnprice added a-tools a-navigation labels Jun 1, 2022

gnprice mentioned this pull request Jun 2, 2022

types: Add TsFlower tool, a test suite, and a small demo use #5396

Merged

chrisbobbe reviewed Jun 2, 2022

View reviewed changes

gnprice force-pushed the pr-tsflower-rnav branch 3 times, most recently from 1102057 to 368318f Compare June 3, 2022 00:22

gnprice mentioned this pull request Jun 4, 2022

types: Auto-format TsFlower-generated type definitions #5399

Merged

gnprice force-pushed the pr-tsflower-rnav branch from 368318f to e782a8e Compare June 4, 2022 01:41

gnprice marked this pull request as ready for review June 6, 2022 19:34

gnprice added 11 commits June 6, 2022 18:43

dep types [nfc]: Move react-nav definitions inside per-package direct…

b392c56

…ories This smooths the path slightly toward generated versions of these definitions, which will naturally go into directories with the same name as the package.

nav types: Mark all FooParamList properties read-only

419e851

nav types: Add a redundant-seeming intersection that will fix errors

afde965

nav types: Provide param-list type at MessageReactionsScreen

e020b48

With the type definitions from upstream, this type parameter becomes mandatory. It's also good to just make this explicit, because it's quite different from the rest of our navigators.

nav type tests: Add another test case for route names

cba96fd

Also where we have a test case that's currently failing -- i.e. the current behavior isn't what we actually want -- make the comment explicit about that in a more visually scannable way, with "FAIL".

dep types: Add stub for react-native-gesture-handler

bb70be0

dep types: Add stub for react-native-reanimated

694c6c3

gnprice force-pushed the pr-tsflower-rnav branch from e782a8e to 067a450 Compare June 7, 2022 01:45

gnprice merged commit 067a450 into zulip:main Jun 7, 2022

gnprice deleted the pr-tsflower-rnav branch June 7, 2022 01:47

chrisbobbe mentioned this pull request Jun 8, 2022

Show custom profile fields in users' profiles #5398

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

types: Switch to TsFlower for react-navigation types #5397

types: Switch to TsFlower for react-navigation types #5397

gnprice commented Jun 1, 2022

chrisbobbe left a comment

chrisbobbe Jun 1, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 2, 2022 •

edited

Loading

chrisbobbe Jun 1, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 2, 2022

chrisbobbe Jun 1, 2022

chrisbobbe Jun 2, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 2, 2022 •

edited

Loading

gnprice Jun 3, 2022

chrisbobbe Jun 1, 2022

chrisbobbe Jun 1, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 1, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 6, 2022

chrisbobbe Jun 1, 2022

chrisbobbe Jun 6, 2022

chrisbobbe Jun 1, 2022

gnprice Jun 2, 2022

chrisbobbe Jun 2, 2022

gnprice commented Jun 2, 2022

gnprice commented Jun 4, 2022

gnprice commented Jun 6, 2022

chrisbobbe commented Jun 6, 2022

gnprice commented Jun 6, 2022

chrisbobbe commented Jun 6, 2022 •

edited

Loading

gnprice commented Jun 7, 2022

types: Switch to TsFlower for react-navigation types #5397

types: Switch to TsFlower for react-navigation types #5397

Conversation

gnprice commented Jun 1, 2022

chrisbobbe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice commented Jun 2, 2022

gnprice commented Jun 4, 2022

gnprice commented Jun 6, 2022

chrisbobbe commented Jun 6, 2022

gnprice commented Jun 6, 2022

chrisbobbe commented Jun 6, 2022 • edited Loading

gnprice commented Jun 7, 2022

chrisbobbe Jun 2, 2022 •

edited

Loading

chrisbobbe Jun 2, 2022 •

edited

Loading

chrisbobbe commented Jun 6, 2022 •

edited

Loading