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

feat: add workout mirroring support #137

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

gentilijuanmanuel
Copy link
Contributor

@gentilijuanmanuel gentilijuanmanuel commented Feb 24, 2025

Context

This PR, in addition to this one, completes the support to Workout Mirroring between an iOS app and its corresponding watchOS app, feature described in this WWDC episode.

Summary of changes

I added support two main APIs:

  • workoutSessionMirroringStartHandler: method to set up the delegate that will react to remote workout session event updates. Official documentation can be found here.
  • HKWorkoutSessionDelegate methods:
  1. workoutSession(_:didChangeTo:from:date:): to react to workout session state changes (paused, resumed, ended). Official documentation can be found here.
  2. workoutSession(_:didFailWithError:): to react to possible errors that can happen during a workout session. Official documentation can be found here.
  3. workoutSession(_:didReceiveDataFromRemoteWorkoutSession:): to react to data that's coming from the workout session (like workout metrics). Official documentation can be found here.

Every time we get updates from delegate methods, we emit an event using the EventEmitter, and consumer apps will be able to listen to them and perform actions accordingly.

Summary by CodeRabbit

  • New Features

    • Introduced workout session mirroring functionality to enhance workout management on iOS 17 and later.
    • Expanded support for remote workout sessions by adding event notifications for state changes, errors, and data reception.
    • Enabled asynchronous handling to improve responsiveness when initiating workout session mirroring.
    • Added new enums and interfaces for improved state management and error handling related to workout sessions.
  • Bug Fixes

    • Improved error handling in workout session management to provide more descriptive error messages.
  • Tests

    • Enhanced mock capabilities for workout session mirroring in the testing setup.

Copy link

coderabbitai bot commented Feb 24, 2025

Walkthrough

This update introduces workout session mirroring functionality for the React Native Healthkit integration. New native methods and properties are added in iOS code to manage workout sessions and delegate events for remote workout state changes, errors, and data reception. Corresponding TypeScript modules have been updated to export and type these new features. Additionally, mocks and type safety improvements have been implemented to support the new workout session mirroring handler across the codebase.

Changes

File(s) Change Summary
ios/ReactNativeHealthkit.m
ios/ReactNativeHealthkit.swift
Added new method workoutSessionMirroringStartHandler to initiate workout mirroring. In Swift, added _workoutSession property, reworked error handling in startWatchAppWithWorkoutConfiguration, and implemented new delegate methods to handle remote workout events.
src/index.ios.tsx
src/index.native.tsx
Imported and exported workoutSessionMirroringStartHandler. In the native file, the handler is defined to always return false.
src/native-types.ts Introduced WorkoutSessionState enum, interfaces (WorkoutStateChangeEvent, WorkoutErrorEvent), and types (RemoteSessionSharableData, WorkoutDataReceivedEvent). Updated the ReactNativeHealthkitTypeNative type and HealthkitEventEmitter to include the new event-handling and handler method.
src/utils/workoutSessionMirroringStartHandler.ts Created a new asynchronous function that calls the native workoutSessionMirroringStartHandler method and exports it as the default export.
src/test-setup.ts Added a new mock function workoutSessionMirroringStartHandler to the mock module in the test setup.
src/utils/subscribeToChanges.ts Updated the callback parameter's type annotation to explicitly define typeIdentifier as readonly HKSampleTypeIdentifier for improved type safety.

Sequence Diagram(s)

sequenceDiagram
    participant JS as JavaScript App
    participant Util as workoutSessionMirroringStartHandler (TS Utility)
    participant Native as ReactNativeHealthkit (Native Module)
    participant Session as HKWorkoutSession

    JS->>Util: Invoke workoutSessionMirroringStartHandler()
    Util->>Native: Call native workoutSessionMirroringStartHandler(resolve, reject)
    Native->>Session: Set _workoutSession and assign delegate
    Session-->>Native: Trigger delegate methods (state change, error, data)
    Native-->>JS: Emit events (onRemoteWorkoutStateChange, onRemoteWorkoutError, onRemoteWorkoutDataReceived)
Loading

Poem

I'm a hopping rabbit in the code's bright glen,
Mirroring workouts now sprout time and again.
With new sessions leaping in Swift and TS style,
My code burrows deeper, mile after mile.
Happy bits and bytes make me smile! 🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/native-types.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

