Upgrade react-native-webview to latest, 11.6.4, on the path to the RN 64 upgrade.

[chrisbobbe]

We'd been on a version 10.x, a previous major version, so this wasn't done automatically with our recent yarn upgrade PRs.

The RN 64 upgrade issue is #4426.

[chrisbobbe]

I tested with the office Android device and my iPhone, and didn't notice any differences in behavior. Messages appear, and reactions are processed, so inbound events still seem to be working as before.

[chrisbobbe]

Just fixed a conflict.

[WesleyAC]

Tested on Android and it seems fine, LGTM!

[chrisbobbe]

Thanks for the review! Merged.

[gnprice]

Thanks to both of you!

Comments below; my only question is about a type annotation we've found tricky.

[gnprice]

I remember there being some uncertainty about how these types for React.createRef should look. I grepped for examples, and we only have a couple of them but it looks like they're like this but without the typeof. So probably that's what we want?

Looking at history with git log --stat -p -G 'React.createRef<React', I find d346073 switching something to the non-typeof form, and that links to #4278 (comment) .

But then, the form we switched away from there wasn't this one -- it had the typeof and not the React$ElementRef. We switched to having the React$ElementRef and not the typeof. Maybe we do want both? Rereading that linked comment, I'm not sure.

How did you end up with this form?

[chrisbobbe]

I think the React$ElementRef is right, especially looking back at your explanation at that comment. Are you having doubts about that?

If I remove the typeof, I get the following error:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/webview/MessageList.js:161:49

Cannot use WebView as a type. A name can be used as a type only if it refers to a type, interface, class, or enum
definition. To get the type of a non-class value, use typeof. [value-as-type]

     158│   static contextType = ThemeContext;
     159│   context: ThemeData;
     160│
     161│   webviewRef = React.createRef<React$ElementRef<WebView>>();
     162│   sendInboundEventsIsReady: boolean;
     163│   unsentInboundEvents: WebViewInboundEvent[] = [];
     164│

Looking at the libdef, we're saying the 'react-native-webview' module exports a value at the named export WebView with a particular type, and indeed, it doesn't use class, interface, etc.:

declare export var WebView: React$ComponentType<WebViewProps>;

If we had instead done declare export class WebView …, then I suspect we wouldn't have to do typeof. The trouble is that we'd want it to be declare export class WebView extends React.Component<WebViewProps, …>, and I don't think we can get React.Component into the libdef without just copying it. @types/react has a TypeScript definition here that we could translate; probably we could find a definition already in Flow to copy if we wanted.

A kind of important thing we drop by saying the WebView value has the type React$ComponentType<WebViewProps> is the postMessage method.

[chrisbobbe]

In fact, looking again, I think this bit at the end of react-native-webview isn't quite correct:

  //
  // node_modules/react-native-webview/lib/WebView.d.ts
  //

  declare export type WebViewProps = $ReadOnly<{| ...IOSWebViewProps, ...AndroidWebViewProps |}>;
  declare export var WebView: React$ComponentType<WebViewProps>;
  declare export default WebView;

Instead of copying/translating from WebView.d.ts (see this comment), we should be using WebView.ios.d.ts and WebView.android.d.ts. Those use declare class WebView extends React.Component<…>. The question remains, though, of how best to get the React.Component definition. Also, I guess, how to tell Flow which type we'll get on Android, and which on iOS.

[gnprice]

I think the React$ElementRef is right, especially looking back at your explanation at that comment. Are you having doubts about that?

Yeah, I'm convinced of that. The question I'm thinking of is whether we want the typeof.

