Events in Service Migration

[Abby-Wheelis]

In some of the service migrations -- startPrefs, storeDeviceSettings, startPrefs pushNotify (and other notify services). We are finding an event-driven implementation, both to reduce tight coupling , and to handle notifications within the app. In digging around, I've found a few other people in similar situations, and it seems that the MOST react way to do this would be to pass callbacks as props to children ... but our events are needing to happen outside of the context of components.

Probably the most promising solution that I've seen is to implement some sort of observer pattern ourselves, like the one in this stack overflow thread in the comment starting with "you can write a simple service and then use it". That same thread also mentions that "The React way would be to pass callbacks down to children explicitly via props — . There's no support for custom events w/ bubbling in React." This sort of centralized event handling service seems like it would make sense in our context, at least for the "cloud" type events.

The other thing that has occurred to me is the potential for use of custom hooks here - but we can't really use hooks outside of components. hook documentation

I'll work on diagramming this up to make sure it would work in our context and hopefully explain more

[Abby-Wheelis]

Not the most detailed diagram, but basically, where the "emits" were we can publish the specific event and where the "ons" were, we take that logic and subscribe to the specific event. If we ever need to stop listening for notifications in a specific context we can unsubscribe.

The list of notifications will be organized by event type (this was the structure the answer in stack overflow used, and I think it would work in our context). For example:

eventSubscriptions = {
  "cloud:push:notification" : 
    "pushNotify" : () => {handle event},
    "remoteNotify": () => {different logic}
}

Then when publishNotification is called for an event, we execute the logic for all subscribers to that event.

This is more than likely how $emit and $on worked behind the scenes, but it does not seem that react has built in support for this kind of custom event handling in the same manner, but we could set it up ourselves.

Does this seem like a reasonable solution?

[shankari]

we can, of course, do something like this, but it seems like a huge kludge. event handling is a fairly fundamental part of modern software architectures, and I can't believe that react doesn't already support this. It looks like react does support custom events; I am not sure if the same method works on react native.
https://blog.logrocket.com/using-custom-events-react/

Although this also seems like you have to copy-and-paste the boilerplate to pub-sub to events. @jiji14 @JGreenlee, surely react supports events natively, right?

[JGreenlee]

I haven't read a ton into this topic, but my understanding so far aligns with @Abby-Wheelis's conclusions in the initial post.

It might be possible to implement event-driven logic by adopting an "observer" or "subscription" pattern, but I think it's not the preferred pattern in the React paradigm.

React prefers to just pass callbacks down as props. The heavy lifting happens higher up in the component tree and actions triggered from the lower-level components often just call functions from their (grand)parents

And since passing props can get excessive we use React Context to make it easier to pass certain things to all (grand)children at once – but it's still the same idea of a unidirectional flow from parent->child

[JGreenlee]

I am a bit unclear on why we are wanting an event-driven implementation in the rewrite.

I understand that the old code is event-driven, but what's stopping us from replacing the event handling with explicit function calls?

[JGreenlee]

For the record, I don't think we need to be worried about circular dependencies at the file level.

We aren't using default exports in the Typescript helpers we've been rewriting into. They're just collections of individual functions (ideally pure functions). We export and import each function individually using named exports.

So there's no such thing as importing module A and importing module B. We could do something like import function X from module A and function Y from module B.

So parts of B might depend on A and parts of A might depend on B. But as long as X and Y don't call each other it should be ok

[Abby-Wheelis]

what's stopping us from replacing the event handling with explicit function calls?

I'm not sure that anything is stopping us, but it would be a shift in design. In the push notification receiver in pushNotify, we would have to call a method from pushNotify, one from remoteNotify and possibly others. If we're OK with such tight coupling then we can go ahead with just importing and calling the functions directly. But if something else later needs to happen on this same event, then we will have to add the call to it here, which does feel like iffy design, but it sounds like that or rolling in some boilerplate publish/subscribe code to enable event-driven logic are the two options really available to us.

[Abby-Wheelis]

For the record, I don't think we need to be worried about circular dependencies at the file level.

That makes sense, and I believe we are clear of circular dependency on the function level. Is the tight coupling of files also less of a concern then, because they're just collections of functions? The sense of tight coupling is what pushed me to look so deeply into keeping the events, but maybe this is just part of the paradigm shift from Angular service modules -> typescript functions.

[Abby-Wheelis]

In our meeting today, we loosely decided on the following course of action:

• leave startPrefs PR as is, with the direct function calls rather than use events (for now) to prevent excess churn
• write the pushNotify service using events, implementing our own custom event handling
• then make a sweep to introduce or re-introduce events to other parts of the app (this would include startPrefs

I'll probably re-start my PR for the pushNotify service since I've already gone back and forth a few times with that change. remoteNotify only has 3 functions in it, so I could roll that in with pushNotify, else whoever takes that up I can work closely with to keep the same approach with the events we're adding.

[Abby-Wheelis]

I think this is also going to help with testing, I'm currently trying to test pushNotify, but running up agains the side-effects in other files. Without the direct calls, if the other files aren't initialized by the tests they won't be affected by the events, allowing for easier and more modular testing.

[shankari]

Yeah, this is the poster child for modularity 😄
Let's move to events - I have written pub-sub code before and it is not that complicated.

[shankari]

Hey I found a library: https://www.npmjs.com/package/pubsub-js

[Abby-Wheelis]

I've implemented and tested the boilerplate from the website you linked in my pushNotify PR. It is working well, and I'm almost done with the tests for the pushNotify part of the PR. Should I switch to the library?

[shankari]

nah! If you hadn't already implemented it, using the library may save you some time. But it is very simple code, and the library has not been updated for a couple of years. If we run into issues with the boilerplate, we can switch to the library.

[Abby-Wheelis]

This was merged in 🎟️ 🔔 📱 Custom Events + pushNotify + storeDeviceSettings + remoteNotify rewrites! I'll keep the issue open a little while longer, would plan to close it once I can confirm that things are in working order on staging.

[Abby-Wheelis]

This change has been on staging for a while now, and I just logged out and back in on one of my test phones, then observed the logs, where I found logDebug statements indicating that the appropriate event listeners were being added, and that the events were being published.

I can see in surrounding log statements that the functions registered to the event are also being executed - ie registering push notifications. I feel reasonably convinced that this is working, based on the log statements I'm seeing and the working functionality, login and push notifications seem to be fully functional.

[shankari]

@Abby-Wheelis can you also confirm that you are seeing push notifications being delivered to your phone (the daily reminder)?

[Abby-Wheelis]

Yes, I have a daily reminder notification on that phone from 1 hr ago, as well as others from earlier times. I also just quit the app and saw the appropriate "please don't quit" notification (screenshot from iPhone)

I also have "please label" notifications on both my LG and Samsung phones for today, as expected.