feat: add Expo config plugin for SDK 56+#22
Conversation
Add a first-party Expo config plugin so consumers can configure react-native-nitro-maps via app.json / app.config.js without manual native edits. Closes #12. - Android mod: inject com.google.android.geo.API_KEY meta-data and location permissions (fine/coarse, background) - iOS mod: set NSLocationWhenInUseUsageDescription / NSLocationAlwaysAndWhenInUseUsageDescription; stub Google Maps key (#2) - Export NitroMapsPluginOptions types; use createRunOncePlugin - Wire app.plugin.js, build:plugin script, jest + ts-jest config - Migrate example app to the plugin (remove manual Google Maps block) - Document setup in README and docs/expo-setup.md
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an Expo config plugin for Nitro Maps, with Android and iOS config mods, package build/test wiring, example app updates, and setup documentation. ChangesExpo config plugin
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/expo-setup.md`:
- Around line 56-58: The `.env` example uses a naked fenced code block, which
triggers markdownlint MD040. Update the fenced block in the Expo setup doc to
include an explicit language hint such as dotenv or text so the example is
lint-clean and consistently highlighted; this is the fenced block containing
GOOGLE_MAPS_API_KEY in the markdown snippet.
In `@package/package.json`:
- Around line 32-34: The build:plugin step is leaving stale files in
plugin/build because tsc does not clean old artifacts before compiling. Update
the package.json scripts so the build:plugin workflow explicitly removes the
existing plugin/build output before running tsc, and make sure prepack still
goes through the cleaned build path. Use the build, build:plugin, and prepack
script entries as the place to apply the fix.
- Around line 34-40: The Expo plugin entry point currently points to a built
artifact that may not exist in a fresh checkout. Update the plugin entry wiring
in package.json and the app.plugin.js entry so Expo resolves the source plugin
directly, or ensure the plugin build is generated before Expo tries to load it.
Use the app.plugin.js and plugin/build/index symbols to locate the startup path
and remove the dependency on a missing prebuilt output.
In `@package/plugin/src/android.ts`:
- Around line 40-49: The Android permission builder in the location permissions
logic is allowing ACCESS_BACKGROUND_LOCATION to be added without any foreground
location permissions, which creates an invalid standalone always-location
config. Update the permission handling in the android.ts flow around
wantsWhenInUseLocation and wantsAlwaysLocation so that locationAlwaysPermission
either also includes the fine/coarse permissions or is rejected unless
foreground location is enabled. Keep the fix localized to the permissions.push
logic used by these helpers.
In `@package/plugin/src/ios.ts`:
- Around line 15-22: The iOS Info.plist setup in the plugin currently writes
only the always-location key when `wantsAlwaysLocation(options)` is true, which
leaves the when-in-use usage string missing for the always flow. Update the
logic in the `wantsAlwaysLocation` branch to ensure
`NSLocationWhenInUseUsageDescription` is always populated as the base
requirement, either by requiring `locationPermission` whenever
`locationAlwaysPermission` is set or by backfilling it from
`locationAlwaysPermission` when absent. Keep the change localized to the
`infoPlist` assignment block in `ios.ts`.
In `@README.md`:
- Around line 52-54: The README section has two separate blockquote paragraphs
separated by a blank line, which triggers markdownlint
MD028/no-blanks-blockquote. Update the existing blockquote content in README.md
so the Google Maps API key and EAS Secrets notes are either merged into a single
contiguous blockquote or the blank line between them is removed; use the
surrounding README blockquote text as the locator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e0d603a-b89f-4a60-a4ac-960f52033f14
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/ci.yml.gitignoreREADME.mddocs/expo-setup.mdeslint.config.mjsexample/android/app/src/main/AndroidManifest.xmlexample/app.config.jsexample/ios/NitroMapsExample/Info.plistpackage/android/README.mdpackage/app.plugin.jspackage/jest.config.jspackage/package.jsonpackage/plugin/src/android.test.tspackage/plugin/src/android.tspackage/plugin/src/index.tspackage/plugin/src/ios.test.tspackage/plugin/src/ios.tspackage/plugin/src/types.tspackage/plugin/src/withNitroMaps.tspackage/tsconfig.plugin.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package/plugin/src/android.test.ts (1)
27-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the omitted-key regression test.
This only proves the happy path. The plugin’s contract also says
googleMapsApiKeycan be omitted without breaking prebuild, so the falsy branch inapplyGoogleMapsApiKeyneeds an explicit test before someone breaks it later.Proposed test
describe('applyGoogleMapsApiKey', () => { it('injects com.google.android.geo.API_KEY meta-data into AndroidManifest', () => { const manifest = createEmptyManifest(); applyGoogleMapsApiKey(manifest, 'test-api-key'); expect( AndroidConfig.Manifest.getMainApplicationOrThrow(manifest)['meta-data'], ).toEqual([ { $: { 'android:name': GOOGLE_MAPS_API_KEY_META, 'android:value': 'test-api-key', }, }, ]); }); + + it('does nothing when the API key is omitted', () => { + const manifest = createEmptyManifest(); + + expect(applyGoogleMapsApiKey(manifest, undefined)).toBe(manifest); + expect( + AndroidConfig.Manifest.getMainApplicationOrThrow(manifest)['meta-data'], + ).toBeUndefined(); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package/plugin/src/android.test.ts` around lines 27 - 40, Add a regression test around applyGoogleMapsApiKey in android.test.ts for the omitted googleMapsApiKey case, since the current test only covers the happy path. Create a manifest with createEmptyManifest(), call applyGoogleMapsApiKey with an undefined or otherwise falsy key, and assert that AndroidConfig.Manifest.getMainApplicationOrThrow(manifest)['meta-data'] is not added or remains empty. Keep the new test alongside the existing injects com.google.android.geo.API_KEY meta-data into AndroidManifest case so the falsy branch is explicitly covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package/plugin/src/android.test.ts`:
- Around line 27-40: Add a regression test around applyGoogleMapsApiKey in
android.test.ts for the omitted googleMapsApiKey case, since the current test
only covers the happy path. Create a manifest with createEmptyManifest(), call
applyGoogleMapsApiKey with an undefined or otherwise falsy key, and assert that
AndroidConfig.Manifest.getMainApplicationOrThrow(manifest)['meta-data'] is not
added or remains empty. Keep the new test alongside the existing injects
com.google.android.geo.API_KEY meta-data into AndroidManifest case so the falsy
branch is explicitly covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b265329-291e-4954-8cec-20aa1cd38ac0
📒 Files selected for processing (8)
README.mddocs/expo-setup.mdpackage/package.jsonpackage/plugin/src/android.test.tspackage/plugin/src/android.tspackage/plugin/src/ios.test.tspackage/plugin/src/ios.tspackage/plugin/src/types.ts
✅ Files skipped from review due to trivial changes (2)
- README.md
- docs/expo-setup.md
🚧 Files skipped from review as they are similar to previous changes (5)
- package/plugin/src/ios.test.ts
- package/plugin/src/types.ts
- package/plugin/src/ios.ts
- package/plugin/src/android.ts
- package/package.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package/plugin/src/types.ts (1)
1-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat blank strings as unset, not as real config.
Line 49 and Line 55 use
??, and the guards in this file accept any string. That means''or whitespace fromapp.config.jsdisables the shared key fallback and can still generate empty location usage strings. That's a stupid foot-gun for env-backed config.Possible fix
+function hasNonEmptyString( + value: string | false | undefined, +): value is string { + return typeof value === 'string' && value.trim().length > 0; +} + export function wantsWhenInUseLocation( options: NitroMapsPluginOptions, ): options is NitroMapsPluginOptions & { locationPermission: string } { - return typeof options.locationPermission === 'string'; + return hasNonEmptyString(options.locationPermission); } export function wantsAlwaysLocation( options: NitroMapsPluginOptions, ): options is NitroMapsPluginOptions & { locationAlwaysPermission: string } { - return typeof options.locationAlwaysPermission === 'string'; + return hasNonEmptyString(options.locationAlwaysPermission); } export function resolveIosGoogleMapsApiKey( options: NitroMapsPluginOptions, ): string | undefined { - return options.iosGoogleMapsApiKey ?? options.googleMapsApiKey; + if (hasNonEmptyString(options.iosGoogleMapsApiKey)) { + return options.iosGoogleMapsApiKey; + } + return hasNonEmptyString(options.googleMapsApiKey) + ? options.googleMapsApiKey + : undefined; } export function resolveAndroidGoogleMapsApiKey( options: NitroMapsPluginOptions, ): string | undefined { - return options.androidGoogleMapsApiKey ?? options.googleMapsApiKey; + if (hasNonEmptyString(options.androidGoogleMapsApiKey)) { + return options.androidGoogleMapsApiKey; + } + return hasNonEmptyString(options.googleMapsApiKey) + ? options.googleMapsApiKey + : undefined; }Also applies to: 45-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package/plugin/src/types.ts` around lines 1 - 25, Treat blank or whitespace-only strings as unset in NitroMapsPluginOptions handling so empty config values don’t override fallbacks or generate empty permissions. Update the option guards and the fallback logic where googleMapsApiKey, iosGoogleMapsApiKey, androidGoogleMapsApiKey, locationPermission, and locationAlwaysPermission are read so they only count as set when the string contains non-whitespace content. Keep the shared-key fallback behavior intact in the plugin config path and avoid emitting usage descriptions for blank strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@package/plugin/src/types.ts`:
- Around line 1-25: Treat blank or whitespace-only strings as unset in
NitroMapsPluginOptions handling so empty config values don’t override fallbacks
or generate empty permissions. Update the option guards and the fallback logic
where googleMapsApiKey, iosGoogleMapsApiKey, androidGoogleMapsApiKey,
locationPermission, and locationAlwaysPermission are read so they only count as
set when the string contains non-whitespace content. Keep the shared-key
fallback behavior intact in the plugin config path and avoid emitting usage
descriptions for blank strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 606af0b8-bbb1-4291-b089-ab92ea6ce43b
📒 Files selected for processing (9)
README.mdexample/app.config.jspackage/android/README.mdpackage/package.jsonpackage/plugin/src/android.test.tspackage/plugin/src/android.tspackage/plugin/src/ios.test.tspackage/plugin/src/ios.tspackage/plugin/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- package/android/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- package/plugin/src/android.test.ts
- package/package.json
- package/plugin/src/android.ts
piotr-graczyk-dev
left a comment
There was a problem hiding this comment.
Approved in CodeRabbit Change Stack
Summary
Ships a first-party Expo config plugin so consumers can configure
react-native-nitro-mapsfromapp.json/app.config.jsand get the required native keys/permissions afterexpo prebuild— no manual edits. Closes #12.What's included
package/plugin/src/):withNitroMaps,android.ts,ios.ts,types.ts,index.tsusingcreateRunOncePlugin.com.google.android.geo.API_KEYmeta-data whengoogleMapsApiKeyis set; addsACCESS_FINE_LOCATION/ACCESS_COARSE_LOCATION(andACCESS_BACKGROUND_LOCATIONfor always) from location options.NSLocationWhenInUseUsageDescription/NSLocationAlwaysAndWhenInUseUsageDescription;googleMapsApiKeyis a documented no-op stub for future iOS Google Maps (Add Google Maps provider on iOS #2).googleMapsApiKey,locationPermission,locationAlwaysPermission.app.plugin.js,package.jsonexpo.plugin+build:pluginscript,@expo/config-pluginsdevDependency,tsconfig.plugin.json.example/native projects.docs/expo-setup.md(SDK 56 walkthrough, EAS secrets pattern).Test plan
bun run test— 13/13 passingbun run typecheck— cleanbun run build:plugin— emitsplugin/build/;app.plugin.jsloads and returns a functionexpo prebuild --cleanon the example app with Expo SDK 56 + New ArchitectureInfo.plisthas the location stringgoogleMapsApiKeyis omittedMade with Cursor