-
Notifications
You must be signed in to change notification settings - Fork 251
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
🚧 WIP: V2 #334
base: react-native-health-v2
Are you sure you want to change the base?
🚧 WIP: V2 #334
Conversation
- Samples are now encodable to String for bridging to react native
- Moved queries to a different file
- Fixed issue with metadata predicate
- Deleted old rn bindings - Created bindings for new core
- Moved sample type parameter to parameter object - Interval and anchor date parameters are now available in react native
- Fixed typo - Fixed issue when saving sample - Added unit field to quantity sampl- Fixed typo - Fixed issue when saving sample - Added unit field to quantity sample
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far :D only had a few questions and nitpicks
Also found a potential issue, not sure if already on your radar
import Foundation | ||
import HealthKit | ||
|
||
public protocol HealthKitType: RawRepresentable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Is the rawValue
always string? If yes I suggest changing it to RawRepresentable<String>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, nice, will do
public struct QuantityQuery { | ||
let startDate: Date? | ||
let endDate: Date? | ||
let isUserEntered: Bool? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: What does isUserEntered == nil
represent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Samples manually added by the user on Health app. I'll think in a better name 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, but what's the difference between false
and nil
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry lol
if set to false, it will only get samples that were not manually added. If nil
it won't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh I see makes sense now, my instinct on those situations is to go with an OptionSet
struct FetchMode: OptionSet {
let rawValue: Int
static var userEntered: Self { .init(rawValue: 1 << 0) }
static var automaticallyEntered: Self { .init(rawValue: 1 << 1) }
}
then you can use it like QuantityQuery(fetchMode: .userEntered)
or QuantityQuery(fetchMode: [.userEntered, .automaticallyEntered])
Bool?
is totally fine as well, in fact is probably a better choice if you plan on exposing this struct to objc
import HealthKit | ||
|
||
public protocol HealthKitType: RawRepresentable { | ||
var type: HKSampleType { get } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Since this protocol is not directly available in objective-c maybe you could add this HKSampleType
as a primary associated type to the protocol. So you don't need to cast the type when using a concrete type.
I totally understand not going for it if the compiler is giving you grief for using PATs, an alternative could be having a computed property that does the type casting on the concrete types.
enum QuantityType {
/// Don't use this directly. Use `quantitySampleType` instead.
public var type: HKSampleType { ... } // I wonder if there's a way to use @available here to help
public var quantitySampleType: HKQuantityType { type as! HKQuantityType }
var core: HealthKitCore? | ||
|
||
@objc | ||
func initHealthKit(_ read: Array<String>, write: Array<String>, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potential-issue: looks like is not possible to request workout
permissions since this won't try to create it. I think you might have to go for the imperative code approach here
var readTypes: [any HealthKitType] = []
for identifier in read {
guard let readType = QuantityType(rawValue: identifier) ?? WorkoutType(rawValue: identifier) else { continue }
readTypes.append(readType)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Workout is not properly exported yet, currently working on that 😅
var core: HealthKitCore? | ||
|
||
@objc | ||
func initHealthKit(_ read: Array<String>, write: Array<String>, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick:
nitpick:
func initHealthKit(_ read: Array<String>, write: Array<String>, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { | |
func initHealthKit(_ read: [String], write: [String], resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { |
- Fixed typo on statistical query - Addded config to fix error warning on exmaple app - Exported ids filter for quantity query
- Added missing resolve on save quantity query
…-to-health-kit-core chore: Add documentation to HealthKitCore module
…-workout-samples feat: Add metadata to workout samples
hey folks, I was just wondering if this is the current progress? https://github.com/agencyenterprise/react-native-health/projects/2 |
…info-to-workout-samples feat: Add activities info to workout samples
Is V2 abandoned? I see a number of recent PRs for v1 haven't been merged. I'm curious as to the status of this project as a whole. |
For everyone curious about the v2, it's not abandoned, but I do not have so much band to work in that lately. The last status is that I tested this branch and it was broken in the last versions of React Native due to the RN new architecture, so we have to finish the refactoring for the bridge between React and the native code. Hopefully, we'll have this launched by the end of this year. |
I appreciate the update, @ruan-azevedo! I apologize—I didn't hope for that to sound accusatory. We've all got day-jobs and life away from the keyboard. I appreciate the efforts you're putting in! |
Hey guys, I just want to let you know that I'm not working at AE Studio anymore and I'm not planning to continue working on this project. If you want some feature, PR merged, or want to ask for someone to take this PR, please reach out to AE Studio guys on LinkedIn. I would suggest to reach out to Melanie or Felipe Taboada. |
First of all, thanks for this library! I appreciate a lot all the time and work you guys have put into it <3 How is the V2 going? I was really hoping to start the migration to the new RN Arch early next year, but this library is preventing me from doing that. I think we would rather get the (bad?) news asap so we can plan a migration to another library :c Thanks again for your work 🙏 |
This is draft PR for a complete overhaul of react-native-health library. This code is a work in progress and is subject to change at any moment. You are welcome to test it but this is not stable or production ready.