ak1394 / react-native-tts

React Native Text-To-Speech library for Android and iOS
626 stars 159 forks source link

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

Open mandrewsan opened 1 year ago

mandrewsan commented 1 year ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch react-native-tts@4.1.0 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 commented 1 year ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch react-native-tts@4.1.0 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 commented 1 year ago

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);
+    }
   }
 }