Discussion: Refactor background observers to use the same eventEmitter

[CaptainJeff]

Is your feature request related to a problem? Please describe. In order to listen to multiple measurement types in the background, you have to create 4 event emitters for every type. e.g.

NativeAppEventEmitter.addListener('healthKit:HeartRate:setup:success', callback)
NativeAppEventEmitter.addListener('healthKit:HeartRate:new', callback)
NativeAppEventEmitter.addListener('healthKit:HeartRate:setup:failure', callback)
NativeAppEventEmitter.addListener('healthKit:HeartRate:failure', callback)

NativeAppEventEmitter.addListener('healthKit:Workout:setup:success', callback)
NativeAppEventEmitter.addListener('healthKit:Workout:new', callback)
NativeAppEventEmitter.addListener('healthKit:Workout:setup:failure', callback)
NativeAppEventEmitter.addListener('healthKit:Workout:failure', callback)

Describe the solution you'd like It seems like we could refactor this to reuse the 4 events and pass the measurement type as the parameter instead of an empty object

Describe alternatives you've considered None

Additional context We could use the following five events (the four listed above plus "sample)

        @"healthKit:new",
        @"healthKit:failure",
        @"healthKit:enabled",
        @"healthKit:sample",
        @"healthKit:setup:success",
        @"healthKit:setup:failure"

And instead of inserting the type into the string of the event emitter

NSString *successEvent = [NSString stringWithFormat:@"healthKit:%@:setup:success", type];

We could instead pass

NSString *successEvent = [NSString stringWithFormat:@"healthKit:setup:success"];
[self sendEventWithName:successEvent body:type];

And in the js side

eventListener = eventEmitter.addListener(`healthKit:new`, (type) => {
   // Inspect the measurement type (workout, heartrate, etc...)
})

I can open a PR for this. I just want to make sure the community agreed this was the right approach first

[CaptainJeff]

@GGGava Hey man. Just wanted to loop back to this and see if you had any thoughts on this approach.

[GGGava]

Hey @CaptainJeff, thank you for your contribution!

This seems like a very good idea, I just have some questions:

1- Why do we need this new event sample? What is the difference between sample and new? 2- Is it possible to implement this without breaking our current method? We would need to deprecate the current method but keep it working so we don't force ppl to update to this new method without previous warning.

[CaptainJeff]

1) sample is actually an event we currently have implemented https://github.com/agencyenterprise/react-native-health/blob/1a5557f09cf70b914150cbbe6ebbd91c58376060/RCTAppleHealthKit/RCTAppleHealthKit.m#L656

https://github.com/agencyenterprise/react-native-health/blob/1a5557f09cf70b914150cbbe6ebbd91c58376060/RCTAppleHealthKit/RCTAppleHealthKit%2BQueries.m#L1049

I don't really use it but I think it's used for different measurement categories 2) I think I could get it to work to be backwards compatible with some stricter checks in various places if necessary. Might be a little funky and verbose but could get it

[GGGava]

Cool, thank you very much, I believe we can proceed with that 🚀

Regarding the sample event, it's not mentioned in the docs, could you also check if it's not dead code? 😅

[CaptainJeff]

@GGGava I opened a PR for this https://github.com/agencyenterprise/react-native-health/pull/312 and made it backwards compatible. Let me know if you have any questions!