Skip to content

feat: add OneKey native image component - #90

Merged
huhuanming merged 8 commits into
mainfrom
codex/onekey-image
Sep 3, 2026
Merged

feat: add OneKey native image component#90
huhuanming merged 8 commits into
mainfrom
codex/onekey-image

Conversation

@huhuanming

@huhuanming huhuanming commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add the @onekeyfe/react-native-image Nitro HybridView backed by SDWebImage and Glide, with native placeholder/fallback rendering, preload/cache APIs, lifecycle events, recycling, TOS resizing, and bounded decode safety.
  • Upstream all current app-monorepo @onekeyfe/* patches into app-modules.
  • Extend @onekeyfe/react-native-tab-view with the existing animation, selection, haptics, iOS immediate-transition, TypeScript compatibility, and Android inactive-scene touch interception fixes.
  • Keep rounded native images and their loading placeholders clipped to the requested border radius.
  • Stabilize Android Glide memory-cache identity by sharing the custom downsample strategy; repeated identical mounts now hit memory instead of falling back to disk.
  • Preserve preload failure semantics for blank URIs and defer Android onDisplay until the native view is attached.
  • Bump all 37 publishable app-modules packages to 3.0.101 for one coherent release.

Image safety behavior

  • Cap encoded images at 32 MiB, animated images at 16 MiB, and data URIs at 8 MiB.
  • Cap animated images at 1000 frames and 60 seconds, with format-specific GIF, WebP, and APNG validation before Android animation decode.
  • Preserve target-size downsampling with a 16 MiB decoded target budget.
  • Isolate authenticated downloads and caches by explicit header digest; iOS automatic cookies are disabled so Cookie authentication must be explicit.
  • Keep iOS system AVIF support and reject Android AVIF until its decoder can enforce target decode dimensions.

Verification

  • Native image JS Jest: 14/14; TypeScript and ESLint passed.
  • Native image Android: 46/46 unit tests passed.
  • Android Example runtime: after unmounting and remounting four identical image requests, all 4 reported a memory-cache hit.
  • iOS cache audit: the default and header-specific managers share SDImageCache.shared, use value-stable cache identities, and have no Android-style object-identity issue.
  • All workspace packages prepared successfully.
  • Publish workflow succeeded: https://github.com/OneKeyHQ/app-modules/actions/runs/33715456651
  • Public npm verification confirmed @onekeyfe/react-native-image@3.0.101 and all 37 packages, with 3.0.101 on the latest dist-tag.
  • app-monorepo integration: refactor: migrate native images away from expo-image app-monorepo#13137.

@socket-security

socket-security Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​nitrogen@​0.36.5 ⏵ 0.37.099 +710083 +196 +1100
Updatednpm/​react-native-nitro-modules@​0.36.5 ⏵ 0.37.0100 +1100100 +196 +1100

View full report

Comment thread native-views/react-native-image/src/index.tsx Outdated
@originalix

Copy link
Copy Markdown
Contributor

Automated code review found blocking issues

@huhuanming

Reviewed commit 7efde38b39f1. This report always reflects the latest reviewed changes.

Review summary

Before: the repository had no OneKey-owned cross-platform image surface, and the tab/skeleton components used their existing native implementations. After: a new Nitro OneKeyImage view routes React props through generated bindings into SDWebImage on iOS and Glide on Android, with cache controls, TOS resizing, placeholders/fallbacks, animated-image safety checks, preload APIs, and lifecycle callbacks. Supporting work adds shared skeleton rendering, lazy tab placeholders and rejected-tab rollback, example coverage, native dependency wiring, and regenerated Nitro bindings. Existing image consumers and unrelated native modules remain behaviorally unchanged except for the explicitly updated Nitro/runtime version alignment.

What needs attention: Please resolve the preload input contract, Android display lifecycle parity, and the cross-platform animated-image safety-limit mismatch before merging.

Issues to address

  • P2 · Preload drops invalid sources and can report success
    • Impact: The public preload contract says success means all requested sources completed successfully, but the wrapper removes blank-URI entries before native validation. A call containing only invalid entries therefore resolves true, masking caller errors and differing from the native implementations, which mark blank sources as failures.
    • Suggested action: Preserve invalid entries for native validation or explicitly return false when any input URI is blank/invalid, and add a wrapper test for that case.
  • P2 · Android can emit onDisplay after the view is detached
    • Impact: The delayed Android callback checks generation and image state but not attachment/visibility. If a recycled or clipped view finishes loading and is detached before the posted frame runs, onDisplay can fire off-screen and clear a React placeholder prematurely; iOS explicitly requires the view to be attached.
    • Suggested action: Track attachment state in the Android host view and require it in scheduleOnDisplay, cancelling or deferring the callback across detach/reattach.
  • P2 · Animated-image safety limits differ across platforms
    • Impact: Android rejects animated canvases above 4,194,304 pixels while iOS allows up to 16,000,000 pixels, despite one shared OneKeyImage API. The same animated asset can therefore render on iOS but fail on Android without any documented platform-specific policy.
    • Suggested action: Unify the animated canvas limit across implementations, or document and expose the platform-specific policy with matching tests and user-facing behavior.

Validation gaps

The added unit tests cover many pure helpers and wrapper transitions, but no full native build or device-level test was run in this checkout because the Yarn install state is absent. Add integration coverage for invalid preload inputs, detach/reattach display callbacks, and a boundary animated canvas accepted or rejected consistently on both platforms.

@huhuanming

Copy link
Copy Markdown
Contributor Author

Review follow-up completed in 505e4da.

  • Revalidated all three reported P2 items against the current head; their existing replies and resolved states remain accurate.
  • Added whitespace-only render URI normalization with focused JS regression coverage.
  • Prevented non-finite Android skeleton durations from producing a zero-nanosecond frame period and crashing the frame callback.

Validation: image Jest 15/15; image and skeleton typecheck/lint passed; image Android JVM tests 46/46; image and skeleton Kotlin compilation passed; example app compileDebugKotlin passed. iOS was source-audited against its existing attachment, animated-boundary, and frame-buffer tests. No device tests or npm publish were run.

@huhuanming
huhuanming merged commit 1ac3562 into main Sep 3, 2026
3 checks passed
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.

3 participants