Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/app/Scenes/Map/GlobalMap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,12 @@ export const GlobalMap: React.FC<Props> = (props) => {
}

const onPressUserPositionButton = () => {
// @ts-expect-error STRICTNESS_MIGRATION --- 🚨 Unsafe legacy code 🚨 Please delete this and fix any type errors if you have time 🙏
const { lat, lng } = userLocation
if (!isValidLatLng(userLocation)) {
return
}

cameraRef.current?.setCamera({
centerCoordinate: [lng, lat],
centerCoordinate: [userLocation.lng, userLocation.lat],
zoomLevel: DefaultZoomLevel,
animationDuration: 500,
})
Expand All @@ -146,7 +148,7 @@ export const GlobalMap: React.FC<Props> = (props) => {
isLoading={!viewer.city}
onPress={onPressCitySwitcherButton}
/>
{!!userLocation && (
{!!isValidLatLng(userLocation) && (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isValidLatLng already returns a boolean, so the !! is a no-op here (the old !!userLocation needed it to coerce an object).

Suggested change
{!!isValidLatLng(userLocation) && (
{isValidLatLng(userLocation) && (

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope

<Box style={{ marginLeft: 10 }}>
<UserPositionButton
highlight={userLocation === currentLocation}
Expand Down Expand Up @@ -337,11 +339,16 @@ export const GlobalMap: React.FC<Props> = (props) => {
}

const onUserLocationUpdate = (location: MapboxGL.Location) => {
if (!location || !location.coords) {
const coords = location?.coords

// The native side sends updates with an empty `coords` object before the first location fix
// lands (and for heading-only updates), which would otherwise leave us with a location made of
// `undefined`s and crash the camera.
if (typeof coords?.latitude !== "number" || typeof coords?.longitude !== "number") {
return
}

setUserLocation(longCoordsToLocation(location.coords))
setUserLocation(longCoordsToLocation(coords))
}

const onRegionIsChanging = async () => {
Expand Down Expand Up @@ -590,6 +597,14 @@ const longCoordsToLocation = (coords: { longitude: number; latitude: number }) =
return { lat: coords.latitude, lng: coords.longitude }
}

/**
* `city.coordinates` is nullable in the schema and location updates can arrive without coordinates,
* so make sure we actually have numbers before handing them over to the map.
*/
const isValidLatLng = (location: any): location is { lat: number; lng: number } => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any here throws away the type info the caller already has. userLocation comes from viewer.city?.coordinates, whose schema type is { lat: Float, lng: Float } (both nullable — data/schema.graphql:22546). Naming that shape keeps the guard honest and still narrows correctly:

Suggested change
const isValidLatLng = (location: any): location is { lat: number; lng: number } => {
const isValidLatLng = (
location: { lat?: number | null; lng?: number | null } | null | undefined
): location is { lat: number; lng: number } => {

Not blocking — @typescript-eslint/no-explicit-any is off in this repo — but this PR is deleting a STRICTNESS_MIGRATION escape hatch, so it'd be nice not to add an any in its place.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this

return typeof location?.lat === "number" && typeof location?.lng === "number"
}

const tracks = {
trackPinTap: (_: any, __: any, args: any) => {
const actionName = args[0]
Expand Down
Loading