Error [ERR_REQUIRE_ESM]: require() of ES Module /node_modules/.pnpm/[email protected][email protected][email protected]/node_modules/eslint-plugin-functional/lib/index.js from /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported.
Instead change the require of index.js in /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs to a dynamic import() which is available in all CommonJS modules.
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at ConfigArrayFactory._loadPlugin (/node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3433:42)
at /node_modules/.pnpm/@eslint[email protected]/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3304:33
at Array.reduce ()


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6206f07 and 3c116d8.

📒 Files selected for processing (1)
  • src/native-types.ts (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: gentilijuanmanuel
PR: kingstinct/react-native-healthkit#137
File: src/utils/workoutSessionMirroringStartHandler.ts:1-11
Timestamp: 2025-02-25T15:13:50.357Z
Learning: The `workoutSessionMirroringStartHandler` function in React Native Healthkit does not initiate workout mirroring between iOS and watchOS. Instead, it sets up a delegate that will receive and handle workout change notifications from the Apple Watch app to the companion iOS app.
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: eas-ios / eas-build
🔇 Additional comments (6)
src/native-types.ts (6)

1854-1856: Great addition of JSDoc documentation!

The documentation link to Apple's reference helps developers understand the source of this enum and improves the overall code quality.


2049-2056: This enum looks well-structured and matches the Apple documentation.

The WorkoutSessionState enum provides clear, descriptive states that correspond to Apple's HKWorkoutSessionState documentation.


2068-2071: The flexibility with Record<string, unknown> aligns with your implementation goals.

Based on your previous feedback about the need for flexibility to mirror Swift's [String: Any] dictionary pattern, this implementation makes sense. Users can implement their own payload serialization pattern similar to your RemoteSessionSharable protocol in Swift.


2073-2079: Well-structured event types for remote workout data.

These types provide a clean interface for consuming workout data and events from the remote session.


2378-2379: The event emitter update properly integrates the new workout mirroring events.

This change correctly extends the HealthkitEventEmitter interface to include the new workout mirroring event types and their corresponding callbacks.


2049-2079: Implementation correctly matches the PR objectives for workout mirroring.

The added types, events, and handler correctly implement the workout session mirroring functionality described in the PR objectives. The workoutSessionMirroringStartHandler sets up a delegate to handle remote workout session events, and the event types provide the necessary structure for communicating workout state changes, errors, and data between iOS and watchOS apps.

I see you've implemented:

  1. The workoutSessionMirroringStartHandler function as described
  2. State change events matching workoutSession(_:didChangeTo:from:date:)
  3. Error handling for workoutSession(_:didFailWithError:)
  4. Data transmission for workoutSession(_:didReceiveDataFromRemoteWorkoutSession:)

Also applies to: 2329-2332, 2371-2380

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

pkg-pr-new bot commented Feb 24, 2025

Open in Stackblitz

npm i https://pkg.pr.new/kingstinct/react-native-healthkit/@kingstinct/react-native-healthkit@137

commit: 3c116d8

@gentilijuanmanuel gentilijuanmanuel marked this pull request as ready for review February 24, 2025 18:46
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b357c61 and 507846a.

📒 Files selected for processing (8)
  • ios/ReactNativeHealthkit.m (1 hunks)
  • ios/ReactNativeHealthkit.swift (3 hunks)
  • src/index.ios.tsx (3 hunks)
  • src/index.native.tsx (3 hunks)
  • src/native-types.ts (3 hunks)
  • src/test-setup.ts (1 hunks)
  • src/utils/subscribeToChanges.ts (1 hunks)
  • src/utils/workoutSessionMirroringStartHandler.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: eas-ios / eas-build
🔇 Additional comments (17)
src/test-setup.ts (1)

47-47: LGTM!

The mock function for workoutSessionMirroringStartHandler is correctly added to the mockModule object, maintaining consistency with other mocks in the test setup.

src/index.ios.tsx (1)

43-43: LGTM!

The workoutSessionMirroringStartHandler is correctly imported and exported, following the established pattern in the file.

Also applies to: 196-196, 276-276

src/utils/subscribeToChanges.ts (1)

11-11: Enhance type safety
The introduced read-only type annotation ensures immutability and clarifies usage.

src/index.native.tsx (3)

106-106: Add placeholder for workout session mirroring
Returning a resolved promise with false is appropriate for unsupported platforms. Consider verifying or implementing the function for platforms that do support it.


160-160: Export newly added function
Exposing the function aligns with existing patterns. Confirm that references to this function are only used when the feature is actually supported.


216-216: Include function in named exports
Ensuring consistency in named exports is beneficial for discoverability. Double-check if the function is being called erroneously on platforms where it's not supported.

ios/ReactNativeHealthkit.swift (6)

16-23: Check bridging approach
Storing _workoutSession in an Any? property is a workable solution given iOS 17 availability. Just ensure there's no unintended casting issues in older iOS versions.


695-700: Add remote workout events
Adding these new event names is consistent with the new workout session mirroring functionality. Confirm that the JavaScript side listens for them.


2248-2269: Add startWatchAppWithWorkoutConfiguration
This method is well-structured and aligned with iOS 17.0's new API. The error handling approach is consistent with the rest of the code.


2272-2287: Setup mirroring session
Correctly references the workoutSessionMirroringStartHandler. The assignment of _workoutSession and setting the delegate ensures the events are funneled.


2293-2313: Handle state changes
This is a clear and appropriate usage of HKWorkoutSessionDelegate to dispatch remote workout state changes.


2315-2327: Handle session errors
Forwarding errors via onRemoteWorkoutError is a consistent approach. If there's a chance of sensitive error data, consider redacting.

src/native-types.ts (5)

2043-2050: LGTM! Well-defined workout session states.

The enum values are properly defined and match the expected state transitions for HKWorkoutSession.


2052-2056: LGTM! Well-structured state change event interface.

The interface is properly typed with readonly fields and captures all necessary information for state transitions.


2067-2069: LGTM! Well-structured data received event type.

The type properly handles an array of sharable data with readonly fields.


2319-2322: LGTM! Well-documented native method.

The method is properly typed and includes a link to Apple's documentation.


2361-2363: LGTM! Well-integrated event handling.

The new event types and callbacks are properly integrated into the existing event emitter interface.

Also applies to: 2367-2368

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 507846a and a7d9522.

📒 Files selected for processing (1)
  • src/utils/workoutSessionMirroringStartHandler.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: test
  • GitHub Check: typecheck
  • GitHub Check: lint
  • GitHub Check: eas-ios / eas-build

* @returns {Promise<boolean>} A promise that resolves to true if mirroring started successfully, false otherwise.
* @throws {Error} If there's an error starting the mirroring session.
*/
const workoutSessionMirroringStartHandler: () => Promise<boolean> = async () => Native.workoutSessionMirroringStartHandler()
Copy link

@coderabbitai coderabbitai bot Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider handling errors explicitly.

While the implementation is correct, you might want to add explicit error handling to provide more context when errors occur. This would make debugging easier for consumers of your API.

-const workoutSessionMirroringStartHandler: () => Promise<boolean> = async () => Native.workoutSessionMirroringStartHandler()
+const workoutSessionMirroringStartHandler: () => Promise<boolean> = async () => {
+  try {
+    return await Native.workoutSessionMirroringStartHandler();
+  } catch (error) {
+    console.error('Failed to start workout session mirroring:', error);
+    throw error;
+  }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const workoutSessionMirroringStartHandler: () => Promise<boolean> = async () => Native.workoutSessionMirroringStartHandler()
const workoutSessionMirroringStartHandler: () => Promise<boolean> = async () => {
try {
return await Native.workoutSessionMirroringStartHandler();
} catch (error) {
console.error('Failed to start workout session mirroring:', error);
throw error;
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertherber I think this can be handled directly by the consumer app, what do you think?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34dd262 and 25eef34.

📒 Files selected for processing (1)
  • ios/ReactNativeHealthkit.swift (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: gentilijuanmanuel
PR: kingstinct/react-native-healthkit#137
File: src/utils/workoutSessionMirroringStartHandler.ts:1-11
Timestamp: 2025-02-25T15:13:50.357Z
Learning: The `workoutSessionMirroringStartHandler` function in React Native Healthkit does not initiate workout mirroring between iOS and watchOS. Instead, it sets up a delegate that will receive and handle workout change notifications from the Apple Watch app to the companion iOS app.
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: eas-ios / eas-build
🔇 Additional comments (1)
ios/ReactNativeHealthkit.swift (1)

694-700: Good job enumerating new events.

Including "onRemoteWorkoutStateChange", "onRemoteWorkoutError", and "onRemoteWorkoutDataReceived" in supportedEvents() cleanly extends the event system for remote workout sessions.

Comment on lines +2247 to +2288

@available(iOS 17.0.0, *)
@objc(startWatchAppWithWorkoutConfiguration:resolve:reject:)
func startWatchAppWithWorkoutConfiguration(
_ workoutConfiguration: NSDictionary,
resolve: @escaping RCTPromiseResolveBlock,
reject: @escaping RCTPromiseRejectBlock
) {
guard let store = _store else {
return reject(INIT_ERROR, INIT_ERROR_MESSAGE, nil)
}

let configuration = parseWorkoutConfiguration(workoutConfiguration)

store.startWatchApp(with: configuration) { success, error in
if let error {
reject(GENERIC_ERROR, error.localizedDescription, error)
return
}

resolve(success)
}
}

@available(iOS 17.0.0, *)
@objc(workoutSessionMirroringStartHandler:reject:)
func workoutSessionMirroringStartHandler(
resolve: @escaping RCTPromiseResolveBlock,
reject: @escaping RCTPromiseRejectBlock
) {
guard let store = _store else {
return reject(INIT_ERROR, INIT_ERROR_MESSAGE, nil)
}

store.workoutSessionMirroringStartHandler = { [weak self] mirroringSession in
self?._workoutSession = mirroringSession
self?._workoutSession?.delegate = self
}

resolve(true)
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Validate or handle edge cases for starting the Watch app.

When calling startWatchAppWithWorkoutConfiguration, consider validating the dictionary keys in workoutConfiguration or handling scenarios where the Watch might be unreachable. This improves resilience against unexpected configurations and partial failures.

Comment on lines 2291 to 2369
// MARK: - HKWorkoutSessionDelegate

extension ReactNativeHealthkit: HKWorkoutSessionDelegate {

@available(iOS 17.0.0, *)
func workoutSession(
_ workoutSession: HKWorkoutSession,
didChangeTo toState: HKWorkoutSessionState,
from fromState: HKWorkoutSessionState,
date: Date
) {
Task { @MainActor [weak self] in
guard let self = self else { return }

if self.bridge != nil && self.bridge.isValid {
self.sendEvent(withName: "onRemoteWorkoutStateChange", body: [
"toState": toState.rawValue,
"fromState": fromState.rawValue,
"date": self._dateFormatter.string(from: date)
])
}
}
}

@available(iOS 17.0.0, *)
func workoutSession(_ workoutSession: HKWorkoutSession, didFailWithError error: any Error) {
Task { @MainActor [weak self] in
guard let self = self else { return }

if self.bridge != nil && self.bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutError",
body: ["error": error.localizedDescription]
)
}
}
}

@available(iOS 17.0.0, *)
func workoutSession(
_ workoutSession: HKWorkoutSession,
didReceiveDataFromRemoteWorkoutSession data: [Data]
) {
Task { [weak self] in
guard let self = self else { return }

do {
let serializedData = try data.compactMap { dataItem -> [String: Any]? in
guard let json = try? JSONSerialization.jsonObject(with: dataItem) as? [String: Any] else {
return nil
}
return json
}

await MainActor.run { [weak self] in
guard let self = self else { return }

if let bridge = self.bridge, bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutDataReceived",
body: ["data": serializedData]
)
}
}
} catch {
await MainActor.run { [weak self] in
guard let self = self else { return }

if self.bridge != nil && self.bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutError",
body: ["error": error.localizedDescription]
)
}
}
}
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Log or handle parsing errors for remote session data.

Within didReceiveDataFromRemoteWorkoutSession, items failing JSON decoding are quietly skipped. Providing a fallback, logging an error, or sending a separate event for invalid payloads could improve debugging and transparency.

Comment on lines +16 to +23
#if os(iOS)
@available(iOS 17.0, *)
var _workoutSession: HKWorkoutSession? {
get { return __session as? HKWorkoutSession }
set { __session = newValue }
}
private var __session: Any?
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider storing the session property with a concrete type.

The private property __session is declared as Any?. Unless there is a specific need to treat the stored object generically, using HKWorkoutSession? directly would improve clarity and remove the need for casting in _workoutSession.

 #if os(iOS)
 @available(iOS 17.0, *)
-var _workoutSession: HKWorkoutSession? {
-  get { return __session as? HKWorkoutSession }
-  set { __session = newValue }
-}
-private var __session: Any?
+var _workoutSession: HKWorkoutSession?
 #endif
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#if os(iOS)
@available(iOS 17.0, *)
var _workoutSession: HKWorkoutSession? {
get { return __session as? HKWorkoutSession }
set { __session = newValue }
}
private var __session: Any?
#endif
#if os(iOS)
@available(iOS 17.0, *)
var _workoutSession: HKWorkoutSession?
#endif

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25eef34 and 6206f07.

📒 Files selected for processing (2)
  • ios/ReactNativeHealthkit.swift (3 hunks)
  • src/native-types.ts (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: gentilijuanmanuel
PR: kingstinct/react-native-healthkit#137
File: src/utils/workoutSessionMirroringStartHandler.ts:1-11
Timestamp: 2025-02-25T15:13:50.357Z
Learning: The `workoutSessionMirroringStartHandler` function in React Native Healthkit does not initiate workout mirroring between iOS and watchOS. Instead, it sets up a delegate that will receive and handle workout change notifications from the Apple Watch app to the companion iOS app.
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: eas-ios / eas-build
🔇 Additional comments (14)
ios/ReactNativeHealthkit.swift (7)

16-23: Consider simplifying session storage

Storing _workoutSession directly as HKWorkoutSession? may remove the need to maintain __session as Any? and avoid repeated casts. If there’s no specific legacy constraint requiring the generic property, you can simplify by removing it.


695-701: New remote workout events look good

These additions cleanly expand the available event types for remote workout interactions, following a consistent naming pattern.


2248-2249: No functional code here

These lines appear to be empty or spacing changes with no observable impact.


2292-2293: Delegate marker comment

Just a new section marker with no direct logic changes.


2294-2295: New extension for HKWorkoutSessionDelegate

Declaring the extension is straightforward. Implementation details follow.


2296-2314: Session state change handling looks good

The code correctly captures the state changes and emits them to JavaScript. This straightforward approach leverages background tasks to update the main actor if necessary.


2316-2328: Error handling is appropriately routed

You send an “onRemoteWorkoutError” event whenever an error occurs, which is consistent with the other delegate callbacks.

src/native-types.ts (7)

2043-2053: Enum values look good.

Defining numeric values for each session state is appropriate, as it aligns well with underlying native constants. This approach preserves consistency and facilitates easier comparisons or references in bridging code.


2055-2067: Enum event definitions appear consistent.

These event types match their native counterparts, ensuring clear mapping from HealthKit’s delegate callbacks.


2075-2077: Repeating previous suggestion on more structured errors.

A single string for error reduces the ability to offer programmatic error handling. Consider returning a structured object with a code, message, and domain for better clarity.


2079-2082: Retaining flexibility for arbitrary payloads.

Users may benefit from a discriminated union for stronger type safety, but since you decided to allow custom dictionaries, this remains fine.


2084-2086: Data reception event is well-defined.

No issues found. The read-only array constraint ensures immutability of received data, which is a good practice.


2088-2090: Straightforward event wrapper.

The property type referencing the new WorkoutSessionEventType is suitably typed, ensuring clarity for event consumers.


2389-2390: Event registration signature is coherent.

The function overload neatly accommodates new event types, preserving clarity. Great job extending your event model!

Comment on lines +2250 to +2270
@objc(startWatchAppWithWorkoutConfiguration:resolve:reject:)
func startWatchAppWithWorkoutConfiguration(
_ workoutConfiguration: NSDictionary,
resolve: @escaping RCTPromiseResolveBlock,
reject: @escaping RCTPromiseRejectBlock
) {
guard let store = _store else {
return reject(INIT_ERROR, INIT_ERROR_MESSAGE, nil)
}

let configuration = parseWorkoutConfiguration(workoutConfiguration)

store.startWatchApp(with: configuration) { success, error in
if let error {
reject(GENERIC_ERROR, error.localizedDescription, error)
return
}

resolve(success)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Ensure thorough error handling when starting the Watch app

You correctly reject the promise on error. Consider supplementing errors with extra context or user-facing instructions if the Watch is inactive or unreachable. This can help users diagnose potential setup issues more easily.

Comment on lines +2271 to +2289

@available(iOS 17.0.0, *)
@objc(workoutSessionMirroringStartHandler:reject:)
func workoutSessionMirroringStartHandler(
resolve: @escaping RCTPromiseResolveBlock,
reject: @escaping RCTPromiseRejectBlock
) {
guard let store = _store else {
return reject(INIT_ERROR, INIT_ERROR_MESSAGE, nil)
}

store.workoutSessionMirroringStartHandler = { [weak self] mirroringSession in
self?._workoutSession = mirroringSession
self?._workoutSession?.delegate = self
}

resolve(true)
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Validate mirroring start logic

This handler succeeds immediately by returning true without waiting for confirmation that mirroring actually began. If you need to confirm a session was established successfully, consider providing a success/failure callback from the property’s closure. Also note that if multiple calls override store.workoutSessionMirroringStartHandler, you’ll want to ensure that’s your intended behavior.

Comment on lines +2330 to +2369
@available(iOS 17.0.0, *)
func workoutSession(
_ workoutSession: HKWorkoutSession,
didReceiveDataFromRemoteWorkoutSession data: [Data]
) {
Task { [weak self] in
guard let self = self else { return }

do {
let serializedData = try data.compactMap { dataItem -> [String: Any]? in
guard let json = try? JSONSerialization.jsonObject(with: dataItem) as? [String: Any] else {
return nil
}
return json
}

await MainActor.run { [weak self] in
guard let self = self else { return }

if let bridge = self.bridge, bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutDataReceived",
body: ["data": serializedData]
)
}
}
} catch {
await MainActor.run { [weak self] in
guard let self = self else { return }

if self.bridge != nil && self.bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutError",
body: ["error": error.localizedDescription]
)
}
}
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider partial error handling for JSON parsing

Currently, individual data items failing JSON decoding are silently skipped. If you need greater visibility, consider logging or sending a separate error event for those partial failures to aid debugging.

Comment on lines +2371 to +2379
@available(iOS 17.0, *)
func workoutSession(_ workoutSession: HKWorkoutSession, didGenerate event: HKWorkoutEvent) {
if self.bridge != nil && self.bridge.isValid {
self.sendEvent(
withName: "onRemoteWorkoutEventReceived",
body: ["type": event.type.rawValue]
)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Provide more event details if needed

Right now, only the event type is sent. If you need to expose finer-grained metadata (e.g., event timestamps or reasons), consider sending additional fields to better inform the JS layer.

Comment on lines +2069 to +2073
export interface WorkoutStateChangeEvent {
readonly toState: WorkoutSessionState;
readonly fromState: WorkoutSessionState;
readonly date: string;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider using a stronger type for date.

If possible, using an ISO 8601 string type (e.g., ISODateString) or a Date object can help with type safety and clarity, ensuring consumers can parse it easily.

Comment on lines +2340 to +2343
/**
* @see {@link https://developer.apple.com/documentation/healthkit/hkhealthstore/4172878-workoutsessionmirroringstarthand Apple Docs }
*/
readonly workoutSessionMirroringStartHandler: () => Promise<boolean>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Confirm error handling for mirrored sessions.

Returning a boolean may not be sufficient in scenarios where watchOS isn’t paired or available. Consider an alternative return value or additional detail (e.g., an error code) to enhance debugging.

Comment on lines +2382 to +2385
type OnRemoteWorkoutStateChangeCallback = (event: WorkoutStateChangeEvent) => void;
type OnRemoteWorkoutErrorCallback = (event: WorkoutErrorEvent) => void;
type OnRemoteWorkoutDataCallback = (event: WorkoutDataReceivedEvent) => void;
type OnRemoteWorkoutEventReceivedCallback = (event: WorkoutEventReceivedEvent) => void;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Callback definitions look neat.

Adding brief doc comments to each callback could improve maintainability and guide integrators, especially around expected parameter shapes and usage contexts.

Copy link

github-actions bot commented Mar 6, 2025

Expo preview is ready for review.

There should soon be a new native build available here

With that installed just scan this QR code:
QR Preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant