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

bug: notifyListeners() only works for the first call #2404

Closed
1 of 4 tasks
kofoeddk opened this issue Feb 4, 2020 · 3 comments · Fixed by #2408
Closed
1 of 4 tasks

bug: notifyListeners() only works for the first call #2404

kofoeddk opened this issue Feb 4, 2020 · 3 comments · Fixed by #2408

Comments

@kofoeddk
Copy link
Contributor

kofoeddk commented Feb 4, 2020

Bug Report

notifyListeners() in Plugin.java only works the first time for retained event arguments due to insufficient empty state check.

Capacitor Version

npx cap doctor output:
💊 Capacitor Doctor 💊

Latest Dependencies:

@capacitor/cli: 1.5.0

@capacitor/core: 1.5.0

@capacitor/android: 1.5.0

@capacitor/ios: 1.5.0

Installed Dependencies:

@capacitor/cli 1.4.0

@capacitor/core 1.4.0

@capacitor/android 1.4.0

@capacitor/ios 1.4.0

Affected Platform(s)

  • Android
  • iOS
  • Electron
  • Web

Current Behavior

From a plugin, notify listener:
notifyListeners("myListener", jsObjInstance, true)

.. and from TypeScript component:

listener = Plugin.addListener('myListener'), (response) => {
    listener.remove();
    // do something with response
}

This works perfectly for the first call.

Expected Behavior

I would expect it to work for subsequent calls as well.

Sample Code or Sample Application Repo

Reproduction Steps

See Current Behavior

Resolutiuon

Add isEmpty() checks for lists instead of only doing null-checks.

git diff output:

diff --git a/android/capacitor/src/main/java/com/getcapacitor/Plugin.java b/android/capacitor/src/main/java/com/getcapacitor/Plugin.java
index cedd7c7..3caeaed 100644
--- a/android/capacitor/src/main/java/com/getcapacitor/Plugin.java
+++ b/android/capacitor/src/main/java/com/getcapacitor/Plugin.java
@@ -300,7 +300,7 @@ public class Plugin {
    */
   private void addEventListener(String eventName, PluginCall call) {
     List<PluginCall> listeners = eventListeners.get(eventName);
-    if (listeners == null) {
+    if (listeners == null || listeners.isEmpty()) {
       listeners = new ArrayList<PluginCall>();
       eventListeners.put(eventName, listeners);
 
@@ -335,7 +335,7 @@ public class Plugin {
   protected void notifyListeners(String eventName, JSObject data, boolean retainUntilConsumed) {
     Log.v(getLogTag(), "Notifying listeners for event " + eventName);
     List<PluginCall> listeners = eventListeners.get(eventName);
-    if (listeners == null) {
+    if (listeners == null || listeners.isEmpty()) {
       Log.d(getLogTag(), "No listeners found for event " + eventName);
       if (retainUntilConsumed) {
         retainedEventArguments.put(eventName, data);
@kofoeddk
Copy link
Contributor Author

kofoeddk commented Feb 5, 2020

Actually, this issue has an iOS counterpart that was fixed almost the same way:
#2094

@kofoeddk kofoeddk changed the title bug: bug: notifyListeners() only works for the first call Feb 5, 2020
@kofoeddk
Copy link
Contributor Author

kofoeddk commented Feb 5, 2020

Added PR: #2408

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 12, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant