Unsupported top level event type "topLoadingStart" dispatched in Custom WebView

[juangl]

Is this a bug report?

I believe so.

Have you read the Contributing Guidelines?

Yes.

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 8.7.0
Yarn: 1.2.1
npm: 5.4.2
Watchman: 4.7.0
Xcode: Xcode 9.1 Build version 9B55
Android Studio: 2.3 AI-162.4069837

Packages: (wanted => installed)
react: 16.0.0 => 16.0.0
react-native: 0.50.3 => 0.50.3

Target Platform: iOS (11.1)

Steps to Reproduce

We implemented a Custom WebView. It used to work in RN 0.49 (we were using the components/WebView.ios.js from master). I'm not an Objective-C developer and probably I'm doing something wrong. Our implementation looks like this:

// RCTCustomWebViewManager.h
#import <React/RCTWebViewManager.h>
#import <React/RCTWebView.h>

@interface RCTCustomWebViewManager : RCTWebViewManager

@end


// RCTCustomWebViewManager.m
#import "RCTCustomWebViewManager.h"

@interface RCTCustomWebViewManager () <RCTWebViewDelegate>

@end

@implementation RCTCustomWebViewManager { }
RCT_EXPORT_MODULE()
RCT_REMAP_VIEW_PROPERTY(keyboardDisplayRequiresUserAction, _webView.keyboardDisplayRequiresUserAction, BOOL)

- (UIView *)view
{
  RCTWebView *webView = [RCTWebView new];
  webView.delegate = self;
  return webView;
}

@end

Expected Behavior

I expect it works.

Actual Behavior

Reproducible Demo

https://github.com/juangl/with-custom-webview

[rhysforyou]

Seeing the same issue here.

[evilDave]

Same here. Working on a new customised WebView, works on Android, has this problem on iOS.

[evilDave]

#16522 Kind of looks similar... On Android we need to do getExportedCustomDirectEventTypeConstants to register custom top level events (and add your own). Is there something like that that is required for iOS too? Incidentally, the event that I have added works fine, it's just the default ones (topLoadStart, topLoadFinish...) that don't.

Edit: on that note, I decided to see what would happen if I added RCT_EXPORT_VIEW_PROPERTY(onLoadingStart, RCTDirectEventBlock) to my custom WebViewManager - it seems to mask the error (I don't get the event, but it does not complain) - it does seem like there is something wrong with the viewConfig code (e.g. something calls RCTComponentData.m) but I don't understand everything that is going on there.

[evilDave]

@cbrevik @jacobp100 @spicyj

(Apologies if this is not the right way to get someone to look at an issue)

#10946 and #15016 introduce a really nice way to handle extending native components, but it seems like something has gone awry with the default events.

There is a note in one of those pull requests that "You'll have to duplicate RCTWebViewManager.m, since the exported macro-defined methods/properties (with RCT_REMAP_VIEW_PROPERTY, RCT_EXPORT_METHOD, etc) are only picked up from the subclass, and not from the parent.". However I don't think any such advice made it into the documentation. Not sure what the issue is, if there is a problem to be fixed, or just some extra steps to customise the webview that need to be taken.

Thanks

Edit: I checked with the example repo (https://github.com/cbrevik/webview-native-config-example) in the pull request, and the same problem is seen there.

[cbrevik]

Quickly tested it out, and was able to reproduce the same error.

Regarding:

You'll have to duplicate RCTWebViewManager.m, since the exported macro-defined methods/properties (with RCT_REMAP_VIEW_PROPERTY, RCT_EXPORT_METHOD, etc) are only picked up from the subclass, and not from the parent.

This should not be the case any more. With #14775 inheritance works on iOS as well.

I am not sure where the topLoadingStart event is coming from though. Searching through the code base I only find references in Android-code. Might be something changed with events on iOS from RN 0.49 -> 0.50.

Maybe @javache knows?

[cbrevik]

Seems related to how events are resolved in requireNativeComponent.js

Spent a bit of time investigating this, and was about to submit a PR, but seems like @shergin already fixed this in master: 2afe7d4

So it will probably be working again in a future version of RN 👍

Edit:
The reason why topLoadingStart was not referenced in the iOS code is because there's a name-normalization method for events for iOS:

react-native/React/Base/RCTEventDispatcher.m

Line 20 in d2c5697

NSString *RCTNormalizeInputEventName(NSString *eventName)

Bit hacky, and makes debugging more difficult, but oh well. Yet another gotcha :)

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.