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

Add function EventEmitter.removeListener, compatible react-native EventEmitter path #14

Merged
merged 2 commits into from
Dec 19, 2017

Conversation

cpunion
Copy link

@cpunion cpunion commented Aug 12, 2017

Compatible react-native-root-siblings (maybe other libraries).

At https://github.com/magicismight/react-native-root-siblings/blob/master/lib/AppRegistryInjection.js#L4, it depends path 'react-native/Libraries/EventEmitter/EventEmitter'.

At https://github.com/magicismight/react-native-root-siblings/blob/master/lib/AppRegistryInjection.js#L36, it depends EventEmitter.removeListener.

* @param {Object} listener - Created by addListener
*/
removeListener(listener: Object) {
this._subscriber.removeSubscription(listener)
Copy link
Owner

Choose a reason for hiding this comment

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

我看到 https://github.com/magicismight/react-native-root-siblings/blob/9cd2e112e5668173c9d98dc6dace6fc3b029274b/lib/AppRegistryInjection.js#L36 调用的是 emitter.removeListener('siblings.update', this._update); ,是两个参数的,而你此处是接收一个参数的。看起来 https://github.com/facebook/react-native/blob/405c573e0d41cdcb9f05b5a1b9a81a4ad9f97369/Libraries/EventEmitter/EventEmitter.js#L204 这个 removeListener(eventType: String, listener) 函数更合适参考。

@@ -0,0 +1,2 @@
var EventEmitter = require('../vendor/EventEmitter');
Copy link
Owner

Choose a reason for hiding this comment

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

然后要不参考一下 facebook/react-native@516bf7b ,将 vendor/emitter/ 变为 Libraries/EventEmitter/ ?当然目前这样 var EventEmitter = require('../vendor/EventEmitter'); 也不失为一种简洁的处理方式 😄

@cpunion
Copy link
Author

cpunion commented Aug 25, 2017

确实。之前的场景只是让它能基本工作,removeListener应该是没走到。

不过我这几天没有工作在这个环境,要晚些再看了。

@flyskywhy flyskywhy merged commit 47f722c into flyskywhy:master Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants