feat: add download support and bridge to React Native - #2
Conversation
|
@coderabbitai review |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new download management feature for TPStreams was introduced, including native Android and React Native module implementations. The Android build configuration was updated to use a newer player library version. The new functionality exposes download operations and status queries to the JavaScript layer, with corresponding TypeScript types and exports. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript (TPStreamsDownload)
participant RN as React Native Bridge
participant Native as TPStreamsDownloadModule (Android)
participant Tracker as DownloadTracker
JS->>RN: getDownloads()
RN->>Native: getDownloads(promise)
Native->>Tracker: fetchDownloads()
Tracker-->>Native: List<DownloadItem>
Native-->>RN: resolve(promise, downloads)
RN-->>JS: Promise resolves with downloads
JS->>RN: pauseDownload(videoId)
RN->>Native: pauseDownload(videoId, promise)
Native->>Tracker: pause(videoId)
Tracker-->>Native: success/failure
Native-->>RN: resolve/reject(promise)
RN-->>JS: Promise resolves with result
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/index.tsx (1)
10-12: Re-export looks fine – double-check tree shakingNothing blocking, but note that wildcard re-exports alongside a default export may prevent treeshaking in consumer bundles if they import
*from your package.
If bundle size is a concern, consider a namespaced export instead.src/TPStreamsDownload.tsx (1)
16-40: Wrap native calls with type guardsA rejected promise from native will propagate an untyped error. Wrapping the calls lets you normalise the error shape for TS consumers:
- getDownloads(): Promise<DownloadItem[]> { - return TPStreamsDownload.getDownloads(); - }, + async getDownloads(): Promise<DownloadItem[]> { + try { + return await TPStreamsDownload.getDownloads(); + } catch (e: any) { + throw new Error(e?.message ?? 'Failed to fetch downloads'); + } + },Replicate for the other methods if you want unified error handling.
android/src/main/java/com/tpstreams/TPStreamsDownloadModule.kt (1)
14-21: RuntimeException on init will crash the appThrowing inside a lazy delegate bubbles up during the first JS call, crashing the whole RN process.
Prefer rejecting the individual promise instead, or at least log & return a non-fatal error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android/build.gradle(1 hunks)android/src/main/java/com/tpstreams/TPStreamsDownloadModule.kt(1 hunks)android/src/main/java/com/tpstreams/TPStreamsRNPackage.kt(1 hunks)src/TPStreamsDownload.tsx(1 hunks)src/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-ios
- GitHub Check: build-android
🔇 Additional comments (2)
android/build.gradle (1)
84-88: Confirm artefact availability for bumped player version 0.0.8The version bump is straightforward, but the custom Maven repo (
TPStreamsAndroidPlayerMaven) occasionally lags behind public tags. A missing artefact will break the Gradle sync at consumer apps.#!/bin/bash # Sanity-check that the pom for 0.0.8 exists in the custom repo curl -s -o /dev/null -w "%{http_code}\n" \ https://raw.githubusercontent.com/testpress/TPStreamsAndroidPlayerMaven/main/repo/com/tpstreams/tpstreams-player/0.0.8/tpstreams-player-0.0.8.pomThe script should return
200. Any other status means the artefact is not yet published.android/src/main/java/com/tpstreams/TPStreamsRNPackage.kt (1)
16-20: Module list update looks good
TPStreamsDownloadModuleis now exposed to JS. No duplicate names detected. 👍
Summary by CodeRabbit