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

Remove event listener not working; NativeEventEmitter.removeListener deprecated #235

Open
mandrewsan opened this issue Dec 21, 2022 · 3 comments

Comments

@mandrewsan
Copy link

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

Remove event listener was breaking, apparently due to deprecation of NativeEventEmitter.removeListener.

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-tts/index.js b/node_modules/react-native-tts/index.js
index 6da7573..684fb7a 100644
--- a/node_modules/react-native-tts/index.js
+++ b/node_modules/react-native-tts/index.js
@@ -116,11 +116,12 @@ class Tts extends NativeEventEmitter {
   }
 
   addEventListener(type, handler) {
-    return this.addListener(type, handler);
+    this.eventSubscription = this.addListener(type, handler)
+    return this.eventSubscription;
   }
 
-  removeEventListener(type, handler) {
-    this.removeListener(type, handler);
+  removeEventListener = (type, handler) => {
+    this.eventSubscription.remove();
   }
 }
 

This issue body was partially generated by patch-package.

@praptoherlambang
Copy link

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

Remove event listener was breaking, apparently due to deprecation of NativeEventEmitter.removeListener.

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-tts/index.js b/node_modules/react-native-tts/index.js
index 6da7573..684fb7a 100644
--- a/node_modules/react-native-tts/index.js
+++ b/node_modules/react-native-tts/index.js
@@ -116,11 +116,12 @@ class Tts extends NativeEventEmitter {
   }
 
   addEventListener(type, handler) {
-    return this.addListener(type, handler);
+    this.eventSubscription = this.addListener(type, handler)
+    return this.eventSubscription;
   }
 
-  removeEventListener(type, handler) {
-    this.removeListener(type, handler);
+  removeEventListener = (type, handler) => {
+    this.eventSubscription.remove();
   }
 }
 

This issue body was partially generated by patch-package.

does not work, can you explain in more detail

@VVVi
Copy link

VVVi commented Jul 19, 2023

Here is the improved working version:

diff --git a/node_modules/react-native-tts/index.js b/node_modules/react-native-tts/index.js
index 6da7573..b6557fd 100644
--- a/node_modules/react-native-tts/index.js
+++ b/node_modules/react-native-tts/index.js
@@ -7,6 +7,8 @@ class Tts extends NativeEventEmitter {
     super(TextToSpeech);
   }
 
+  eventSubscription = new Map(); 
+
   getInitStatus() {
     if (Platform.OS === 'ios' || Platform.OS === 'windows') {
       return Promise.resolve(true);
@@ -116,11 +118,19 @@ class Tts extends NativeEventEmitter {
   }
 
   addEventListener(type, handler) {
-    return this.addListener(type, handler);
-  }
-
-  removeEventListener(type, handler) {
-    this.removeListener(type, handler);
+    const uniqueKey = [type, handler]; 
+    const listener = this.addListener(type, handler); 
+    this.eventSubscription.set(uniqueKey, listener); 
+    return listener; 
+  }
+
+  removeEventListener = (type, handler) => {
+    const uniqueKey = [type, handler];
+    const listener = this.eventSubscription.get(uniqueKey);
+    if (listener) {
+      listener.remove();
+      this.eventSubscription.delete(uniqueKey);
+    }
   }
 }

@cashtotalszero
Copy link

Just bumping this one as it appears to still be outstanding.

The removeEventListener calls are still throwing errors with the following setup:

react: 18.2.0
react-native: 0.74.5
react-native-tts: 4.1.1
xcode: 15.4

Are there plans to merge in the patch by @VVVi?

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

No branches or pull requests

4 participants