Skip to content

Conversation

@brentvatne
Copy link
Collaborator

As mentioned in #906, in the docs it mentions sending native app events eg: calendar event reminder received, through the RCTNativeAppEventEmitter, but the JS module for that is missing. This adds it - it's nothing more than an instance of EventEmitter, just like RCTDeviceEventEmitter.

- As referenced in RCTEventDispatcher#sendAppEventWithName
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 19, 2015
@brentvatne
Copy link
Collaborator Author

e2e test failed but that seems to be an existing issue unrelated to this branch

@brentvatne
Copy link
Collaborator Author

Any reason not to merge this one?

@henter
Copy link
Contributor

henter commented May 8, 2015

@brentvatne why not just use DeviceEventEmitter ?

@brentvatne
Copy link
Collaborator Author

@henter - good question, this is expected to be there in RCTEventDispatcher already, see here - but the actual module in JS is not there, so this just adds that module.

@henter
Copy link
Contributor

henter commented May 8, 2015

get it. thanks :-)

Henter
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Friday, May 8, 2015 at 11:26, Brent Vatne wrote:

@henter (https://github.com/henter) - good question, this is expected to be there in RCTEventDispatcher already, see here (https://github.com/brentvatne/react-native/blob/master/React/Base/RCTEventDispatcher.m#L32-L36) - but the actual module in JS is not there, so this just adds that module.


Reply to this email directly or view it on GitHub (#918 (comment)).

@bparadie
Copy link

@brentvatne I think, @henter kind of has a point...

Your RCTNativeAppEventEmitter.js reads:

var RCTNativeAppEventEmitter = new EventEmitter();

But the documentation suggests registering at DeviceEventEmitter:

var subscription = DeviceEventEmitter.addListener(...)

Now, I see that there is a separate RCTDeviceEventEmitter.js with this code:

var RCTDeviceEventEmitter = new EventEmitter();

I wonder whether those two pieces add up. Is it possible that the documentation suggests registering at RCTDeviceEventEmitter while your fix applies to RCTNativeAppEventEmitter, which is a different emitter? At least that is my theory, because the example is still not working for me.

Intuitively I was expecting a fix along the lines of:

var RCTNativeAppEventEmitter = RCTDeviceEventEmitter;

Am I completely wrong?

@bparadie
Copy link

@brentvatne What really fixed my problem was #1275, which simply suggests using sendDeviceEventWithName instead of sendAppEventWithName. Now I am confused what this PR tries to fix. Is the purpose of RCTNativeAppEventEmitter providing support sendAppEventWithName? But why would I use sendAppEventWithName if I have sendDeviceEventWithName?

@yelled3
Copy link
Contributor

yelled3 commented May 15, 2015

@brentvatne the CI seems to fail for some reason

The operation couldn’t be completed. (NSURLErrorDomain error -1004.)" - RedBox error: Could not connect to development server. Ensure node server is running - run 'npm start' from ReactKit root
The operation couldn’t be completed. (NSURLErrorDomain error -1004.):
60     }];
61   }
62 
63   XCTAssertNil(redboxError, @"RedBox error: %@", redboxError);
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
64   XCTAssertTrue(foundElement, @"Cound't find element with text '%@' in %d seconds", TEXT_TO_LOOK_FOR, TIMEOUT_SECONDS);
65 }
━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    ✗ -[EndToEndTestTests testRendersWelcomeScreen] (383 ms) (0)
    0 passed, 1 failed, 0 errored, 1 total (387 ms)
Failures:
  0) -[EndToEndTestTests testRendersWelcomeScreen] (EndToEndTestTests.xctest)

any idea why?

@sahrens
Copy link
Contributor

sahrens commented May 15, 2015

@yelled3 unfortunately we have some instability in our tests, so that is probably an unrelated failure :(

I believe (but wasn't involved so not sure) the purpose of having both emitters is to provide namespacing and clarity. Device events should only be emitted by the system, whereas app events should be used for whatever custom events you have in your app. I'll merge this in soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants