feat: add map provider adapters#19
Merged
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds provider-aware map contracts and runtime selection, native Apple and Google provider adapters, example provider switching, generated-code patching, and updated documentation plus roadmap entries. ChangesMulti-provider MapView
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 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 |
piotr-graczyk-dev
changed the title
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
package/src/providers.ts (1)
9-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo sources of truth, one inevitable drift.
SUPPORTED_PROVIDERSis your declared source of truth, butgetDefaultMapProviderre-hardcodes'apple'/'google'in a parallelswitch, andgetSupportedProvidersForCurrentPlatformwrites a thirdPlatform.OSswitch that just re-reads the same map. Add a provider toSUPPORTED_PROVIDERSlater and the default silently keeps pointing at whatever you remembered to also edit here. Derive the default fromSUPPORTED_PROVIDERS[platform][0]and collapse the redundant lookup switch.♻️ Suggested consolidation
-export function getDefaultMapProvider(): MapProvider { - switch (Platform.OS) { - case 'ios': - return 'apple'; - case 'android': - return 'google'; - default: - throw new Error( - `react-native-nitro-maps does not support platform "${Platform.OS}".`, - ); - } -} +export function getDefaultMapProvider(): MapProvider { + const supported = getSupportedProvidersForCurrentPlatform(); + const fallback = supported[0]; + if (fallback == null) { + throw new Error( + `react-native-nitro-maps does not support platform "${Platform.OS}".`, + ); + } + return fallback; +}🤖 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/src/providers.ts` around lines 9 - 25, The provider mapping has duplicated platform logic in getDefaultMapProvider and getSupportedProvidersForCurrentPlatform, which can drift from SUPPORTED_PROVIDERS. Update getDefaultMapProvider to derive its return value directly from SUPPORTED_PROVIDERS[Platform.OS][0], and make getSupportedProvidersForCurrentPlatform reuse the same source of truth instead of a separate Platform.OS switch. Keep the existing supported provider mapping in SUPPORTED_PROVIDERS as the single authoritative definition.example/App.tsx (1)
266-330: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThirty lines of copy-paste with two characters of difference.
The
applebranches are almost byte-identical: samekey,ref,style,region, handlers, andmapChildren. The only real differences areproviderand that Apple addsshowsScale. Maintaining two near-clones means every future prop change has to be made twice, and the next person will forget one. Extract the shared props into an object and only branch on the provider-specific bits.♻️ Suggested shape
const commonProps = { key: `${scenario.id}-${mapProvider}`, ref: mapRef, style: styles.map, mapType: MAP_TYPES[mapTypeIndex], region: scenario.region, clusteringEnabled: scenario.advanced?.clusteringEnabled, showsUserLocation: scenario.advanced?.showsUserLocation, followsUserLocation: scenario.advanced?.followsUserLocation, showsCompass: scenario.advanced?.showsCompass, customMapStyle: scenario.advanced?.customMapStyle, mapPadding: scenario.advanced?.mapPadding, onMapReady: handleMapReady, onRegionChange: handleRegionChange, onRegionChangeComplete: handleRegionChange, onClusterPress: handleClusterPress, onPress: /* ... */, onLongPress: /* ... */, } as const; const mapView = mapProvider === 'google' ? ( <MapView {...commonProps} provider="google">{mapChildren}</MapView> ) : ( <MapView {...commonProps} provider="apple" showsScale={scenario.advanced?.showsScale}> {mapChildren} </MapView> );(Verify the spread satisfies the discriminated
MapViewPropsunion — TS may need the provider literal inline, as written.)🤖 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 `@example/App.tsx` around lines 266 - 330, The `mapView` branch in `App.tsx` is duplicating almost all `MapView` props between the google and apple cases, with only `provider` and `showsScale` differing. Refactor the shared props into a common object or helper around `mapView`, `handleMapReady`, `handleRegionChange`, and `handleClusterPress`, then keep only the provider-specific props inline so future changes are made once instead of twice. Make sure the refactor still satisfies the `MapViewProps` union by keeping the `provider` literal on each branch and adding `showsScale` only for the apple case.
🤖 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
`@package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt`:
- Around line 331-335: The fit-to-bounds flow in GoogleMapProviderAdapter’s
fitToCoordinates/applyRegion still risks calling
CameraUpdateFactory.newLatLngBounds before the map view is actually laid out.
Replace the current view.post fallback with a real layout callback such as
OnGlobalLayoutListener or doOnLayout, and only invoke runUpdate after rechecking
view.width and view.height are non-zero so the bounds update runs on a measured
view.
- Around line 476-479: The location-permission check in GoogleMapProviderAdapter
should accept approximate location as well, not just fine location. Update the
permission logic around the existing hasPermission check to consider both
Manifest.permission.ACCESS_FINE_LOCATION and
Manifest.permission.ACCESS_COARSE_LOCATION so showsUserLocation remains enabled
when either valid location permission is granted.
- Around line 320-329: Keep isProgrammaticUpdate true for animated camera
operations until the animation actually finishes, instead of clearing it
immediately after animateCamera(...). Update the camera update flow in
GoogleMapProviderAdapter.kt, especially runUpdate and the related
fitToCoordinates, applyRegion, and updateMapCamera paths, so
isProgrammaticUpdate is reset from the animation completion callback while
non-animated moveCamera paths can still clear it immediately. Make sure the
setOnCameraMoveListener/setOnCameraIdleListener callbacks no longer emit
programmatic onRegionChange events during in-flight animations.
In `@package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt`:
- Around line 299-316: The adapter swap in installAdapter currently tears down
the existing adapter before makeAdapter(provider) succeeds, which can leave
adapter pointing at a dead view if creation throws. Update installAdapter to
create/validate the next adapter first using makeAdapter, then only call
prepareForRecycle and removeView on the old adapter, assign adapter to the new
instance, and attach/sync it afterward so the swap is atomic.
- Around line 329-343: Update syncState in HybridMapView so _mapPadding is
assigned to the MapProviderAdapter before setting region or camera, since
adapter.region may fit bounds using whatever padding is already present. Keep
the existing sync order for the other flags, but move the adapter.mapPadding
assignment earlier in the method so initial mounts and remounts with both region
and mapPadding use the correct viewport.
In `@package/ios/AppleMapProviderAdapter.swift`:
- Line 8: The map-ready state is being consumed too early in
AppleMapProviderAdapter, because notifyMapReadyIfNeeded() sets hasFiredMapReady
even when onMapReady is not yet available. Update the readiness handling so the
ready event is latched separately from delivery, and only mark it as fired after
onMapReady has actually been invoked; then ensure the callback setter replays
any pending ready event.
In `@package/ios/HybridMapView.swift`:
- Around line 296-302: The makeAdapter(for:) path in HybridMapView currently
uses preconditionFailure for .google, .openstreetmap, and .mapbox, which aborts
the iOS app if those typed values reach native code. Replace that fatal branch
with a safe fallback or explicit rejection path that returns a supported adapter
or surfaces a handled error instead of crashing. Keep the change localized to
makeAdapter(for:) and the MapProviderAdapter selection logic so unsupported
providers are dealt with before native execution.
In `@package/src/components/MapView.tsx`:
- Line 39: The MapView render path currently calls resolveMapProvider(provider)
with values that are type-valid but unsupported on the current platform, which
can throw during render and crash the tree. Update MapView and
resolveMapProvider to either narrow the public provider prop to only
platform-supported values or handle unsupported inputs explicitly before
rendering. Use the MapView component and resolveMapProvider helper as the main
touchpoints, and ensure unsupported providers like openstreetmap/mapbox are not
passed into the throw path on iOS/Android.
In `@package/src/types/map.ts`:
- Around line 70-85: The default-provider branch in ExistingDefaultProviderProps
is inconsistent with provider resolution on Android, since omitted provider maps
to Google Maps but still allows showsScale. Update the default branch in map
types to match the most restrictive default behavior used by providers.ts, or
otherwise remove/guard showsScale in ExistingDefaultProviderProps so it cannot
be accepted when provider is omitted on Android. Keep the fix centered on
ExistingDefaultProviderProps and the provider-based discriminated union in
map.ts so the default path no longer accepts props that the explicit google
branch rejects.
In `@package/type-tests/provider-props.ts`:
- Around line 3-35: The type-test fixtures in provider-props are missing the
required mapType field on every MapViewProps and MapViewPropsForProvider object,
so add mapType to each fixture before the provider-specific assertions. Update
the appleProps, googleProps, defaultProviderProps, plannedProviderProps,
googleScaleProps, and openStreetMapClusteringProps objects so they all satisfy
the current MapViewProps shape while keeping the existing `@ts-expect-error` cases
intact.
---
Nitpick comments:
In `@example/App.tsx`:
- Around line 266-330: The `mapView` branch in `App.tsx` is duplicating almost
all `MapView` props between the google and apple cases, with only `provider` and
`showsScale` differing. Refactor the shared props into a common object or helper
around `mapView`, `handleMapReady`, `handleRegionChange`, and
`handleClusterPress`, then keep only the provider-specific props inline so
future changes are made once instead of twice. Make sure the refactor still
satisfies the `MapViewProps` union by keeping the `provider` literal on each
branch and adding `showsScale` only for the apple case.
In `@package/src/providers.ts`:
- Around line 9-25: The provider mapping has duplicated platform logic in
getDefaultMapProvider and getSupportedProvidersForCurrentPlatform, which can
drift from SUPPORTED_PROVIDERS. Update getDefaultMapProvider to derive its
return value directly from SUPPORTED_PROVIDERS[Platform.OS][0], and make
getSupportedProvidersForCurrentPlatform reuse the same source of truth instead
of a separate Platform.OS switch. Keep the existing supported provider mapping
in SUPPORTED_PROVIDERS as the single authoritative definition.
🪄 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: dc331c70-f502-4d32-ac14-466f76e28c78
📒 Files selected for processing (24)
.github/workflows/ci.ymlCONTEXT.mdREADME.mddocs/adr/0001-map-provider-adapters.mddocs/architecture.mddocs/roadmap.mdexample/App.tsxpackage.jsonpackage/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.ktpackage/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.ktpackage/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.ktpackage/ios/AppleMapProviderAdapter.swiftpackage/ios/HybridMapView.swiftpackage/ios/HybridMapViewDelegate.swiftpackage/ios/MapProviderAdapter.swiftpackage/package.jsonpackage/src/components/MapView.tsxpackage/src/index.tspackage/src/native/specs/MapView.nitro.tspackage/src/providers.tspackage/src/types/index.tspackage/src/types/map.tspackage/tsconfig.type-tests.jsonpackage/type-tests/provider-props.ts
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
piotr-graczyk-dev
force-pushed
the
codex/map-provider-adapters
branch
from
June 26, 2026 15:05
809445f to
8b65568
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
example/App.tsx (1)
543-555: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winYou stuffed
setStatusinside asetProviderIndexupdater — that's not clever, it's wrong.State updater functions are supposed to be pure: input
current, output next state, nothing else. CallingsetStatus(...)as a side effect inside the updater means it fires every time React invokes the function — which, under StrictMode, is twice per update. It happens to be idempotent today, so it won't visibly explode, but you're relying on luck, not design. Hoist the side effects out of the updater.♻️ Proposed refactor
- const cycleProvider = useCallback(() => { - setProviderIndex((current) => { - if (SUPPORTED_MAP_PROVIDERS.length <= 1) { - setStatus(PROVIDER_LABELS[provider]); - return current; - } - - const next = (current + 1) % SUPPORTED_MAP_PROVIDERS.length; - setMapReady(false); - setStatus(PROVIDER_LABELS[SUPPORTED_MAP_PROVIDERS[next] ?? provider]); - return next; - }); - }, [provider]); + const cycleProvider = useCallback(() => { + if (SUPPORTED_MAP_PROVIDERS.length <= 1) { + setStatus(PROVIDER_LABELS[provider]); + return; + } + + const next = (providerIndex + 1) % SUPPORTED_MAP_PROVIDERS.length; + setProviderIndex(next); + setMapReady(false); + setStatus(PROVIDER_LABELS[SUPPORTED_MAP_PROVIDERS[next] ?? provider]); + }, [provider, providerIndex]);🤖 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 `@example/App.tsx` around lines 543 - 555, The cycleProvider callback mixes state updates and side effects by calling setStatus and setMapReady inside the setProviderIndex updater, which should stay pure. Refactor it so the updater only computes and returns the next provider index, then perform setMapReady(false) and setStatus(...) outside the updater using the computed next provider value, keeping the behavior in cycleProvider intact while avoiding side effects during state calculation.
🤖 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 `@example/App.tsx`:
- Around line 57-66: The provider switch in getSupportedMapProviders() only ever
returns one provider on iOS and Android, so cycleProvider cannot actually switch
and the control becomes a no-op. Update the UI logic around the provider cycling
button to hide or disable it when SupportedExampleProvider has only one entry,
or extend getSupportedMapProviders()/cycleProvider to support a real alternate
provider path.
In `@package/ios/AppleMapProviderAdapter.swift`:
- Around line 213-240: The programmatic-update guard in AppleMapProviderAdapter
is being cleared too early in fitToCoordinates, applyRegion, and
updateMapCamera, which allows region-change callbacks to fire during imperative
map transitions. Keep isProgrammaticUpdate true until the map or animation
actually completes, and move the reset into the relevant completion path rather
than immediately after setVisibleMapRect, setRegion, or the UIView.animate
block. Use the existing fitToCoordinates, applyRegion, updateMapCamera, and any
related region-change completion handling in AppleMapProviderAdapter to ensure
onRegionChange and onRegionChangeComplete stay suppressed for programmatic
moves.
In `@package/scripts/patch-nitrogen-generated.mjs`:
- Around line 8-17: The idempotency logic in replaceOnce is too loose and can
report success without actually patching the intended generated block. Update
replaceOnce so it verifies the target “from” block is present and unique before
replacing, and only treats the patch as already-applied when the exact
replacement corresponds to that same location rather than any matching “to” text
elsewhere in the file. If the expected generated block is missing or appears
multiple times, fail loudly instead of silently skipping, and keep the logic
localized to replaceOnce in patch-nitrogen-generated.mjs.
---
Nitpick comments:
In `@example/App.tsx`:
- Around line 543-555: The cycleProvider callback mixes state updates and side
effects by calling setStatus and setMapReady inside the setProviderIndex
updater, which should stay pure. Refactor it so the updater only computes and
returns the next provider index, then perform setMapReady(false) and
setStatus(...) outside the updater using the computed next provider value,
keeping the behavior in cycleProvider intact while avoiding side effects during
state calculation.
🪄 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: 74d0f86b-e231-4d61-b326-ddb904c49537
📒 Files selected for processing (24)
CONTEXT.mdREADME.mddocs/adr/0001-map-provider-adapters.mddocs/architecture.mddocs/roadmap.mdexample/App.tsxpackage.jsonpackage/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.ktpackage/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.ktpackage/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.ktpackage/ios/AppleMapProviderAdapter.swiftpackage/ios/HybridMapView.swiftpackage/ios/HybridMapViewDelegate.swiftpackage/ios/MapProviderAdapter.swiftpackage/package.jsonpackage/scripts/patch-nitrogen-generated.mjspackage/src/components/MapView.tsxpackage/src/index.tspackage/src/native/specs/MapView.nitro.tspackage/src/providers.tspackage/src/types/index.tspackage/src/types/map.tspackage/tsconfig.type-tests.jsonpackage/type-tests/provider-props.ts
✅ Files skipped from review due to trivial changes (6)
- package/tsconfig.type-tests.json
- package/src/index.ts
- docs/adr/0001-map-provider-adapters.md
- CONTEXT.md
- docs/roadmap.md
- docs/architecture.md
🚧 Files skipped from review as they are similar to previous changes (13)
- package/src/types/index.ts
- package.json
- package/ios/HybridMapViewDelegate.swift
- package/package.json
- package/src/providers.ts
- package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt
- package/type-tests/provider-props.ts
- README.md
- package/src/native/specs/MapView.nitro.ts
- package/ios/MapProviderAdapter.swift
- package/src/types/map.ts
- package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt
- package/ios/HybridMapView.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package/ios/AppleMapProviderAdapter.swift (1)
264-266: 🎯 Functional Correctness | 🔵 TrivialMake the intent explicit here.
animateToClusterRegionis reached from a tapped cluster, so lettingonRegionChange*fire is fine; add a short comment if that’s the contract, otherwise wrap it inbeginProgrammaticUpdate()to match the other map mutations.🤖 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/ios/AppleMapProviderAdapter.swift` around lines 264 - 266, The cluster-region animation in animateToClusterRegion should make its update intent explicit. If the desired contract is to allow onRegionChange* callbacks to fire when a cluster is tapped, add a brief comment near the setRegion call to document that behavior; otherwise, wrap the region change in beginProgrammaticUpdate() so it matches the other programmatic map mutations in AppleMapProviderAdapter.
🤖 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 `@package/ios/AppleMapProviderAdapter.swift`:
- Around line 213-224: The animated region update path in
AppleMapProviderAdapter should not rely only on region callbacks to clear the
programmatic update state. Update the fitToCoordinates and applyRegion flow
around beginProgrammaticUpdate and setVisibleMapRect so that animated updates
also use a completion/fallback reset like updateMapCamera, ensuring
programmaticUpdateCount is cleared even when the target region does not actually
change. Keep the fix localized to the methods handling these region setters and
the existing endProgrammaticUpdateIfRegionDoesNotChange logic.
---
Nitpick comments:
In `@package/ios/AppleMapProviderAdapter.swift`:
- Around line 264-266: The cluster-region animation in animateToClusterRegion
should make its update intent explicit. If the desired contract is to allow
onRegionChange* callbacks to fire when a cluster is tapped, add a brief comment
near the setRegion call to document that behavior; otherwise, wrap the region
change in beginProgrammaticUpdate() so it matches the other programmatic map
mutations in AppleMapProviderAdapter.
🪄 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: f20cfd13-02ac-46a9-bab3-79dcc9c463c8
📒 Files selected for processing (4)
example/App.tsxpackage/ios/AppleMapProviderAdapter.swiftpackage/ios/HybridMapViewDelegate.swiftpackage/scripts/patch-nitrogen-generated.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- package/scripts/patch-nitrogen-generated.mjs
- example/App.tsx
piotr-graczyk-dev
force-pushed
the
codex/map-provider-adapters
branch
from
June 26, 2026 15:30
57a730f to
ffeb5f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@package/ios/AppleMapProviderAdapter.swift`:
- Around line 301-309: Do not reset or reuse programmatic update IDs in
AppleMapProviderAdapter because delayed fallbacks scheduled by
scheduleProgrammaticUpdateFallback can still fire after reuse and mistakenly
call endProgrammaticUpdate on a new update. Keep nextProgrammaticUpdateID
monotonic, or introduce a recycle-generation/token check that lets
endProgrammaticUpdate ignore stale asyncAfter callbacks. Update the reset logic
around nextProgrammaticUpdateID and ensure any fallback validation uses both
updateID and the current generation/state so old callbacks cannot clear a newer
programmatic guard.
🪄 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: 6272cb03-804c-41ec-a97d-9ab462529b1c
📒 Files selected for processing (4)
example/App.tsxpackage/ios/AppleMapProviderAdapter.swiftpackage/ios/HybridMapViewDelegate.swiftpackage/scripts/patch-nitrogen-generated.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- package/ios/HybridMapViewDelegate.swift
- package/scripts/patch-nitrogen-generated.mjs
- example/App.tsx
piotr-graczyk-dev
force-pushed
the
codex/map-provider-adapters
branch
from
June 26, 2026 15:36
ffeb5f3 to
feafd79
Compare
Contributor
Author
|
@coderabbitai review |
This was referenced Jun 26, 2026
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
1 participant
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
Validation