Conversation
Add a husky pre-commit hook and lint-staged config; update package scripts to run prepare/husky and run lint+codegen before publish. Switch babel config to react-native-builder-bob preset and add babel-plugin-react-compiler. Remove scripts/remove-source-maps.cjs and stop invoking it from the build pipeline. Refactor runtime and API internals: lazy-initialize the Nitro hybrid object with docs, improve JSDoc across specs/types, simplify useInAppBrowser by centralizing loading/error tracking (remove useCallback/useMemo churn), and significantly rewrite normalizeOptions to use per-field sanitizers with fast-paths to avoid unnecessary allocations. Minor tsconfig cleanup and dependency/devDependency updates.
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s tooling/CI setup (moving from Bun to Yarn 4), refreshes build/release workflows, and improves the library’s developer-facing API docs while refactoring a few runtime utilities.
Changes:
- Migrates CI workflows and project scripts from Bun to Yarn 4 + Corepack; updates caching and toolchain versions.
- Adds Husky + lint-staged (Biome) for pre-commit enforcement; removes the source-map stripping script and disables source maps in bob targets.
- Enhances TypeScript docs and refactors option sanitization and the
useInAppBrowserhook implementation.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lockfile for Yarn 4 dependency graph (adds husky/lint-staged/react-compiler plugin, removes Bun-related deps). |
| tsconfig.json | Removes excludes for now-irrelevant config files. |
| src/utils/options.ts | Refactors option sanitization with “fast path” + sanitizer map changes. |
| src/types.ts | Expands JSDoc, platform annotations, and clarifies API types (incl. type alias for auth result). |
| src/specs/inappbrowser-nitro.nitro.ts | Improves spec documentation for Nitrogen bridge interface. |
| src/hooks/useInAppBrowser.ts | Simplifies hook logic and updates documentation around memoization/React Compiler. |
| src/core/native.ts | Adds richer JSDoc and examples for public imperative APIs. |
| scripts/remove-source-maps.cjs | Removes the source-map deletion script. |
| package.json | Switches scripts to Yarn-centric flow, adds husky/lint-staged/react-compiler plugin, updates bob build options. |
| example/package.json | Removes unused ESLint config dependency from the example app. |
| babel.config.js | Switches to bob Babel preset and adds React Compiler plugin. |
| .husky/pre-commit | Adds a pre-commit hook to run lint-staged. |
| .github/workflows/release.yml | Migrates release workflow from Bun to Yarn 4/Corepack and updates actions versions. |
| .github/workflows/ios-build.yml | Migrates iOS workflow from Bun to Yarn 4/Corepack and updates toolchain versions. |
| .github/workflows/android-build.yml | Migrates Android workflow from Bun to Yarn 4/Corepack and updates toolchain versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const key of Object.keys(source) as (keyof T)[]) { | ||
| const v = source[key] | ||
| if (v !== undefined) originalKeyCount++ | ||
| } | ||
|
|
||
| for (const key of keys) { | ||
| const value = source[key] | ||
| if (typeof value !== 'string') { | ||
| if (value !== undefined) mutated = true | ||
| continue | ||
| } | ||
| const trimmed = value.trim() | ||
| if (!trimmed) { | ||
| mutated = true | ||
| continue | ||
| } | ||
| if (trimmed !== value) mutated = true | ||
| if (out === null) out = {} | ||
| out[key] = trimmed as T[keyof T] | ||
| kept++ | ||
| } | ||
|
|
||
| if (!mutated && kept === originalKeyCount) return null | ||
| return out ? (compact(out) as T | undefined) : undefined | ||
| } |
There was a problem hiding this comment.
trimStringFields can return null ("no mutation") even when the input contains explicit undefined values for whitelisted keys (e.g. { light: undefined }). In that case, normalizeOptions will take the fast path and return the original options, leaving undefined entries (and potentially other non-whitelisted enumerable keys) in nested payloads that are supposed to be sanitized/compacted before crossing JSI.
To make the null fast-path safe, treat an own property with value undefined (or any non-string own property) as a mutation, and/or verify there are no extra enumerable keys beyond the whitelist before returning null.
| // Fast first pass: detect whether anything needs to change. | ||
| let dirty = false | ||
| const keys = Object.keys(options) | ||
|
|
||
| for (const key of keys) { | ||
| const typedKey = key as keyof InAppBrowserOptions & string | ||
| const value = options[typedKey] | ||
| if (value === undefined || value === null) { | ||
| dirty = true | ||
| break | ||
| } | ||
| const sanitizer = SANITIZERS[typedKey] as Sanitizer<unknown> | undefined | ||
| if (sanitizer && sanitizer(value) !== null) { | ||
| dirty = true | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
normalizeOptions calls each sanitizer in the "fast first pass" (sanitizer(value) !== null) to decide whether the options are dirty. With the current trimStringFields semantics, this can incorrectly conclude an options object is clean and return it unchanged (e.g. nested DynamicColor/BrowserAnimations objects containing explicit undefined values). That means the function can skip the intended sanitization entirely.
Consider replacing the sanitizer call in the dirty-check with a dedicated cheap predicate (e.g. needsSanitization(value)), or ensure sanitizers never return null unless the original value is guaranteed bridge-safe.
| @@ -63,54 +77,38 @@ export const useInAppBrowser = (): UseInAppBrowserReturn => { | |||
| } | |||
| }, []) | |||
|
|
|||
| const open = useCallback( | |||
| async (url: string, options?: InAppBrowserOptions) => { | |||
| if (isMountedRef.current) { | |||
| setIsLoading(true) | |||
| setError(null) | |||
| } | |||
| try { | |||
| return await nativeOpen(url, options) | |||
| } catch (err) { | |||
| const next = toError(err) | |||
| if (isMountedRef.current) setError(next) | |||
| throw next | |||
| } finally { | |||
| if (isMountedRef.current) setIsLoading(false) | |||
| } | |||
| }, | |||
| [] | |||
| ) | |||
| const runTracked = async <T>(fn: () => Promise<T>): Promise<T> => { | |||
| if (isMountedRef.current) { | |||
| setIsLoading(true) | |||
| setError(null) | |||
| } | |||
| try { | |||
| return await fn() | |||
| } catch (err) { | |||
| const next = toError(err) | |||
| if (isMountedRef.current) setError(next) | |||
| throw next | |||
| } finally { | |||
| if (isMountedRef.current) setIsLoading(false) | |||
| } | |||
| } | |||
|
|
|||
| const open = (url: string, options?: InAppBrowserOptions) => | |||
| runTracked(() => nativeOpen(url, options)) | |||
|
|
|||
| const openAuth = useCallback( | |||
| async (url: string, redirectUrl: string, options?: InAppBrowserOptions) => { | |||
| if (isMountedRef.current) { | |||
| setIsLoading(true) | |||
| setError(null) | |||
| } | |||
| try { | |||
| return await nativeOpenAuth(url, redirectUrl, options) | |||
| } catch (err) { | |||
| const next = toError(err) | |||
| if (isMountedRef.current) setError(next) | |||
| throw next | |||
| } finally { | |||
| if (isMountedRef.current) setIsLoading(false) | |||
| } | |||
| }, | |||
| [] | |||
| ) | |||
| const openAuth = ( | |||
| url: string, | |||
| redirectUrl: string, | |||
| options?: InAppBrowserOptions | |||
| ) => runTracked(() => nativeOpenAuth(url, redirectUrl, options)) | |||
|
|
|||
| return useMemo<UseInAppBrowserReturn>( | |||
| () => ({ | |||
| open, | |||
| openAuth, | |||
| close: nativeClose, | |||
| closeAuth: nativeCloseAuth, | |||
| isAvailable: nativeIsAvailable, | |||
| isLoading, | |||
| error, | |||
| }), | |||
| [open, openAuth, isLoading, error] | |||
| ) | |||
| return { | |||
| open, | |||
| openAuth, | |||
| close: nativeClose, | |||
| closeAuth: nativeCloseAuth, | |||
| isAvailable: nativeIsAvailable, | |||
| isLoading, | |||
| error, | |||
| } | |||
There was a problem hiding this comment.
useInAppBrowser now returns fresh open/openAuth function instances every render (and a new object literal), after removing useCallback/useMemo. For React Native consumers, the package exports this file under the react-native condition (./src/hooks/useInAppBrowser.ts), so it will be compiled by the app’s Babel config (typically without babel-plugin-react-compiler). In that common case, memoization will not be applied, and these unstable references can cause unnecessary re-renders and make effect dependency arrays behave differently.
Recommendation: keep useCallback for open/openAuth (and useMemo for the return object if desired), or ensure the react-native entrypoint points at a compiler-transformed build artifact and document/enforce the React Compiler requirement.
| @@ -0,0 +1 @@ | |||
| yarn lint-staged | |||
There was a problem hiding this comment.
This hook script is missing a shebang. Git executes hooks as standalone executables; without a #!/usr/bin/env sh (or similar) header, this will typically fail with an “exec format error” on POSIX systems.
Add the appropriate shebang (and, if required by Husky, the standard husky.sh bootstrap) and ensure the file is marked executable in git.
Upgrade react-native-nitro-modules / Nitrogen to 0.35.6 across package manifests and native Podfile, and update related dev deps (@biomejs/biome -> 2.4.13, lint-staged -> 16.4.0). Change InAppBrowserAuthResult from a type alias to an interface extending InAppBrowserResult so Nitrogen emits a dedicated Swift/Kotlin type and IDE tooltips show the distinct name. Updated lockfiles (yarn.lock and Podfile.lock) to reflect these dependency changes.
useInAppBrowser: add useCallback/useMemo to memoize runTracked, open, openAuth and the returned object, ensuring stable identities for consumers (e.g. useEffect deps or React.memo). Also update import list and docs to clarify memoization behavior.\n\nutils/options: enhance trimStringFields to treat explicit undefined, non-string own properties, and extra enumerable keys as mutations (uses ownKeyCount and `key in source`), returning null only when the input is already bridge-safe; improves sanitization before crossing the bridge.
Rewrite README with a full docs overhaul: updated badges, quick-start, installation (yarn/npm/pnpm/bun), requirements (New Architecture / no Expo Go), API and options tables, migration notes from react-native-inappbrowser-reborn, FAQ, platform notes, examples, contributing instructions, and CI/dev workflow. Also adjust examples to use the hook import path and demonstrate open/openAuth usage. Add clarifying JSDoc to InAppBrowser iOS options in src/types.ts describing iOS 26 "Liquid Glass" behavior for preferredBarTintColor and preferredControlTintColor and noting backward compatibility.
Make the .husky/pre-commit script executable and add a POSIX shebang so the hook runs reliably. In src/utils/options.ts, add a clarifying comment explaining that sanitizers return null when the input is already bridge-safe (e.g. trimStringFields), and that using `sanitizer(value) !== null` is a valid dirty-check without a separate traversal. No functional logic changes.
This pull request modernizes the build and release workflows, improves developer tooling, and enhances documentation and code quality. The key changes include migrating from Bun to Yarn 4, updating GitHub Actions workflows for both Android and iOS, adding linting and pre-commit hooks, and improving TypeScript API documentation and code comments. Several dependencies and configurations have also been updated for better maintainability and compatibility.
Build and Release Workflow Modernization:
.github/workflows/android-build.yml,.github/workflows/ios-build.yml, and.github/workflows/release.yml. This includes enabling Corepack, updating Node.js to v24, and caching Yarn dependencies. [1] [2] [3] [4] [5]Developer Tooling and Linting Improvements:
package.jsonand a new.husky/pre-commitscript. [1] [2] [3]remove-source-maps.cjsscript, as source map generation is disabled via Babel config. [1] [2] [3]Babel and Project Configuration Updates:
react-native-builder-bob/babel-presetand addedbabel-plugin-react-compilerfor improved compatibility and future-proofing. [1] [2]API Documentation and Code Quality Enhancements:
src/core/native.ts,src/hooks/useInAppBrowser.ts, andsrc/specs/inappbrowser-nitro.nitro.ts. [1] [2] [3] [4] [5] [6]useInAppBrowserhook, removing unnecessary React hooks and improving memoization and error handling. [1] [2] [3]Dependency and Config Cleanup:
These changes collectively improve the developer experience, streamline CI/CD, and enhance code maintainability and usability.