Feat/new nitro#85
Conversation
…tionStyle` BREAKING CHANGE: No more `NitroInAppBrowserPresentationStyle` enum import required. Use string literals directly for presentation types.
WalkthroughReplace presentationStyle enum with string-literal union, update example app imports/safe-area handling and dependency versions, adjust iOS native presentation mapping and error throwing, and add multiple example Android/Gradle and iOS CI/workflow updates. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant JS as Nitro JS
participant iOS as iOS Native
App->>JS: open(url, { presentationStyle: "fullScreen" })
JS->>iOS: open(options with typed presentationStyle)
iOS->>iOS: map presentationStyle enum -> UIModalPresentationStyle
iOS->>iOS: create & present SFSafariViewController
iOS-->>JS: resolve/notify
JS-->>App: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
README.md (1)
39-46: README usage is out of sync with the new API and includes an async bug.
- presentationStyle should now use a string literal (e.g. 'fullScreen'), not the old enum.
- onPress callback uses await but is not async.
Apply this patch to fix the snippet:
-```js +```tsx ... - onPress={() => { + onPress={async () => { try { await NitroInAppBrowser.open('https://nowietech.com', { barColor: 'purple', controlColor: '#000000', dismissButtonLabel: 'close', - presentationStyle: NitroInAppBrowserPresentationStyle.FullScreen, + presentationStyle: 'fullScreen', }); } catch (error) { console.error(error); } }}ios/SafariBrowserPresenter.swift (1)
75-82: Missing UIKit import (UIApplication usage).
UIApplicationresides in UIKit. This will fail to compile without importing UIKit in this file.Apply:
import Foundation import NitroModules import SafariServices +import UIKit
🧹 Nitpick comments (5)
ios/SafariBrowserPresenter.swift (3)
55-59: Ensure UI dismissal happens on the main thread.
dismiss(animated:)must be called on the main thread. While most call sites dispatch to main,deinitalso callscleanupBrowser()which could execute off-main.Apply one of the following:
Option A — Dispatch if needed:
private func cleanupBrowser() { - safariVC?.dismiss(animated: true) - safariVC = nil - isPresenting = false + let performCleanup = { [weak self] in + self?.safariVC?.dismiss(animated: true) + self?.safariVC = nil + self?.isPresenting = false + } + if Thread.isMainThread { + performCleanup() + } else { + DispatchQueue.main.async { performCleanup() } + } }Option B — Annotate as main-actor (Swift Concurrency):
- private func cleanupBrowser() { + @MainActor private func cleanupBrowser() { safariVC?.dismiss(animated: true) safariVC = nil isPresenting = false }
21-21: Remove unused propertycurrentBrowser.This property is never read; consider removing to reduce noise.
- private var currentBrowser: SFSafariViewController? = nil
82-82: Switch to RuntimeError aligns with the rest of the module.Consistent with other Swift files using
RuntimeError.error(withMessage:). Consider tightening the message slightly.- throw RuntimeError.error(withMessage: "Failed to get root view controller getRootViewController(...)") + throw RuntimeError.error(withMessage: "Failed to get root view controller (getRootViewController())")src/types.ts (1)
3-3: Consider exporting constants to reduce typos and aid autocomplete.Providing named constants helps consumers avoid stringly-typed mistakes while keeping the union type.
Add the following near the type declarations:
export const NitroInAppBrowserPresentationStyles = { pageSheet: 'pageSheet', formSheet: 'formSheet', fullScreen: 'fullScreen', } as constConsumers can then use NitroInAppBrowserPresentationStyles.fullScreen while still benefitting from the union type.
ios/BrowserConfiguration.swift (1)
29-35: Optional: Remove the default case to keep the switch exhaustive.Guarding nil and omitting
defaultmakes the compiler warn if new cases are added upstream, improving forward-compatibility.Apply:
- private func configurePresentationStyle(for style: NitroInAppBrowserPresentationStyle?) -> UIModalPresentationStyle { - switch style { - case .formSheet: return .formSheet - case .pageSheet: return .pageSheet - case .fullScreen: return .fullScreen - default: return .automatic - } - } + private func configurePresentationStyle(for style: NitroInAppBrowserPresentationStyle?) -> UIModalPresentationStyle { + guard let style = style else { return .automatic } + switch style { + case .formSheet: return .formSheet + case .pageSheet: return .pageSheet + case .fullScreen: return .fullScreen + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (27)
bun.lockis excluded by!**/*.lockexample/ios/Podfile.lockis excluded by!**/*.locknitrogen/generated/android/NitroInAppBrowserOnLoad.cppis excluded by!**/generated/**nitrogen/generated/android/c++/JHybridNitroInAppBrowserSpec.cppis excluded by!**/generated/**nitrogen/generated/android/c++/JHybridNitroInAppBrowserSpec.hppis excluded by!**/generated/**nitrogen/generated/android/c++/JNitroInAppBrowserDismissButtonLabel.hppis excluded by!**/generated/**nitrogen/generated/android/c++/JNitroInAppBrowserOptions.hppis excluded by!**/generated/**nitrogen/generated/android/c++/JNitroInAppBrowserPresentationStyle.hppis excluded by!**/generated/**nitrogen/generated/android/kotlin/com/margelo/nitro/nitroinappbrowser/NitroInAppBrowserDismissButtonLabel.ktis excluded by!**/generated/**nitrogen/generated/android/kotlin/com/margelo/nitro/nitroinappbrowser/NitroInAppBrowserOptions.ktis excluded by!**/generated/**nitrogen/generated/android/kotlin/com/margelo/nitro/nitroinappbrowser/NitroInAppBrowserPresentationStyle.ktis excluded by!**/generated/**nitrogen/generated/ios/NitroInAppBrowser-Swift-Cxx-Bridge.cppis excluded by!**/generated/**nitrogen/generated/ios/NitroInAppBrowser-Swift-Cxx-Bridge.hppis excluded by!**/generated/**nitrogen/generated/ios/NitroInAppBrowser-Swift-Cxx-Umbrella.hppis excluded by!**/generated/**nitrogen/generated/ios/NitroInAppBrowserAutolinking.mmis excluded by!**/generated/**nitrogen/generated/ios/NitroInAppBrowserAutolinking.swiftis excluded by!**/generated/**nitrogen/generated/ios/c++/HybridNitroInAppBrowserSpecSwift.hppis excluded by!**/generated/**nitrogen/generated/ios/swift/Func_void.swiftis excluded by!**/generated/**nitrogen/generated/ios/swift/Func_void_std__exception_ptr.swiftis excluded by!**/generated/**nitrogen/generated/ios/swift/HybridNitroInAppBrowserSpec.swiftis excluded by!**/generated/**nitrogen/generated/ios/swift/HybridNitroInAppBrowserSpec_cxx.swiftis excluded by!**/generated/**nitrogen/generated/ios/swift/NitroInAppBrowserOptions.swiftis excluded by!**/generated/**nitrogen/generated/ios/swift/NitroInAppBrowserPresentationStyle.swiftis excluded by!**/generated/**nitrogen/generated/shared/c++/HybridNitroInAppBrowserSpec.hppis excluded by!**/generated/**nitrogen/generated/shared/c++/NitroInAppBrowserDismissButtonLabel.hppis excluded by!**/generated/**nitrogen/generated/shared/c++/NitroInAppBrowserOptions.hppis excluded by!**/generated/**nitrogen/generated/shared/c++/NitroInAppBrowserPresentationStyle.hppis excluded by!**/generated/**
📒 Files selected for processing (7)
README.md(1 hunks)example/App.tsx(1 hunks)example/package.json(1 hunks)ios/BrowserConfiguration.swift(1 hunks)ios/SafariBrowserPresenter.swift(1 hunks)package.json(1 hunks)src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
example/App.tsx (2)
src/specs/nitro-in-app-browser.nitro.ts (1)
NitroInAppBrowser(3-7)ios/HybridNitroInAppBrowser.swift (1)
HybridNitroInAppBrowser(11-40)
ios/SafariBrowserPresenter.swift (2)
nitrogen/generated/ios/NitroInAppBrowser-Swift-Cxx-Bridge.hpp (2)
error(89-91)error(89-89)ios/HybridNitroInAppBrowser.swift (1)
HybridNitroInAppBrowser(11-40)
src/types.ts (2)
src/specs/nitro-in-app-browser.nitro.ts (1)
NitroInAppBrowser(3-7)ios/HybridNitroInAppBrowser.swift (1)
HybridNitroInAppBrowser(11-40)
ios/BrowserConfiguration.swift (1)
ios/HybridNitroInAppBrowser.swift (2)
HybridNitroInAppBrowser(11-40)open(14-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build iOS Example App (old architecture)
- GitHub Check: Build iOS Example App (new architecture)
- GitHub Check: Build Android Example App (old architecture)
- GitHub Check: Build Android Example App (new architecture)
🔇 Additional comments (8)
README.md (1)
21-24: ✅ Update installation snippet to use a caret range (no stale references found)
Changed the pinned
react-native-nitro-modules@0.28.0to a caret range so users automatically receive patch updates:- bun install react-native-nitro-in-app-browser react-native-nitro-modules@0.28.0 + bun install react-native-nitro-in-app-browser react-native-nitro-modules@^0.28.0Verified README.md, package.json, and example/package.json—no lingering references to 0.26.x or 0.27.x.
example/package.json (1)
16-16: Version bump to ^0.28.0 looks good.Example app now aligns with the README and root devDependencies.
package.json (2)
67-67: react-native-nitro-modules devDependency bump is consistent across workspace.Matches example and README. No action needed.
63-63: Regenerate and commit updated Nitro bindingsPlease run the codegen script and ensure all generated sources reflect the v0.28.0 bump:
• Execute:
bun run codegen(this runs
nitro-codegen --logLevel="debug" && bun run build && node post-script.js)
• Review & stage all modified files under:
- nitrogen/generated/**
- ios/HybridNitroInAppBrowser.swift
- any shared/generated or android artifacts
• Commit the resulting diffs so the repo stays in sync with the new codegen version.src/types.ts (1)
3-9: Enum → string union migration looks correct.Types align with iOS mapping and example usage; good move for tree-shaking and DX.
example/App.tsx (1)
18-18: Migration to string-literal "fullScreen" looks correct.This aligns with the updated NitroInAppBrowserPresentationStyle union type and the iOS bridge mapping. No issues spotted.
ios/BrowserConfiguration.swift (2)
29-29: Good move to a typed enum for presentationStyle.Switching from String? to NitroInAppBrowserPresentationStyle? improves type-safety and prevents invalid values from slipping through.
31-33: Incorrect enum case names concern — resolved
The generatedNitroInAppBrowserPresentationStyleenum actually declares its cases as.formsheet,.pagesheet, and.fullscreen(see lines 36, 38, and 40 innitrogen/generated/ios/swift/NitroInAppBrowserPresentationStyle.swift), soBrowserConfiguration.swiftis already using the correct cases and no changes are required.Likely an incorrect or invalid review comment.
| export type NitroInAppBrowserPresentationStyle = 'pageSheet' | 'formSheet' | 'fullScreen' | ||
|
|
||
| export interface NitroInAppBrowserOptions { | ||
| dismissButtonLabel?: NitroInAppBrowserDismissButtonLabel | ||
| presentationStyle?: string // NitroInAppBrowserPresentationStyle | ||
| barColor?: string | ||
| controlColor?: string | ||
| dismissButtonLabel?: NitroInAppBrowserDismissButtonLabel | ||
| presentationStyle?: NitroInAppBrowserPresentationStyle | ||
| barColor?: string | ||
| controlColor?: string |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify there are no remaining references to the old enum members.
Ensure code/docs don’t use NitroInAppBrowserPresentationStyle.FullScreen, etc.
🏁 Script executed:
#!/bin/bash
# Find lingering enum-style references and non-literal usage
rg -n -C2 -P '\bNitroInAppBrowserPresentationStyle\.\w+\b|presentationStyle:\s*NitroInAppBrowserPresentationStyle\.' \
--ignore-file .gitignore
# Also list presentationStyle assignments for manual scan
rg -n -C2 -P '\bpresentationStyle\s*:\s*[^\'"]' --type=ts --type=tsxLength of output: 5619
Update lingering enum reference in README.md
A stray usage of the old enum member remains in the docs. Please replace it with the corresponding string literal.
• README.md (line 44)
- presentationStyle: NitroInAppBrowserPresentationStyle.FullScreen,
+ presentationStyle: 'fullScreen',No other .FullScreen/.pageSheet/.formSheet references were found in your TS/TSX files.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
README.md around line 44: the docs still reference the old enum member (e.g.,
.FullScreen) which no longer exists; update that stray reference to the matching
string literal from src/types.ts (use 'fullScreen' — casing exactly as defined)
so the README uses the current API; search surrounding lines for any other
.pageSheet/.formSheet/.FullScreen-style enum references and replace them with
'pageSheet'/'formSheet'/'fullScreen' respectively.
- Bump React and React Native versions to 19.1.0 and 0.81.0 respectively. - Update various development dependencies to their latest versions. - Modify iOS build workflow to include setup for the latest Xcode version. - Adjust Android build configuration to use updated SDK and build tools. - Refactor example app to utilize SafeAreaProvider and handle errors with alerts. - Remove deprecated properties from configuration files.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
example/android/gradle/wrapper/gradle-wrapper.properties (1)
3-3: Pin Gradle wrapper checksum & verify AGP/JDK compatibilityPlease complete the following before merging:
- Confirm your Android Gradle Plugin version (as declared in the
buildscriptclasspath inexample/android/build.gradle) is compatible with Gradle 8.14.3.- Ensure your JDK meets Gradle 8.14.3’s requirements (JDK 17 or later).
- Pin the Gradle distribution checksum by updating
example/android/gradle/wrapper/gradle-wrapper.properties:distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.3-bin.zip +distributionSha256Sum=bd71102213493060956ec229d946beee57158dbd89d0e62b91bca0fa2c5f3531Once AGP/JDK compatibility is confirmed and the checksum is in place, this bump is ready to merge.
example/android/gradlew (1)
117-117: Wrapper script updated to -jar invocation — LGTM; keep wrapper files generated, not hand-editedThe empty CLASSPATH + -jar pattern is the current Gradle wrapper behavior. Recommend continuing to regenerate via
gradle wrapperto avoid drift and ensure wrapper JAR/properties stay in sync across updates.Also applies to: 216-217
example/android/gradle.properties (1)
41-44: edgeToEdgeEnabled is declarative only here — verify it’s actually consumedThis Gradle property won’t have any effect unless your Activity opts in. If you intend to support edge-to-edge on the example app, wire it up (ReactActivity + WindowCompat).
To implement, in your MainActivity:
import androidx.core.view.WindowCompat class MainActivity : ReactActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) if (BuildConfig.EDGE_TO_EDGE_ENABLED) { WindowCompat.setDecorFitsSystemWindows(window, false) } } }And surface the Gradle property to BuildConfig (in example/android/app/build.gradle):
- buildConfigField("boolean", "EDGE_TO_EDGE_ENABLED", project.findProperty("edgeToEdgeEnabled")?.toString()?.toBoolean()?.toString() ?: "false")
Or alternatively read the Gradle property at runtime and branch accordingly.If you already handle this elsewhere, ignore.
.github/workflows/ios-build.yml (1)
52-56: Pin Xcode instead of latest-stable to reduce CI flakinesslatest-stable can lead to surprise simulator/name changes (e.g., device identifiers), breaking builds unexpectedly. Prefer a pinned major.minor and bump deliberately.
Apply:
- - name: Setup Xcode - uses: maxim-lobanov/setup-xcode@v1 - with: - xcode-version: latest-stable + - name: Setup Xcode + uses: maxim-lobanov/setup-xcode@v1 + with: + # Pin to a known-good version; update intentionally as needed + xcode-version: "16.2"If you want flexibility, consider a matrix or a repository env var to centralize the version.
example/android/gradlew.bat (1)
73-77: Drop redundant classpath when using -jarWhen invoking Java with -jar, -classpath is ignored. Keeping an empty CLASSPATH and passing -classpath is unnecessary and slightly noisy.
Apply this diff:
-@rem Setup the command line - -set CLASSPATH= - -@rem Execute Gradle -"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* +@rem Execute Gradle +"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %*example/App.tsx (2)
19-25: Defensively normalize error messages before AlertCasting to Error can yield undefined messages if a non-Error is thrown. Normalize to string to avoid empty alerts.
Apply this diff:
- } catch (error) { - Alert.alert('Error', (error as Error).message, [ + } catch (error) { + const message = + error instanceof Error ? error.message : String(error) + Alert.alert('Error', message, [
14-14: Prefer hex colors for cross-platform consistencyNamed colors aren’t uniformly supported across platforms. Hex avoids platform parsing differences.
Apply this diff:
- barColor: 'purple', + barColor: '#800080',package.json (2)
56-70: Consider updating TypeScript to align with React 19/RN 0.81 toolchainsTS 5.0.4 may be fine, but React 19 and RN 0.81 ecosystems commonly target newer 5.x. Bumping reduces type-compat friction.
Example change:
- "typescript": "5.0.4" + "typescript": "5.4.5"Please pick the version your CI/tooling supports.
71-75: Tighten peerDependency ranges for clearer compatibility guaranteesUsing "*" makes consumers resolve potentially incompatible versions. Consider semver ranges aligned with what you test against.
Example:
- "peerDependencies": { - "react": "*", - "react-native": "*", - "react-native-nitro-modules": "*" - }, + "peerDependencies": { + "react": ">=18.2 <21", + "react-native": ">=0.72 <1", + "react-native-nitro-modules": ">=0.28 <1" + },Adjust as appropriate for your supported matrix.
example/package.json (1)
7-8: Avoid hard-coding a specific Simulator model‘iPhone 16’ may not exist on all machines. Prefer omitting the simulator flag or making it configurable.
Example:
- "ios": "react-native run-ios --simulator='iPhone 16'", + "ios": "react-native run-ios",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockexample/android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarexample/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.github/workflows/ios-build.yml(3 hunks)example/.prettierrc.js(0 hunks)example/App.tsx(2 hunks)example/android/app/src/main/java/com/nitroinappbrowserexample/MainApplication.kt(2 hunks)example/android/build.gradle(1 hunks)example/android/gradle.properties(1 hunks)example/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)example/android/gradlew(2 hunks)example/android/gradlew.bat(1 hunks)example/ios/NitroInAppBrowserExample/Info.plist(1 hunks)example/package.json(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- example/.prettierrc.js
🧰 Additional context used
🧬 Code Graph Analysis (3)
example/ios/NitroInAppBrowserExample/Info.plist (1)
example/ios/NitroInAppBrowserExample/AppDelegate.swift (3)
AppDelegate(6-34)ReactNativeDelegate(36-48)application(13-33)
example/android/app/src/main/java/com/nitroinappbrowserexample/MainApplication.kt (1)
example/ios/NitroInAppBrowserExample/AppDelegate.swift (2)
application(13-33)AppDelegate(6-34)
example/App.tsx (3)
ios/HybridNitroInAppBrowser.swift (1)
HybridNitroInAppBrowser(11-40)src/specs/nitro-in-app-browser.nitro.ts (1)
NitroInAppBrowser(3-7)src/types.ts (1)
NitroInAppBrowserOptions(9-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build iOS Example App (new architecture)
- GitHub Check: Build Android Example App (new architecture)
- GitHub Check: Build Android Example App (old architecture)
🔇 Additional comments (10)
example/ios/NitroInAppBrowserExample/Info.plist (1)
36-37: LGTM: Enabling the New Architecture (RCTNewArchEnabled) for the example appThis aligns with the ReactNativeFactory-based setup seen in AppDelegate.swift. The placement and value look correct.
example/android/app/src/main/java/com/nitroinappbrowserexample/MainApplication.kt (1)
9-9: Modern RN entrypoint (loadReactNative) adoption looks goodSwitching to ReactNativeApplicationEntryPoint.loadReactNative and using getDefaultReactHost is aligned with current RN templates; no SoLoader/init boilerplate needed.
Also applies to: 38-39
.github/workflows/ios-build.yml (2)
41-43: Concurrency block reformat — LGTMNo semantic change; keeps the workflow’s run deduplication behavior intact.
141-141: No-op formatting change — OK to keepEnd-of-line/formatting normalization; no functional impact.
example/android/build.gradle (2)
3-9: API 36/Kotlin 2.1.20 bump looks aligned with RN 0.81The SDK/Tools bumps are consistent with the rest of the PR and modern RN requirements.
3-9: Confirm AGP/Kotlin/JDK/SDK compatibility on CII’ve checked the project setup and found:
- Gradle wrapper is pinned to 8.14.3 (
distributionUrl).ext.kotlinVersionis set to 2.1.20 and the Kotlin Gradle plugin is applied.- No explicit Java toolchain is configured; default
sourceCompatibility/targetCompatibilityremain at Java 1.8.- No references to JDK 17+ enforcement in any Gradle file.
Please verify the following on your CI environment:
- The Android Gradle Plugin version you’re using supports Kotlin 2.1.20.
- CI is running with JDK 17 or newer (otherwise the Kotlin plugin may fail).
- Android SDK Platform 36 and Build-Tools 36.0.0 are installed and available to Gradle.
example/App.tsx (2)
6-39: Nice switch to string-literal presentationStyle + Safe Area adoptionDefault import usage and SafeAreaProvider/SafeAreaView integration look good and align with the new types and example dependency updates.
3-4: Default export and presentationStyle type verifiedBoth checks passed:
src/index.tsexportsNitroInAppBrowseras the default.NitroInAppBrowserPresentationStyleis defined as the expected string-literal union.No changes needed.
package.json (1)
14-16: Scripts depend on Bun; make it explicit or provide a Node fallbackBuild and codegen use bun. Either:
- declare the package manager to ensure CI/devs have bun, or
- make scripts PM-agnostic (e.g., “npm run build”).
If you prefer bun, consider adding to package.json (adjust version to what CI uses):
"packageManager": "bun@1.1.x"Alternatively, replace:
- "bun run typecheck && bob build" with "npm run typecheck && bob build"
- In "codegen", "bun run build" with "npm run build"
example/package.json (1)
14-18: Example deps bump looks goodreact/react-native/nitro-modules upgrades and safe-area-context addition align with the App changes.
| <key>RCTNewArchEnabled</key> | ||
| <true/> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Podfile/xcconfig alignment with this runtime flag
Ensure your build settings also enable the New Architecture (e.g., Podfile new_arch_enabled, relevant xcconfig macros). A mismatch between runtime flag and build-time settings can lead to unexpected behavior.
Run this script to check for consistency:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for the Info.plist flag:"
rg -n -C1 -S 'RCTNewArchEnabled' || true
echo ""
echo "Checking Podfile(s) for New Architecture settings:"
fd -a 'Podfile' | while read -r pod; do
echo "---- $pod ----"
rg -n -C2 -P 'use_react_native|new_arch_enabled|fabric_enabled|hermes_enabled' "$pod" || true
done
echo ""
echo "Scanning xcconfig files for related macros:"
fd -a '.xcconfig' | xargs -r rg -n -C1 -P 'RCT_NEW_ARCH_ENABLED|USE_HERMES|HERMES' || true
echo ""
echo "Swift usage of ReactNativeFactory (context only):"
rg -n -C3 -S 'ReactNativeDelegate|RCTReactNativeFactory|newArch' --type=swift || trueLength of output: 3517
Enable New Architecture in build configuration
You’ve set <key>RCTNewArchEnabled</key><true/> in Info.plist, but your Podfile and build‐time macros aren’t aligned. A mismatch can lead to runtime failures when the new architecture is instantiated. Please update as follows:
- In
example/ios/Podfile, modify theuse_react_native!call to include
:new_arch_enabled => true(and:fabric_enabled => trueif you’re using Fabric). - No
.xcconfigin your project definesRCT_NEW_ARCH_ENABLED. If you use Xcode config files (e.g.Debug.xcconfig/Release.xcconfig), add
RCT_NEW_ARCH_ENABLED = 1
Otherwise, setRCT_NEW_ARCH_ENABLEDas a User‐Defined Setting in your Xcode target.
Aligning these ensures your runtime flag and build‐time settings match.
🤖 Prompt for AI Agents
In example/ios/NitroInAppBrowserExample/Info.plist around lines 36-37 the
RCTNewArchEnabled flag is set to true but your build settings aren’t aligned;
update example/ios/Podfile to call use_react_native! with :new_arch_enabled =>
true (and add :fabric_enabled => true if using Fabric), and ensure
RCT_NEW_ARCH_ENABLED is defined at build time by adding RCT_NEW_ARCH_ENABLED = 1
to your Xcode config files (Debug.xcconfig/Release.xcconfig) or by adding
RCT_NEW_ARCH_ENABLED as a User-Defined Setting on the Xcode target so runtime
and build-time flags match.
| "scripts": { | ||
| "typecheck": "tsc --noEmit", | ||
| "clean": "git clean -dfX", | ||
| "release": "semantic-release", | ||
| "build": "bun run typecheck && bob build", | ||
| "codegen": "nitro-codegen --logLevel=\"debug\" && bun run build && node post-script.js", | ||
| "postcodegen": "bun --cwd example pod" | ||
| }, |
There was a problem hiding this comment.
postcodegen script is broken (incorrect bun usage)
bun requires “run” to execute a package script; “bun --cwd example pod” will attempt to run a file named “pod”, not the example’s script.
Apply one of these fixes (pick one):
Option A: Use npm to avoid bun-specific flags:
- "postcodegen": "bun --cwd example pod"
+ "postcodegen": "npm run --prefix example pod"Option B: Keep bun, but use the correct form:
- "postcodegen": "bun --cwd example pod"
+ "postcodegen": "bun --cwd example run pod"📝 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.
| "scripts": { | |
| "typecheck": "tsc --noEmit", | |
| "clean": "git clean -dfX", | |
| "release": "semantic-release", | |
| "build": "bun run typecheck && bob build", | |
| "codegen": "nitro-codegen --logLevel=\"debug\" && bun run build && node post-script.js", | |
| "postcodegen": "bun --cwd example pod" | |
| }, | |
| "scripts": { | |
| "typecheck": "tsc --noEmit", | |
| "clean": "git clean -dfX", | |
| "release": "semantic-release", | |
| "build": "bun run typecheck && bob build", | |
| "codegen": "nitro-codegen --logLevel=\"debug\" && bun run build && node post-script.js", | |
| "postcodegen": "bun --cwd example run pod" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 10 to 17, the "postcodegen" script uses incorrect
bun invocation ("bun --cwd example pod") which makes bun try to execute a file
named "pod"; change it to use bun's run form by invoking the script in the
example package: update the script to call bun with the cwd flag and the run
subcommand (e.g., bun --cwd example run pod) so the example package's "pod"
script is executed correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ios-build.yml (1)
52-56: Ensure simulator availability matches Xcode 16.4; consider adding a quick diagnostics step.Good call explicitly selecting Xcode 16.4. The Build App step targets destination name "iPhone 16"; ensure this device type is available on the macOS-15 image for the selected Xcode. If it's missing, xcodebuild will fail early.
Optional hardening:
- Add a pre-build diagnostics step to list available simulators to aid debugging.
- If flakiness occurs, consider switching to a more generic destination constraint (e.g., OS=latest) or a widely-available device name on the chosen image.
Example diagnostic step you can insert after Setup Xcode:
- name: List available simulators run: | xcodebuild -version xcrun simctl list devices availableAlso, consider pinning the action to a specific commit SHA for supply-chain safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ios-build.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Android Example App (old architecture)
- GitHub Check: Build Android Example App (new architecture)
- GitHub Check: Build iOS Example App (new architecture)
🔇 Additional comments (2)
.github/workflows/ios-build.yml (2)
41-42: Concurrency keying LGTM; will prevent duplicate runs per ref.Using group "${{ github.workflow }}-${{ github.ref }}" with cancel-in-progress: true is a solid choice to cancel stale runs per branch/PR.
141-141: No-op formatting change is fine.Trailing newline/format normalization is harmless.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/ios-build.yml (2)
52-56: Pin Xcode to a concrete version to avoid CI flakinessUsing latest-stable can break builds when Apple releases a new Xcode. Pin to a tested major/minor and update deliberately.
Apply this diff to pin Xcode:
- - name: Setup Xcode - uses: maxim-lobanov/setup-xcode@v1 - with: - xcode-version: latest-stable + - name: Setup Xcode + uses: maxim-lobanov/setup-xcode@v1 + with: + xcode-version: '16.4' # or your validated versionAdditionally, consider making the Simulator destination resilient across Xcode updates by specifying OS=latest (outside this hunk):
- -destination 'platform=iOS Simulator,name=iPhone 16' \ + -destination 'platform=iOS Simulator,OS=latest,name=iPhone 16' \
103-107: Repeat: Pin Xcode version in old-arch job for consistencySame concern as the new-arch job—pin Xcode here too.
- - name: Setup Xcode - uses: maxim-lobanov/setup-xcode@v1 - with: - xcode-version: latest-stable + - name: Setup Xcode + uses: maxim-lobanov/setup-xcode@v1 + with: + xcode-version: '16.4'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ios-build.yml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build iOS Example App (old architecture)
- GitHub Check: Build iOS Example App (new architecture)
- GitHub Check: Build Android Example App (old architecture)
- GitHub Check: Build Android Example App (new architecture)
🔇 Additional comments (2)
.github/workflows/ios-build.yml (2)
41-43: Concurrency settings look goodGroup and cancel-in-progress are correctly configured to avoid duplicate runs per ref.
146-146: No-op change acknowledgedTrailing newline/formatting only; no behavioral impact on the build step.
| - name: Setup Xcode | ||
| uses: maxim-lobanov/setup-xcode@v1 | ||
| with: | ||
| xcode-version: latest-stable | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bundler cache misconfigured: working-directory is ignored by ruby/setup-ruby
The working-directory key in a uses: step isn’t a valid input for ruby/setup-ruby and will be ignored. As a result, bundler-cache runs at repo root and may miss example/ios/Gemfile, leading to non-deterministic Cocoapods usage.
Recommended fix (outside this hunk): point Bundler to the example Gemfile via BUNDLE_GEMFILE and use bundle exec for pod install to honor the locked versions.
- name: Setup Ruby (bundle)
uses: ruby/setup-ruby@v1
with:
ruby-version: 2.7.2
bundler-cache: true
env:
BUNDLE_GEMFILE: example/ios/Gemfile
- name: Install Pods
working-directory: example/ios
run: bundle exec pod installOptional: consider upgrading Ruby to a supported version (e.g., 3.2+) if your Gemfile allows it, as 2.7 is EOL.
🤖 Prompt for AI Agents
.github/workflows/ios-build.yml around lines 52-56: the workflow currently uses
ruby/setup-ruby with a working-directory input (which is ignored) causing the
bundler-cache to run at repo root and miss example/ios/Gemfile; update the
workflow by adding a ruby/setup-ruby step that enables bundler-cache and sets
the environment variable BUNDLE_GEMFILE to point to example/ios/Gemfile (do not
rely on working-directory for that action), and change the pod install step to
run inside example/ios using "bundle exec pod install" so Bundler honors the
locked Gemfile; optionally consider updating the ruby-version to a supported
release if your Gemfile allows it.
| - name: Setup Xcode | ||
| uses: maxim-lobanov/setup-xcode@v1 | ||
| with: | ||
| xcode-version: latest-stable | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Repeat: Fix bundler-cache scoping to example/ios
As above, the working-directory input is ignored in uses:. Use BUNDLE_GEMFILE and bundle exec to ensure the correct Cocoapods version is used under the example app’s Gemfile.
- name: Setup Ruby (bundle)
uses: ruby/setup-ruby@v1
with:
ruby-version: 2.7.2
bundler-cache: true
env:
BUNDLE_GEMFILE: example/ios/Gemfile
- name: Install Pods
working-directory: example/ios
run: bundle exec pod install🤖 Prompt for AI Agents
In .github/workflows/ios-build.yml around lines 103-107, the bundler-cache
scoping is incorrect because working-directory is ignored for uses:, so set
BUNDLE_GEMFILE to point at example/ios/Gemfile on the ruby/setup-ruby step (keep
bundler-cache: true) and then run pod install via bundle exec in the example/ios
directory; specifically, add env: BUNDLE_GEMFILE: example/ios/Gemfile to the
ruby/setup-ruby step and replace the direct pod install with a step that runs
bundle exec pod install with working-directory: example/ios so the correct
Cocoapods version from the example app’s Gemfile is used.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Summary by CodeRabbit
Refactor
Bug Fixes
Documentation
Chores