Going and rereading the React libdef (in /tmp/flow/flowlib_*/react.js -- it's built into Flow) again, I think we do. So I think this code is right, and our handful of other uses of createRef should switch to looking like this too. I guess I'll go try that and see what happens.

First, though, to write down my reasoning:

• What we're trying to express here is what the type of the current property is going to be, when it's not null.
• The type of the ref pseudoprop says the answer needs to look like React$ElementRef<ElementType>, for some ElementType that's a subtype of React$ElementType. (That's from React$Ref, which I quoted in Decouple Navigation from Redux (Part 2) #4278 (comment) .)
• OK, so what's an appropriate type to use for ElementType here? Does it look like WebView, or like typeof WebView?
• Well, one thing is that WebView isn't even a type -- it's just a value. And in general, when a React component-type is a class, then it's a type as well as a value… but when it's a function, it's only a value.
  • Hmm, I guess ref can't be used on function components anyway. But this is a generic React API, so it must work the same for React DOM; and there, there's a third type of React element, namely where the element-type is just a string and it makes an HTML element with that string as the HTML element type. And those do support refs.
  • So in the HTML case, the element-type in the actual React element that render returns is a string, like 'div'. And that isn't a type -- you can't say React$ElementRef<'div'>. So it must be React$ElementRef<typeof 'div'>.
  • In our case, the element-type in the actual React element is WebView, the class itself. So the parallel thing would be React$ElementRef<typeof WebView>.
• Separately: when the element-type is a class, the value of current will be an instance of that class. But when the element-type is a string, the value of current will be a DOM element. How is that getting expressed?
  • I think the answer must be that that's the job of React$ElementRef. It's not defined in that react.js libdef -- it must be defined effectively by logic in the Flow implementation itself.
  • That also provides an explanation for why there's that React$ElementRef indirection in the first place. It's there to solve the problem that "class -> instance of that class; string -> DOM element" is kind of irregular and can't be expressed in the type system itself.

Hmm, actually, now that I've reasoned through it more fully like that, I see that if my understanding of what's going on here is right, then React$ElementRef<typeof Foo> -- for any Foo we'll ever write -- is actually just a long way of spelling Foo.

So maybe the actual answer is that we should just write it that way. Where possible, that is -- which in a case like WebView means saying declare export class instead of declare export var.

In the case discussed at #4278 (comment) , I think the answer we got was right, with React$ElementRef and no typeof. But that's because the thing we pass to ElementRef is already abstract:

  declare export type NavigationContainerType = React$AbstractComponent<
    /* … */,
    BaseNavigationContainerInterface,
  >;

In particular, it's equivalent to typeof NavigationContainer.

[gnprice]

A kind of important thing we drop by saying the WebView value has the type React$ComponentType<WebViewProps> is the postMessage method.

I think this is expressed by saying React$AbstractComponent<WebViewProps, { postMessage: /* … */, ... }>.

Here's the relationship between ComponentType and AbstractComponent:

declare type React$ComponentType<-Config> = React$AbstractComponent<Config, mixed>;

So ComponentType is just a simplification of AbstractComponent, with its second type parameter dropped.

And that second type parameter appears to be a type bound for the resulting instances, based on how it's used elsewhere in react.js and also how it seems to be successfully used for NavigationContainerType (in the bit I quoted above).

[chrisbobbe]

I think this is expressed by saying React$AbstractComponent<WebViewProps, { postMessage: /* … */, ... }>.

Very interesting! Shall I give this a try? I wasn't looking forward to doing a declare export class WebView extends React.Component because of the importing-React thing, but this looks like it might work well instead?

[gnprice]

Hmm, I guess most of the existing examples are all TextInput; and that also ends up needing the ElementRef for similar reasons. (And then doesn't actually work -- the .current ends up being any when used -- for reasons I don't understand.)

The one thing I see that we can update is the comment on mentionWarnings in ComposeBox.js. I think we want

   // TODO: Type-check this, once we've adjusted our `react-redux`
   // wrapper to do the right thing. It should be
   //
-  //   mentionWarnings = React.createRef<React$ElementRef<MentionWarnings>>()
+  //   mentionWarnings = React.createRef<MentionWarnings>()
   //
   // but we need our `react-redux` wrapper to be aware of
   // `{ forwardRef: true }`, since we use that.

[gnprice]

Great! Yeah, agreed.