Skip to content

Introduce React Native OTA updates with zephyr-ota-plugin#293

Open
HigorAlves wants to merge 14 commits intomainfrom
feat/OTA-updates
Open

Introduce React Native OTA updates with zephyr-ota-plugin#293
HigorAlves wants to merge 14 commits intomainfrom
feat/OTA-updates

Conversation

@HigorAlves
Copy link
Contributor

What's added in this PR?

This PR introduces the initial implementation of the zephyr-ota-plugin library for managing OTA (Over-The-Air) updates in React Native applications. Key features include:

  • Core library implementation for seamless OTA updates via Zephyr Cloud.
  • API client for fetching remote metadata and updates.
  • React hooks and providers for integration into React Native apps.
  • Features like periodic update checks, app reload, and version tracking.
  • Utilities for dependency management and platform detection.
  • Inclusion of unit tests for key components and functionalities.
  • Configuration of ESLint, Jest, and TypeScript.
  • Comprehensive documentation, example usage code, and default configurations.

Screenshots

Not applicable.

What's the issues or discussion related to this PR ?

This library was created to streamline the integration of OTA updates into React Native apps, simplifying version management and minimizing user disruption during updates.

What are the steps to test this PR?

  1. Install the library in a React Native project.
  2. Configure the API client with valid credentials.
  3. Test periodic update checks, version tracking, and reloading functionality.
  4. Run unit tests to ensure utilities and integrations work as expected.

Documentation update for this PR (if applicable)?

The library's setup guide and usage examples are included in the documentation. Please include this PR into zephyr-documentation.

(Optional) What's left to be done for this PR?

Fine-tuning and feedback-based iterations may be required after wider usage.

(Optional) What's the potential risk and how to mitigate it?

Potential configuration issues or platform-specific edge cases. Mitigation includes thorough testing across platforms and providing fallback mechanisms.

(Required) Pre-PR/Merge checklist

  • I have added/updated/opened a PR to documentation to cover this new behavior
  • I have added an explanation of my changes
  • I have written new tests (if applicable)
  • I have tested this locally (standing from a first-time user point of view, never touched this app before)
  • I have/will run tests, or ask for help to add test

@HigorAlves HigorAlves self-assigned this Dec 17, 2025
…pdates

- Added core implementation of the `zephyr-ota-plugin` library.
- Integrated API client for resolving remote metadata from Zephyr Cloud.
- Included React hooks and provider for easy integration into React Native applications.
- Supported periodic update checks, app reloads, and version tracking.
- Implemented utilities for dependency parsing and platform detection.
- Added comprehensive unit tests for key utilities and API integration.
- Configured ESLint, Jest, and TypeScript for the library.
- Provided documentation, examples, and default configurations.
@nx-cloud
Copy link

nx-cloud bot commented Dec 17, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit b16db16

Command Status Duration Result
nx run e2e-deployment:e2e-test -- --passWithNoT... ❌ Failed 14s View ↗
nx affected -t build --skipRemoteCache ✅ Succeeded 3m 23s View ↗
nx affected -t test ✅ Succeeded 3m 6s View ↗
nx affected -t lint ✅ Succeeded 8s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-23 12:40:57 UTC

# Conflicts:
#	pnpm-lock.yaml
@arthurfiorette
Copy link
Member

Some of them are nitpicks and you can just ignore and some are valid questions.

Nice work btw

@HigorAlves
Copy link
Contributor Author

@claude please review

@claude
Copy link

claude bot commented Dec 18, 2025

Claude finished @HigorAlves's task in 2m 1s —— View job


Code Review Progress

  • Read repository guidelines and setup files
  • Review package configuration and dependencies
  • Review core API client implementation
  • Review OTA service implementation
  • Review React components and hooks
  • Review utilities and helpers
  • Analyze existing review comments
  • Provide comprehensive feedback

Code Review for zephyr-ota-plugin

Great work on implementing this OTA plugin! The overall architecture is solid, with good separation of concerns between the API client, storage, service layer, and React integration. Here's my detailed feedback:

Critical Issues

1. Security: Platform.OS Type Safety (libs/zephyr-ota-plugin/src/lib/utils/platform.ts:12)

