Skip to content

[v2] Fixes issues#1397#1816

Merged
guyca merged 2 commits into
wix:v2from
joonhocho:patch-1
Jan 10, 2018
Merged

[v2] Fixes issues#1397#1816
guyca merged 2 commits into
wix:v2from
joonhocho:patch-1

Conversation

@joonhocho
Copy link
Copy Markdown

#1397

Tested it and it works well with or without remote debugging enabled.

@joonhocho
Copy link
Copy Markdown
Author

joonhocho commented Sep 9, 2017

Note that it does not fix appLoaded event being raised multiple times due to many js bundles. It fixes app hanging due to not receiving the event.

@bogobogo bogobogo added the v2 label Sep 10, 2017
@DanielZlotin
Copy link
Copy Markdown
Contributor

Thanks for the PR.
I want to reproduce it somehow with tests before merging this,
I think this reproduces on latest RN, I'll upgrade soon and check

@joonhocho
Copy link
Copy Markdown
Author

@DanielZlotin FYI I am using RN 0.47.2.

@joonhocho joonhocho changed the title Fixes issues#1397 [v2] Fixes issues#1397 Sep 11, 2017
@AdamLiechty
Copy link
Copy Markdown

This reproduces on RN 0.50.3 as well.

@shuse2
Copy link
Copy Markdown

shuse2 commented Nov 15, 2017

For RN 0.50.3, I checked both with and without debugger, and this fix works!

@steffenmllr
Copy link
Copy Markdown

Fix works for:

    "react-native": "0.50.3",
    "react-native-navigation": "^2.0.2031",

Copy link
Copy Markdown

@brunolemos brunolemos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, this is what I see:

Fixes

  • It's now possible to run the app with JS Debugging enabled (#491, #862, #963)

Doesn't fix

  • Warning still appears: Sending RNN.appLaunched with no listeners registered. (#1397)
  • App still crashes after reloading (#2030, #1514) (this fixes it: #1836)

"react-native": "^0.51.0",
"react-native-gesture-handler": "^1.0.0-alpha.35",

@anonrig
Copy link
Copy Markdown

anonrig commented Dec 21, 2017

Such a major bug in v2, didn't get fixed. Any body working on this atm?

@brunolemos
Copy link
Copy Markdown

brunolemos commented Dec 23, 2017

@anonrig while this is not merged, you can apply the fix using patch-package.

Follow the instructions in the patch-package README. After the step npx patch-package react-native-navigation replace the created patches/react-native-navigation+2.0.2042.patch file with this content:

patch-package
--- a/node_modules/react-native-navigation/lib/ios/RNNEventEmitter.m
+++ b/node_modules/react-native-navigation/lib/ios/RNNEventEmitter.m
@@ -1,6 +1,9 @@
 #import "RNNEventEmitter.h"
 
-@implementation RNNEventEmitter
+@implementation RNNEventEmitter {
+  NSInteger _appLaunchedListenerCount;
+  BOOL _appLaunchedEventDeferred;
+}
 
 RCT_EXPORT_MODULE();
 
@@ -15,9 +18,14 @@ static NSString* const onNavigationButtonPressed	= @"RNN.navigationButtonPressed
 
 # pragma mark public
 
--(void)sendOnAppLaunched {
+-(void)sendOnAppLaunched {
 	[self send:onAppLaunched body:nil];
-}
+	if (_appLaunchedListenerCount > 0) {
+		[self send:onAppLaunched body:nil];
+	} else {
+		_appLaunchedEventDeferred = TRUE;
+	}
+ }
 
 -(void)sendContainerDidAppear:(NSString *)containerId {
 	[self send:containerDidAppear body:containerId];
@@ -31,10 +39,24 @@ static NSString* const onNavigationButtonPressed	= @"RNN.navigationButtonPressed
 	[self send:onNavigationButtonPressed body:@{@"containerId":containerId , @"buttonId": buttonId }];
 }
 
+- (void)addListener:(NSString *)eventName {
+	[super addListener:eventName];
+	if ([eventName isEqualToString:onAppLaunched]) {
+		_appLaunchedListenerCount++;
+		if (_appLaunchedEventDeferred) {
+			_appLaunchedEventDeferred = FALSE;
+			[self sendOnAppLaunched];
+		}
+	}
+}
+
 # pragma mark private
 
 -(void)send:(NSString *)eventName body:(id)body {
-	[self sendEventWithName:eventName body:body];
+	// TODO This is a quick and dirty fix to issues/#1514.
+	if ([self bridge] != nil) {
+		[self sendEventWithName:eventName body:body];
+	}
 }
 
 @end

After every npm install or yarn it will apply this fix to the node_modules/react-native-navigation project.

This patch includes this pull request and #1836 as well.

@pentarex
Copy link
Copy Markdown

this patch does not work for me.
cannot debug and also receiving

Sending `RNN.appLaunched` with no listeners registered. 
react-native-navigation: 2.0.2070
react-native: 0.51.0
node: 9.3.0
npm: 5.6.0

@guyca guyca requested a review from yogevbd January 10, 2018 08:45
@guyca guyca merged commit 1586de0 into wix:v2 Jan 10, 2018
@yogevbd yogevbd mentioned this pull request Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.