Why was onPublish removed? about cordova-plugin-mqtt

[arcoirislabs]

@RangerMauve : Feel free to fork the repository and if you feel that something can be contributed to this root repo, do share it with us, we will add a reference to you in our README as a contributor:-)
Also one of the main motives of the Cordova framework is to do the heavy lifting on the native side by using the native capabilities of the operating system, which obviously perform better, rather going for pure JS implementation, which can create an overhead on the single Javascript thread of the WebView. Which is exactly what we are doing. We don't want the developer to add extra burden over the Javascript thread whereas we already have the liberty to add that burden on native threads
If you want any assistance on integrating this plugin with your existing codebase, do ping me at [email protected]. I will be happy to help you.

Amey K,
Maintainer

[arcoirislabs]

We removed the onPublish method because we wanted the developer to be able to run their own payload receiving logic anywhere in their own code just by putting an Event listener for the topic.
If there is a DOM event similar to topic name, then yes it is going to interfere with the code logic. But that is developer's responsibility not to program such events.
If you are worried about ghost listeners that remain after the unsubscribe method, then we are actually removing the event listener of topic you will unsubscribe.
Also can you elaborate the memory leak issue a little bit?? Because if it is major we will look into it. But so far, we have already written the code that nullifies the connection object on the disconnection, on the native side.

[RangerMauve]

Well, whenever you use addEventListener on the document, you need to make sure to save a reference to the event handler in order to removeEventListener it later when it's no longer useful.

If it was registered as, say, a single event handler, then it'd line up more with other MQTT clients in JS and mean that the connection can be disposed of along with all of its handlers, just by getting rid of any references to the connection.

Whereas keeping listeners in the DOM means that they need to be tracked through some other mechanism and explicitly removed, thus adding a bunch more code that needs to wrap over top of this library.

On the topic of expecting developers to make sure that their events don't conflict with the DOM, it's useful for people making a brand new application that's making use of this library, but if somebody has an existing codebase, they might not have that luxury.

Other than that, using the DOM makes it harder to do routing in a clean and easy to unit test way. In other JS clients, since all messages are sent to the same place, people can route messages around through whatever mechanism they want, either with an EventEmitter implementation, or with something fancier. Doing something like that becomes more tedious because one now needs to add separate event handlers to the DOM and increase the footprint in order to route with something other than DOM events. Memory leaks on DOM events are pretty common and are easily avoided by not using DOM events for non DOM-specific purposes.

As well, this makes handling wildcards more difficult since you can't know in advance what types of events will match against a given pattern. For instance if you subscribe to foo/#, you might get foo/bar or foo/baz/fizz, but you can't always know to subscribe to those events in advance.

[arcoirislabs]

I am dividing the responses to your concern in bits. So here it goes

"Well, whenever you use addEventListener on the document, you need to make sure to save a reference to the event handler in order to removeEventListener it later when it's no longer useful."

"Whereas keeping listeners in the DOM means that they need to be tracked through some other mechanism and explicitly removed, thus adding a bunch more code that needs to wrap over top of this library."

• That is correct and we would like to apologize that we haven't added out latest commit to the repo that actually contains the event listener removal method on unsubscribe, it will rectified ASAP.

"If it was registered as, say, a single event handler, then it'd line up more with other MQTT clients in JS and mean that the connection can be disposed of along with all of its handlers, just by getting rid of any references to the connection."

• That is possible only if the connection object is created in the webpage & being handled inside the webpage by using any external libraries, that actually handle connections via underlaying Websocket feature. But since this is the native implementation, everything works under the native domain (Java in case of Android). The JS functions are merely an interface created using Cordova APIs. And we strongly discourage the JS implementations along with the Cordova plugin as it is being redundant and beats the point of Cordova's necessity of a plugin.

"On the topic of expecting developers to make sure that their events don't conflict with the DOM, it's useful for people making a brand new application that's making use of this library, but if somebody has an existing codebase, they might not have that luxury."

• Your point is correct but when it comes to topic names as per the MQTT standard, there is very small possibility that the clash of events might even happen.

"Other than that, using the DOM makes it harder to do routing in a clean and easy to unit test way. In other JS clients, since all messages are sent to the same place, people can route messages around through whatever mechanism they want, either with an EventEmitter implementation, or with something fancier."

• Well we know one thing from our 4+ years of Cordova development, that excessive libraries create excessive overhead on the application, so from our perspective we want that the developer can have the most generic way of getting payload without constricting the developer to one namespace of onPublish. The issue is maintaining the code inside the onPublish method for processing the received payload, which in the course of time expands and becomes difficult to maintain the code.
  Also your implementation of event-emitter is based on arrays, which can effect into memory leaks, depending on the user's implementation, we did thought of array based callbacks just like yours, but the again a bloated array can affect the app's memory performance.

"Doing something like that becomes more tedious because one now needs to add separate event handlers to the DOM and increase the footprint in order to route with something other than DOM events. Memory leaks on DOM events are pretty common and are easily avoided by not using DOM events for non DOM-specific purposes."

• Cordova framework utilises the DOM events for non DOM purposes for various purposes. So in Cordova development, using event based callbacks are perfectly fine. It is all about removing the event listeners after a successful unsubscribe, that can avoid the memory leaks.

"As well, this makes handling wildcards more difficult since you can't know in advance what types of events will match against a given pattern. For instance if you subscribe to foo/#, you might get foo/bar or foo/baz/fizz, but you can't always know to subscribe to those events in advance."

• We haven't added a complete wild card support yet as we are still working on it. So no comments as such.

[RangerMauve]

I guess it might be good to also clarify that in my specific use case, we've got an application that uses the same codebase to target a bunch of different environments seamlessly, that's one of the main reasons I would have liked it to align more with the things that exit out there already.

I think that this seems to be more focused on building Cordova apps that are meant to just run within Cordova, so it may just be that I'm trying to make it do something it's not made for.

I'll see if I can work it into the existing workflow, else it might make sense to fork or find an alternative.

Thank you very much for the quick responses, and I can't wait to see the wildcard support when it comes out!:D

[RangerMauve]

Yeah, I just implemented an alternative connect method for the plugin by calling cordova.exec manually. Pretty simple change on my end and it lets others use what's easier for them.

Seems to work perfectly. Thanks!