Repository navigation
fix: v2 release hardening (Nitro migration + static-image fix + review fixes) - #20
Merged
Merged
Conversation
- android: recycle intermediate bitmaps in ImagePreprocessor, guarded by reference-equality checks since Bitmap.createBitmap/scale return the source instance when no transform is actually applied - android: serialize access to the shared ML Kit detector between the synchronous frame-processor path and the async static-image path - ios: skip the redundant re-render pass in normalized(to:) when the target orientation is already .up - ci: validate the release tag before building the package in npm-release.yml, so a mismatch fails fast - chore: drop native-tests/ios/StaticImageDecodePlan.test.swift, the test:ios-static script, and its CI wiring; the hardcoded /private/tmp paths and hand-rolled swiftc compile step weren't worth keeping without a proper SPM/XCTest target
The XCODE_VERSION: 16.3 pin predates this branch and no longer exists on GitHub's macos-latest runner image (only 26.0.1-26.6.0 are installed now, per Apple's year-aligned Xcode versioning). Switch to latest-stable so this doesn't silently break again next time GitHub rotates old Xcode versions out of the image.
example/Gemfile.lock's BUNDLED WITH 2.1.4 forces bundle install to download and switch to that ancient Bundler on every run, but its vendored thor lib references DidYouMean::SPELL_CHECKERS, which no longer exists on Ruby 3.4 (the default on current macOS runners) - crashing before pod install ever runs. Bump to 4.0.17, the version already installed and working on the runner before the forced downgrade kicked in.
android/build.gradle sits at the repo root and is linked into the example app by local path, unlike every other native dependency here (all node_modules-nested). Without jsRootDir set, RNGP appears to resolve the JS root against this package's own package.json, which also declares "workspaces": ["example"], causing it to aggregate codegen output for every native module in the workspace - including @swmansion/react-native-bottom-sheet's - duplicating a class that module already builds for itself and failing app:mergeLibDexDebug.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
turbo.json's build:ios task didn't allowlist IOS_DESTINATION, so
Turborepo stripped it from the task's environment before running
"react-native build-ios --destination \"\${IOS_DESTINATION:-...}\"".
The script's fallback (generic/platform=iOS Simulator) then hit
xcodebuild's own destination handling and failed instantly - "xcodebuild
exited with error code '64'" with no compile output, less than 2ms after
starting. Allowlisting it lets the simulator destination computed by
build-ios.yml actually reach the build command.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
react-native build-ios unconditionally prepends its own
"generic/platform=iOS Simulator," to whatever --destination value is
passed - it never replaces it. Our computed IOS_DESTINATION already
included "platform=iOS Simulator", so the combined string ended up
with two "platform=" keys ("generic/platform=iOS Simulator,platform=iOS
Simulator,OS=...,name=..."), which xcodebuild rejected instantly with
exit code 64 and zero compile output. Drop the redundant prefix from
both the computed value and the local-dev fallback.
Also revert the earlier jsRootDir addition to android/build.gradle: two
CI runs confirmed it had no effect on the BottomSheetSurfaceViewManagerDelegate
duplicate-class error, and the yarn-hoisting theory behind it doesn't
hold (the package isn't hoisted to the repo root node_modules), so it
wasn't worth keeping as unverified guesswork.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Every other native dependency in the tree (nitro-image, vision-camera, vision-camera-worklets) ships an identical self-descriptor at its package root declaring empty ios/android platforms. Ours was missing entirely, even though package.json's files list expects it to exist. Without it, example/react-native.config.js worked around the gap by inlining an empty platforms override directly on the manual dependencies[pkg.name] entry. That's a different code path than a proper self-descriptor, and correlates with the one concrete anomaly in CI: every sibling library's generateCodegenSchemaFromJavaScript task logs "No modules to process" - ours didn't, and our own module ended up compiling com.facebook.react.viewmanagers.BottomSheetSurface- ViewManagerDelegate into itself, colliding with bottom-sheet's own copy at dex-merge time. Restoring the root config and dropping the now-redundant inline override lets the CLI resolve our package the same way it resolves every other properly-behaved dependency.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
Root cause found via a stray local build artifact
(android/build/generated/source/codegen/schema.json): our library's
generateCodegenSchemaFromJavaScript task was producing a 158-module
schema covering the ENTIRE example app's dependency tree (BlurView,
RNGestureHandler, RNSVG, BottomSheetSurfaceView, etc.), not just our
own package. That's why our module compiled its own copy of
com.facebook.react.viewmanagers.BottomSheetSurfaceViewManagerDelegate,
colliding with bottom-sheet's own copy at D8 dex-merge time.
Without a codegenConfig, RNGP has nothing to scope the schema
combiner to just our package, so it fell back to scanning our whole
package root - which, unlike sibling npm packages, has the entire
example/ workspace nested inside it.
Verified locally with combine-js-to-schema-cli:
- scanning "." (previous behavior): 158 modules, includes
BottomSheetSurfaceView
- scanning "src/specs" (this fix): 0 modules, "No modules to
process" - matching every other Nitro library in the tree
src/specs only contains .nitro.ts files, which never match the old
architecture's codegenNativeComponent/TurboModuleRegistry patterns,
so scoping codegen there is safe and produces an empty schema by
design.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
The dex-merge duplicate that failed earlier builds was masking a second issue: android/build.gradle never declares a Gradle dependency on react-native-nitro-image, so its prefab package was never on CMAKE_FIND_ROOT_PATH, and find_package(react-native-nitro-image REQUIRED) failed with "Findreact-native-nitro-image.cmake not found". Verified react-native-nitro-image is never referenced anywhere in cpp/, nitrogen/, or src/ - it's unused leftover from a template, not an actual dependency. Removing the find_package call and its target_link_libraries entry fixes the CMake configure step without touching anything the library actually uses.
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three commits hardening the library ahead of the v2 release:
feat: harden v2 Nitro release architecture— migrates iOS/Android off the legacy TurboModule bridge (handlers/serializers/parsers/use-cases) onto NitroHybridObjects, commits the generated Nitrogen output (nitrogen/generated/**, previously gitignored) so consumers don't need to run codegen, and hardens the npm release workflow (SHA-pinned actions, droppedid-token: write/workflow_dispatchfrom the build job, release-tag-vs-version validation, publish now uses the actualnpm packtarball with--ignore-scripts).fix(ios): downsample full-resolution static images— bounds ImageIO decoding for static images up to 25MP/10,000px down to ≤4MP/4,096px, with correct coordinate remapping so bounding boxes still map back to source pixels.fix: address v2 release-hardening review findings— fixes from an internal review pass of the two commits above:Bitmaps inImagePreprocessor(guarded by reference-equality checks, sinceBitmap.createBitmap/.scale()return the source instance when no transform is actually applied)normalized(to:)when the target orientation is already.upnpm-release.yml, so a mismatch fails fasttest:ios-staticscript andnative-tests/ios/StaticImageDecodePlan.test.swift(hand-rolledswiftccompile-and-run, hardcoded to machine-specific/private/tmppaths) along with its CI wiring — wasn't worth keeping without a proper SPM/XCTest target.No CRITICAL or HIGH-severity issues were found in the migration itself (clean deletion of the legacy bridge on both platforms, no dangling references, no force-unwraps/
!!in the new glue code, comprehensive error handling).Test plan
yarn lint,yarn typecheck,yarn test(93 tests) — passing (verified by pre-push hooks)