The cast Platform.OS as BuildTarget is unsafe. Platform.OS can be 'ios', 'android', 'web', 'windows', or 'macos'. Sending unintended values to the API could cause issues.

Recommendation: Add validation:

export function getBuildTarget(): BuildTarget {
  const os = Platform.OS;
  if (os !== 'ios' && os !== 'android') {
    throw new Error(`Unsupported platform: ${os}. OTA updates only work on iOS and Android.`);
  }
  return os;
}

2. Potential API Bombardment (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:171)

As @arthurfiorette noted, with 100 remotes, this could hammer the API with 100 parallel requests. While your response about OTA needing to check all remotes is valid, consider implementing:

  • A concurrency limit (e.g., max 10 parallel requests)
  • Request batching if the API supports it
  • Exponential backoff on failures

Recommendation: Use a promise queue with concurrency limit:

async function resolveWithConcurrency<T>(
  items: T[],
  fn: (item: T) => Promise<any>,
  concurrency: number
): Promise<any[]> {
  const results: any[] = [];
  for (let i = 0; i < items.length; i += concurrency) {
    const batch = items.slice(i, i + concurrency);
    results.push(...await Promise.all(batch.map(fn)));
  }
  return results;
}

3. Missing Error Handling in Cache Clearing (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:254-273)

The cache clearing logic silently catches errors but continues. If cache clearing fails for some remotes, the user might not get the updates after reload.

Recommendation: Collect failures and warn the user, or retry the failed operations.

High Priority Issues

4. Unused Function - createZephyrOTAService (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:385)

As @arthurfiorette mentioned, this function is redundant. Users can just call new ZephyrOTAService() directly. The function doesn't add any value since it's just a wrapper.

Recommendation: Remove this function or make it do something useful (like validation, setup, etc.).

5. Unused Factory Function - OTAStorage (libs/zephyr-ota-plugin/src/lib/core/storage.ts)

Similarly, there's no need for a factory function if it's just calling the constructor.

Recommendation: Remove the factory function if it exists.

6. Inconsistent Logging Structure (libs/zephyr-ota-plugin/src/lib/utils/logger.ts:40)

As @arthurfiorette noted, the logging here uses a custom implementation while other Zephyr plugins use the debug package (libs/zephyr-agent/src/lib/logging/debug.ts). This creates inconsistency across the ecosystem.

Recommendation: Refactor to use the shared logging structure from zephyr-agent for consistency:

  • Use debug package with namespace pattern like debug('zephyr:ota:...')
  • This provides better debugging experience with selective namespace enabling
  • Aligns with the existing Zephyr ecosystem patterns

7. URL Construction Readability (libs/zephyr-ota-plugin/src/lib/core/api-client.ts:49-53)

As @arthurfiorette mentioned, using URL object throughout would be more readable. You already use it here, which is good! However, line 116 uses string concatenation instead:

const url = `${defaultUrl}/__get_version_info__`;

Recommendation: Use URL object consistently for all URL construction.

8. useZephyrOTA Hook Naming (libs/zephyr-ota-plugin/src/lib/react/ZephyrOTAProvider.tsx:375)

As @arthurfiorette asked, why not call it useZephyrOTAContext? The current name doesn't clearly indicate it's accessing context.

Counterpoint: useZephyrOTA is actually fine - it's the public API name and follows common React patterns (like useAuth, useTheme, etc.). The internal implementation using context is an implementation detail.

Verdict: I'd keep the current name as it's more user-friendly.

9. Large File - Extract Types (libs/zephyr-ota-plugin/src/lib/react/ZephyrOTAProvider.tsx)

@arthurfiorette suggested extracting types to a separate file. You already did this in types.ts, so this is addressed!

10. Questionable useMemo (libs/zephyr-ota-plugin/src/lib/react/ZephyrOTAProvider.tsx:126-140)

@arthurfiorette noted the useMemo might not have benefits. Looking at the dependencies [config, dependencies], if these change, you DO want to recreate the service. The useMemo is appropriate here, but you could improve it:

Issue: config object might cause unnecessary rerenders if it's recreated on every render.

Recommendation: Advise users to memoize the config prop, or use useMemo inside to stabilize the config object.

11. Package.json Exports Field (libs/zephyr-ota-plugin/package.json:13)

As @arthurfiorette mentioned, RN bundlers sometimes have issues with exports field. Consider adding:

