Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complete switch to RCTEventEmitter. #35

Closed
wants to merge 1 commit into from

Conversation

athibaud
Copy link
Contributor

this module's event emitting is currently broken on latest versions of react (only tested with 0.30+, can't say for previous versions)

unfortunately the documentation isn't up to date but my understanding is that events should no longer be emitted on the global bus and that native modules should now use a subclassed/module-scopped NativeEventEmitter as can be seen here and there

the recent switch to RCTEventEmitter only affected the native code. so the current js code was listening on the global bus while the native code was sending events on the RNBranch-scopped bus... this pull request updates the js code to listen on that scopped bus.

cc @nicklockwood (sorry for blindly cc'ing you in here, but i would appreciate if you could confirm this is indeed the new/correct way of communicating via events.)

@rt2zz
Copy link
Contributor

rt2zz commented Jul 26, 2016

@athibaud my mistake I published and emergency v0.4.1 with the fix, but did not merge to master branch. The one thing missing from your PR is android support which still uses the old style event emitter. I pushed the fix to master now.

Thanks!!

@rt2zz rt2zz closed this Jul 26, 2016
@athibaud
Copy link
Contributor Author

@rt2zz perfect! oops, my bad for android :) thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants