[android] add messaging passing without a react tag #12

nealmanaktola · 2022-09-01T20:03:04Z

This PR adds messaging passing. This was much simpler to do than the iOS counter part. This is the analogue to the following PR: #6. I've followed the same test plan. The sample app was really nice to use when testing.

In the previous PR #11, I broke all native->react-native communication since events were being passed by the InternalWebView viewId not the RNCWebView viewId. This is now fixed.

Here's a demo:

message-passing.mov

nealmanaktola · 2022-09-02T16:18:38Z

example/examples/Portals.tsx

+const scriptMessageEmitter = new NativeEventEmitter(
+  Platform.select({
+    ios: NativeModules.ScriptMessageEventEmitter,
+    android: null


android doesn't really need an associated module to emit an event

nealmanaktola · 2022-09-02T16:19:20Z

src/WebViewTypes.ts

@@ -1280,4 +1280,32 @@ export interface WebViewSharedProps extends ViewProps {
   * An object that specifies the credentials of a user to be used for basic authentication.
   */
  basicAuthCredential?: BasicAuthCredential;
+
+  /**
+   * By default, if this is undefined or false, the native WebView will get released when


I've copied these props to Android

It looks like you've added these to export interface WebViewSharedProps extends ViewProps. They were previously defined on export interface IOSWebViewProps extends WebViewSharedProps. Instead of copying this to WebViewSharedProps, should we copy this to export interface AndroidWebViewProps extends WebViewSharedProps?

oops - good catch 👍

nealmanaktola · 2022-09-02T16:28:24Z

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java

@@ -1835,6 +1849,11 @@ protected void onScrollChanged(int x, int y, int oldX, int oldY) {
    }

    protected void dispatchEvent(WebView webView, Event event) {
+      if (event.getViewTag() == RNCWebView.INVALID_VIEW_ID) {
+        FLog.w(TAG, "Unable to dispatch event: ", event.getEventName() + "due to InternalWebView not being attached.");


I think a warning is enough. This will likely happen for any callbacks such as onLoadingStarted etc when the webview is unmounted. I am assuming it's likely we'll have the same issue on iOS. It might be nicer in the future to dispatch events for all of these callbacks so they can be listened to without a react tag.

nealmanaktola · 2022-09-02T16:29:41Z

android/src/main/java/com/reactnativecommunity/webview/RNCWebView.java

@@ -9,6 +10,7 @@

 public class RNCWebView extends FrameLayout {
  private static final String TAG = "RNCWebView";
+  public static final int INVALID_VIEW_ID = -1;


https://developer.android.com/reference/android/view/View#findViewById(int)

if the ID is invalid (< 0) or there is no matching view in the hierarchy.

I used -1 as it's a recognized invalid view id

nealmanaktola · 2022-09-02T16:33:57Z

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java

+          WritableMap data = mRNCWebViewClient.createWebViewEvent(webView, webView.getUrl());
+          data.putString("webViewKey", webViewKey);
+          data.putString("data", message);
+          reactContext.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class).emit("onMessage", data);


So I learned that the event names need to be unique for your entire app, I think we should rename this to ReactNativeWebViewOnMessage or something of that sort.

https://github.com/facebook/react-native/blob/main/Libraries/EventEmitter/NativeEventEmitter.js#L35

I'll rename both in a follow up PR for both iOS and Android

https://app.asana.com/0/1201254368985004/1202916067056138/f

donaldchen

Looks good! Nice work 🙂 . My comments are all non-blocking

donaldchen · 2022-09-02T22:02:26Z

android/src/main/java/com/reactnativecommunity/webview/RNCWebView.java

+  /**
+   * Provides the associated parent RNCWebView viewId for the provided
+   * webView view id.
+   * @param webView


nit: On all these javadoc comments, we probably don't need the @param parameter name lines unless there's a comment that specifically describes what that parameter is. Otherwise, that line from the javadoc will be redundant with the method signature

donaldchen · 2022-09-02T22:21:24Z

src/WebViewTypes.ts

@@ -1280,4 +1280,32 @@ export interface WebViewSharedProps extends ViewProps {
   * An object that specifies the credentials of a user to be used for basic authentication.
   */
  basicAuthCredential?: BasicAuthCredential;
+
+  /**
+   * By default, if this is undefined or false, the native WebView will get released when


It looks like you've added these to export interface WebViewSharedProps extends ViewProps. They were previously defined on export interface IOSWebViewProps extends WebViewSharedProps. Instead of copying this to WebViewSharedProps, should we copy this to export interface AndroidWebViewProps extends WebViewSharedProps?

android/src/main/java/com/reactnativecommunity/webview/RNCWebView.java

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java

donaldchen · 2022-09-02T23:32:29Z

lib/injectJavaScriptWithWebViewKey.android.js

@@ -1,4 +1,4 @@
 import { NativeModules } from 'react-native';
 export default function injectJavaScriptWithWebViewKey(webViewKey, script) {
-    NativeModules.RNCWebView.injectJavaScript(webViewKey, script);
+    NativeModules.RNCWebView.injectJavaScriptWithWebViewKey(webViewKey, script);


Is this change generated from yarn build? In the past, sometimes when I rebuilt the lib folder, it resulted in some filename.d.ts changes. I'm wondering if we should expect to see similar changes given the WebViewTypes.ts changes in this PR

This was generated via yarn build. I don't think we should see any d.ts changes since no types were changed.

…bView in view hierarchy * add support to reuse instance with webViewKey * change up * address comments related to layout params * add nullability checks * fix * get full stack trace * add secondary map * rename to ifHasInternalWebView * additional fixes * fix * fix boolean [android] add messaging passing without a react tag (#12) This PR adds messaging passing. This was much simpler to do than the iOS counter part. This is the analogue to the following PR: #6. I've followed the same test plan. The sample app was really nice to use when testing. In the previous PR #11, I broke all native->react-native communication since events were being passed by the `InternalWebView` viewId not the `RNCWebView` viewId. This is now fixed. [android] check source prop if changed (#14) * [android] check source prop if changed * detect if new source is null too

[android] add messaging passing

7f1a527

nealmanaktola changed the title ~~[android] add messaging passing~~ [android] add messaging passing (not-ready-for-review) Sep 1, 2022

nealmanaktola added 7 commits September 1, 2022 17:14

lib fixes

b123589

fixes

34da532

handle some edge cases

1c595af

some helpful comments

7fecd93

more comments

355cc3c

revert

76a13e6

remove this

8da3691

nealmanaktola commented Sep 2, 2022

View reviewed changes

more cleanup

daf78f4

nealmanaktola commented Sep 2, 2022

View reviewed changes

nealmanaktola changed the title ~~[android] add messaging passing (not-ready-for-review)~~ [android] add messaging passing without a react tag Sep 2, 2022

nealmanaktola requested a review from donaldchen September 2, 2022 16:36

donaldchen approved these changes Sep 2, 2022

View reviewed changes

nealmanaktola added 3 commits September 6, 2022 09:55

rename variable

f1e59b4

move props over

fd1188e

update types

c8fc5fa

nealmanaktola merged commit cc8a18a into 11.18.1-discord-1 Sep 6, 2022

donaldchen mentioned this pull request Sep 6, 2022

support message passing and keeping native WebView in memory when react component is unmounted react-native-webview/react-native-webview#2469

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[android] add messaging passing without a react tag #12

[android] add messaging passing without a react tag #12

nealmanaktola commented Sep 1, 2022 •

edited

nealmanaktola Sep 2, 2022

nealmanaktola Sep 2, 2022

donaldchen Sep 2, 2022 •

edited

nealmanaktola Sep 6, 2022

nealmanaktola Sep 2, 2022

nealmanaktola Sep 2, 2022 •

edited

nealmanaktola Sep 2, 2022

nealmanaktola Sep 2, 2022

donaldchen left a comment

donaldchen Sep 2, 2022

donaldchen Sep 2, 2022 •

edited

donaldchen Sep 2, 2022 •

edited

nealmanaktola Sep 6, 2022

[android] add messaging passing without a react tag #12

[android] add messaging passing without a react tag #12

Conversation

nealmanaktola commented Sep 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donaldchen Sep 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nealmanaktola Sep 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donaldchen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donaldchen Sep 2, 2022 • edited

Choose a reason for hiding this comment

donaldchen Sep 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nealmanaktola commented Sep 1, 2022 •

edited

donaldchen Sep 2, 2022 •

edited

nealmanaktola Sep 2, 2022 •

edited

donaldchen Sep 2, 2022 •

edited

donaldchen Sep 2, 2022 •

edited