"exports": {
  ".": {
    "types": "./dist/index.d.ts",
    "require": "./dist/index.js",
    "default": "./dist/index.js"
  },
  "./*": "./*",
  "./package.json": "./package.json"
}

12. API Base URL Configuration (libs/zephyr-ota-plugin/src/lib/types/config.ts:37)

@arthurfiorette suggested using env vars as default values like edge-contract does. This would be helpful for consistency.

Recommendation: Import from edge-contract or add similar env var support:

import { ZE_API_ENDPOINT } from 'zephyr-edge-contract';

export const DEFAULT_OTA_CONFIG: Required<ZephyrOTAConfig> = {
  authToken: process.env.ZE_AUTH_TOKEN || '',
  apiBaseUrl: ZE_API_ENDPOINT(),
  // ...
};

Note: Ensure this works in React Native context (env vars might need special handling).

Medium Priority Issues

13. Parallel Updates in Service (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:120)

@arthurfiorette asked if this can work inside Promise.all. Yes, it can! The initializeVersionTracking method resolves all remotes in parallel (line 171), which is good. However, it's already using Promise.all, so this comment is addressed.

14. Missing Jsdoc for Public APIs

While the code has some inline comments, public APIs could benefit from more comprehensive JSDoc comments with examples.

15. Rate Limiting Logic (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:134-138)

The rate limiting only checks time since last check, but doesn't account for concurrent checks. If multiple components call checkForUpdates() simultaneously, you might have race conditions.

Recommendation: Add a lock mechanism or return the in-flight promise.

16. DevSettings Availability (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:299-306)

DevSettings might not be available in production builds. The code handles this gracefully, but consider documenting this limitation and providing an alternative reload mechanism.

Low Priority / Nitpicks

17. Unused onReloadRequested Prop (libs/zephyr-ota-plugin/src/lib/react/types.ts:98)

The onReloadRequested prop is defined but never used in the implementation.

Recommendation: Either implement it or remove it.

18. Test Coverage

Good that you included tests! However, consider adding:

  • Integration tests for the full OTA flow
  • Tests for error scenarios (network failures, invalid responses)
  • Tests for React hooks and provider

19. TypeScript Strictness

Consider enabling stricter TypeScript options:

  • strictNullChecks: true
  • noUncheckedIndexedAccess: true

20. Storage Error Handling

All storage methods catch errors and log warnings, but don't propagate them. This could hide issues. Consider:

  • Propagating critical errors
  • Providing a way for users to handle storage failures

Positive Highlights

  1. Excellent architecture - Clean separation of concerns between API, storage, service, and React layers
  2. Good documentation - The README is comprehensive with clear examples
  3. Auto-detection feature - The ability to detect remotes from MF runtime is a great DX improvement
  4. Rate limiting - Built-in rate limiting prevents API abuse
  5. Dismiss functionality - Thoughtful UX for users who want to postpone updates
  6. TypeScript usage - Good type definitions throughout
  7. Test coverage - Unit tests for utilities show attention to quality

Summary

This is a solid implementation with good architectural decisions. The main concerns are:

  1. Type safety for platform detection (critical)
  2. API concurrency limits to prevent overwhelming the backend (high priority)
  3. Logging consistency with other Zephyr packages (high priority)
  4. Minor code cleanup (removing redundant functions)

I'd recommend addressing the critical and high-priority issues before merging. The rest can be handled in follow-up PRs.


Copy link
Collaborator

@ryok90 ryok90 left a comment

Choose a reason for hiding this comment

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

As a ReactProvider this makes sense since it allows the user to input behavior into the OTA responses.
Since this is tightly related to ReactNative and Metro, it would be interesting to see this into metro plugin or repack plugin (if it supports and with a reexport in metro) instead of a separate NPM package.

One thing I noticed on metro plugin is the verbose declaration of options in the withZephyr when we do have all of that in zephyr-engine being derived from users config in MF plugin and other configs such as, target, remotes, name, and all of that. We don't need such parameters since we are already grabbing that info from configs. OTAProvider also won't need those configs if we have access to zephyr-manifest.json.

All of these changes can be done after this PR is merged but for now what is really important is that we reduce user's boilerplate into making OTA work, try and move this Provider into metro plugin, we can document and work on other changes on the next iteration.

…alidation, and imports

- Added `mapWithConcurrency` helper to limit concurrent API requests.
- Introduced platform validation to restrict usage to iOS and Android.
- Simplified imports and code structure by removing unused exports and redundant functions.
- Updated `apiBaseUrl` to dynamically resolve from `zephyr-edge-contract`.
- Enhanced package export definitions in `package.json`.
@HigorAlves
Copy link
Contributor Author

@claude please review

@claude
Copy link

claude bot commented Dec 19, 2025

Claude finished @HigorAlves's task in 1m 45s —— View job


Code Review Progress

  • Read repository guidelines (CLAUDE.md)
  • Review existing feedback and comments
  • Examine core implementation files
  • Check package.json and configuration
  • Provide review summary

Review of zephyr-ota-plugin PR

I've thoroughly reviewed this PR and the previous feedback from @arthurfiorette and the earlier Claude review. Here's my assessment:

Summary

This is a well-architected implementation of OTA updates for React Native with Module Federation. The code demonstrates good separation of concerns, thoughtful API design, and solid test coverage. Most of the critical issues raised in previous reviews have been addressed.

What's Been Fixed ✅

  1. Platform validation (libs/zephyr-ota-plugin/src/lib/utils/platform.ts:14-22) - Now properly validates Platform.OS before casting, throwing an error for unsupported platforms
  2. Concurrency limiting (libs/zephyr-ota-plugin/src/lib/core/ZephyrOTAService.ts:27-42) - Implemented mapWithConcurrency with MAX_CONCURRENT_REQUESTS = 5 to prevent API bombardment
  3. Package exports (libs/zephyr-ota-plugin/package.json:13-21) - Added proper exports field with ./* and ./package.json entries
  4. Environment variable defaults (libs/zephyr-ota-plugin/src/lib/types/config.ts:39) - Now imports ZE_API_ENDPOINT() from zephyr-edge-contract for consistency
  5. Types extracted - Types properly separated into dedicated files

Remaining Issues & Recommendations

High Priority

1. Logging Inconsistency

As @arthurfiorette noted, this plugin uses a custom logger (libs/zephyr-ota-plugin/src/lib/utils/logger.ts) while other Zephyr packages use the debug package with namespace patterns like debug('zephyr:*').

Impact: Inconsistent debugging experience across the Zephyr ecosystem. Users familiar with enabling debug output in other Zephyr tools won't have the same experience here.

Recommendation:

  • Consider refactoring to use the debug package with a namespace like zephyr:ota:*
  • This would align with libs/zephyr-agent/src/lib/logging/debug.ts patterns
  • Can be addressed in a follow-up PR if preferred

2. Unused Factory Functions

The codebase no longer exports createZephyrOTAService (checked index.ts:1-53), so @arthurfiorette's comment about removing it appears to be already addressed or never exported. Good!

3. URL Construction Inconsistency

Good use of URL object in api-client.ts:49-53, but line 116 uses string concatenation:

const url = `${defaultUrl}/__get_version_info__`;

Minor: Could use URL object for consistency, but string concatenation is fine here since we're just appending a path.

Medium Priority

4. Storage Error Handling Strategy

The storage module (libs/zephyr-ota-plugin/src/lib/core/storage.ts) catches all errors and logs warnings without propagating. This could hide critical issues like:

  • Corrupted AsyncStorage
  • Quota exceeded errors
  • Permission issues

Recommendation: Consider providing an error callback or event for storage failures so users can implement their own fallback/monitoring strategies.

5. useZephyrOTA Hook Name

@arthurfiorette questioned why not call it useZephyrOTAContext. I agree with the current naming - useZephyrOTA follows React ecosystem conventions (like useAuth, useTheme) and is more user-friendly. The internal implementation detail of using context shouldn't leak into the public API. ✅ Keep as-is.

6. useMemo in Provider

@arthurfiorette questioned the useMemo at ZephyrOTAProvider.tsx:126-140. The memo is appropriate since:

  • It prevents service recreation on every render
  • Dependencies [config, dependencies] correctly trigger recreation when needed
  • However, if config is an object literal passed inline, it could cause unnecessary recreations

Recommendation: Document in README that users should memoize/stabilize the config object if passing as a prop to avoid unnecessary service recreations.

7. Cache Clearing Error Handling

In ZephyrOTAService.ts:276-302, cache clearing catches errors but continues. If cache clearing fails for multiple remotes, the user might reload without getting the updates.

Suggestion: Consider collecting failures and:

  • Warning the user with specific remotes that failed
  • Possibly retrying failed cache clears
  • Can be a follow-up improvement

Low Priority / Nitpicks

8. DevSettings Availability

The code gracefully handles DevSettings not being available (ZephyrOTAService.ts:327-334). This is well done! Consider documenting this limitation in the README for production builds.

9. Missing onReloadRequested Implementation

The types define onReloadRequested prop but I don't see it used in the implementation. Either implement it or remove it from the types.

10. Test Coverage Gaps

Good unit test coverage for utilities! Consider adding:

  • Integration tests for the full OTA flow
  • Error scenario tests (network failures, invalid API responses)
  • React hooks/provider tests

Can be follow-up work.

Architecture Strengths 🌟

  1. Auto-detection feature - Brilliant DX improvement detecting remotes from MF runtime
  2. Clean separation - API client, storage, service, and React layers well separated
  3. Rate limiting - Built-in protection against API abuse
  4. Dismiss functionality - Thoughtful UX consideration
  5. Comprehensive types - Excellent TypeScript usage throughout
  6. Concurrency control - Good implementation of bounded parallelism

Addressing @ryok90's Architectural Comments

@ryok90 makes an excellent point about potentially moving this into metro/repack plugin to reduce boilerplate. This is a great idea for future iteration but shouldn't block this PR:

Pros of current approach:

  • Standalone package is easier to test and iterate on
  • Users can opt-in to OTA independently
  • Clearer separation of concerns

Future improvement:

  • Metro plugin could auto-inject the provider
  • Leverage zephyr-manifest.json to eliminate config duplication
  • Can be explored after this foundation is solid

Final Verdict

Recommendation: Approve with minor follow-ups

This is high-quality code that's ready to merge. The critical issues (platform validation, concurrency) have been addressed. The remaining items are:

  1. Must fix before merge: None - all critical issues resolved
  2. Should fix before merge: Consider the logging consistency (though can be follow-up)
  3. Nice to have: Everything else can be addressed in follow-up PRs

The implementation demonstrates solid engineering practices and will serve as a strong foundation for OTA updates in the Zephyr ecosystem.


Testing Notes:

  • All CI checks passing ✅
  • Nx Cloud pipeline succeeded ✅
  • Lint, build, and test targets all green ✅

# Conflicts:
#	pnpm-lock.yaml
…ug logging

- Introduced `StorageError` and `StorageErrorHandler` types for robust error reporting in storage operations.
- Added `reportError` utility for centralized error handling across storage methods.
- Integrated `debug` package for improved, scoped logging with pre-defined namespaces.
- Updated async storage operations with error reporting callbacks.
- Enhanced `logger` utility to support scoped loggers and dynamic debug enabling/disabling.
- Updated dependencies to include `debug` and `@types/debug`.
@Nsttt Nsttt closed this Dec 22, 2025
@Nsttt Nsttt deleted the feat/OTA-updates branch December 22, 2025 11:01
@Nsttt Nsttt restored the feat/OTA-updates branch December 22, 2025 11:01
@Nsttt Nsttt reopened this Dec 22, 2025
HigorAlves and others added 3 commits December 22, 2025 10:30
…tch resolve API

- Introduced `BatchResolveRequest` and `BatchResolveResponse` types.
- Implemented `resolveRemotesBatch` method in the API client for handling batch resolve requests.
- Replaced `mapWithConcurrency` logic with batch resolve in version tracking and remote resolution.
- Improved error handling with fallback to individual requests when batch resolve fails.
HigorAlves and others added 3 commits December 22, 2025 12:57
…ing utilities

- Deleted `detect-remotes` and `parse-dependencies` modules along with their tests.
- Removed unused exports and related types from the library.
- Updated `ZephyrOTAProvider` to adopt the new manifest-based update detection flow.
- Streamlined the library API to align with the updated architecture.
@HigorAlves HigorAlves removed their assignment Dec 23, 2025
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.

4 participants