Map callbacks not working with useState hook after version 4.8.6

[wtanna]

It seems that version 5 has broken the callbacks for the <Map /> component when using the useState hook. Here is a simplified example map that uses onMoveStart, onMoveEnd with a Marker that also handles the same piece of state. The below code can be put into a https://codesandbox.io/ after adding the following packages:

• mapbox-gl @2.0.1
• react-mapbox-gl @5.1.1

import React, { useState } from "react";
import "./styles.css";

import ReactMapboxGl, { Marker } from "react-mapbox-gl";
import "mapbox-gl/dist/mapbox-gl.css";

const appStyles = {
  display: "flex",
  "flex-direction": "column",
  "text-align": "center",
  "font-family": "sans-serif",
  height: "100vh",
  width: "100vw"
};

const markerStyles = {
  "background-color": "pink",
  display: "flex",
  "align-items": "center",
  height: "50px",
  "font-size": "20px"
};

const mapContainerStyles = {
  flex: "1 1 auto"
};

// Make sure you put in your access token
const Map = ReactMapboxGl({
  accessToken: ACCESS_TOKEN
  dragRotate: false,
  pitchWithRotate: false
});

export default function App() {
  const [count, setCount] = useState(0);
  const [stylesLoaded, setStylesLoaded] = useState(false);

  return (
    <div style={appStyles} className="App">
      <h1>Count: {count}</h1>
      <h3>Styles Loaded: {JSON.stringify(stylesLoaded)}</h3>
      <Map
        onMoveStart={() => {
          console.log("onMoveStart count", count);
          setCount(1);
        }}
        onMoveEnd={() => {
          console.log("onMoveEnd count", count);
          if (count > 0) setCount(0);
        }}
        onStyleLoad={() => {
          setStylesLoaded(true);
        }}
        containerStyle={mapContainerStyles}
        style="mapbox://styles/mapbox/streets-v11"
      >
        <Marker
          style={markerStyles}
          coordinates={[-0.2416815, 51.5285582]}
          anchor="bottom"
          onClick={() => {
            console.log("Marker count", count);
            setCount(2);
          }}
        >
          <div className="marker-content">A Marker</div>
        </Marker>
      </Map>
    </div>
  );
}

When you move the map, the counter adds 1. When the map stops moving it should then reset it back to 0. However, when looking at the console.log it is saying that the state is still a count of 0 even though the app is displaying a count of 1.

If you then click the marker, to make the count 2, then move the map, it will go back to 1 because the the map started moving again, but the console log in that callback does still say that the count is 0, even though it's able to set it correctly. It then also won't set it to 0 when it is done moving. Looking at the console logs, it says that the count is 0 in onMoveEnd which is why it's not going to call setCount(0), it's already 0.

I've tried various different ways to get this work with hooks and the only way that I can seem to get this to work is if I go back to making this a class component or going back to version 4.8.6. Using this.state resolves the issue with all the other code completely the same.

The passed in handlers don't seem to be updating correctly when the local state of the component changes.

[JClackett]

Have the exact same, seems to be using an old instance of the callback?

[SiddharthMantri]

@wtanna @JClackett I have identified the issue and I fixed it for the use cases I've had. The issue is in this line of code where the event handlers are removed and reattached: https://github.com/alex3165/react-mapbox-gl/blob/master/src/map-events.tsx#L139

I think the code should be listeners[eventKey] && typeof currentProps[eventKey] === 'function'

This change works for me and the way I did it is by creating a new file locally like this

import * as MapboxGl from "mapbox-gl";
import * as React from "react";
import { MapContext } from "react-mapbox-gl";
import {
  Events,
  listenEvents,
  events,
  Listeners,
} from "react-mapbox-gl/lib/map-events";
import { createPortal } from "react-dom";
const isEqual = require("deep-equal"); //tslint:disable-line

export const updateEvents = (
  listeners: Listeners,
  currentProps: Partial<Events>,
  map: MapboxGl.Map
) => {
  const toListenOff = Object.keys(events).filter(
    (eventKey) =>
      // @ts-ignore
      listeners[eventKey] && typeof currentProps[eventKey] === "function"
  );

  toListenOff.forEach((key) => {
    // @ts-ignore
    map.off(events[key], listeners[key]);
    // @ts-ignore
    delete listeners[key];
  });

  const toListenOn = Object.keys(events)
    // @ts-ignore
    .filter((key) => !listeners[key] && typeof currentProps[key] === "function")
    // @ts-ignore
    .reduce((acc, next) => ((acc[next] = events[next]), acc), {});

  const newListeners = listenEvents(toListenOn, currentProps, map);

  return { ...listeners, ...newListeners };
};

export interface PaddingOptions {
  top: number;
  bottom: number;
  left: number;
  right: number;
}

export interface FitBoundsOptions {
  linear?: boolean;
  easing?: (time: number) => number;
  padding?: number | PaddingOptions;
  offset?: MapboxGl.Point | [number, number];
  maxZoom?: number;
  duration?: number;
}

export type FitBounds = [[number, number], [number, number]];

export interface AnimationOptions {
  duration: number;
  animate: boolean;
  easing(time: number): number;
  offset: number[];
}

export interface FlyToOptions {
  curve: number;
  minZoom: number;
  speed: number;
  screenSpeed: number;
}

// React Props updated between re-render
export interface Props {
  style: string | MapboxGl.Style;
  center?: [number, number];
  zoom?: [number];
  maxBounds?: MapboxGl.LngLatBounds | FitBounds;
  fitBounds?: FitBounds;
  fitBoundsOptions?: FitBoundsOptions;
  bearing?: [number];
  pitch?: [number];
  containerStyle?: React.CSSProperties;
  className?: string;
  movingMethod?: "jumpTo" | "easeTo" | "flyTo";
  animationOptions?: Partial<AnimationOptions>;
  flyToOptions?: Partial<FlyToOptions>;
  children?: JSX.Element | JSX.Element[] | Array<JSX.Element | undefined>;
  renderChildrenInPortal?: boolean;
}

export interface State {
  map?: MapboxGl.Map;
  ready: boolean;
}

export type RequestTransformFunction = (
  url: string,
  resourceType: any // tslint:disable-line:no-any
) => any; // tslint:disable-line:no-any

// Static Properties of the map
export interface FactoryParameters {
  accessToken: string;
  apiUrl?: string;
  minZoom?: number;
  maxZoom?: number;
  hash?: boolean;
  preserveDrawingBuffer?: boolean;
  scrollZoom?: boolean;
  interactive?: boolean;
  dragRotate?: boolean;
  pitchWithRotate?: boolean;
  attributionControl?: boolean;
  customAttribution?: string | string[];
  logoPosition?: "top-left" | "top-right" | "bottom-left" | "bottom-right";
  renderWorldCopies?: boolean;
  trackResize?: boolean;
  touchZoomRotate?: boolean;
  doubleClickZoom?: boolean;
  keyboard?: boolean;
  dragPan?: boolean;
  boxZoom?: boolean;
  refreshExpiredTiles?: boolean;
  failIfMajorPerformanceCaveat?: boolean;
  bearingSnap?: number;
  transformRequest?: RequestTransformFunction;
  antialias?: boolean;
  mapInstance?: MapboxGl.Map;
}

// Satisfy typescript pitfall with defaultProps
const defaultZoom = [11];
const defaultMovingMethod = "flyTo";
const defaultCenter = [-0.2416815, 51.5285582];

// tslint:disable-next-line:no-namespace
declare global {
  namespace mapboxgl {
    export interface MapboxOptions {
      failIfMajorPerformanceCaveat?: boolean;
      transformRequest?: MapboxGl.TransformRequestFunction;
    }
  }
}

const MapFactory = ({
  accessToken,
  apiUrl,
  minZoom = 0,
  maxZoom = 20,
  hash = false,
  preserveDrawingBuffer = false,
  scrollZoom = true,
  interactive = true,
  dragRotate = true,
  pitchWithRotate = true,
  attributionControl = true,
  customAttribution,
  logoPosition = "bottom-left",
  renderWorldCopies = true,
  trackResize = true,
  touchZoomRotate = true,
  doubleClickZoom = true,
  keyboard = true,
  dragPan = true,
  boxZoom = true,
  refreshExpiredTiles = true,
  failIfMajorPerformanceCaveat = false,
  bearingSnap = 7,
  antialias = false,
  mapInstance,
  transformRequest,
}: FactoryParameters) => {
  return class ReactMapboxGl extends React.Component<Props & Events, State> {
    public static defaultProps = {
      // tslint:disable-next-line:no-any
      onStyleLoad: (map: MapboxGl.Map, evt: any) => null,
      center: defaultCenter,
      zoom: defaultZoom,
      bearing: 0,
      movingMethod: defaultMovingMethod,
      pitch: 0,
      containerStyle: {
        textAlign: "left",
      },
    };

    public state: State = {
      map: mapInstance,
      ready: false,
    };

    public listeners: Listeners = {};

    // tslint:disable-next-line:variable-name
    public _isMounted = true;

    public container?: HTMLElement;

    public calcCenter = (bounds: FitBounds): [number, number] => [
      (bounds[0][0] + bounds[1][0]) / 2,
      (bounds[0][1] + bounds[1][1]) / 2,
    ];

    public componentDidMount() {
      const {
        style,
        onStyleLoad,
        center,
        pitch,
        zoom,
        fitBounds,
        fitBoundsOptions,
        bearing,
        maxBounds,
      } = this.props;

      // tslint:disable-next-line:no-any
      (MapboxGl as any).accessToken = accessToken;
      if (apiUrl) {
        // tslint:disable-next-line:no-any
        (MapboxGl as any).config.API_URL = apiUrl;
      }

      if (!Array.isArray(zoom)) {
        throw new Error(
          "zoom need to be an array type of length 1 for reliable update"
        );
      }

      const opts: MapboxGl.MapboxOptions = {
        preserveDrawingBuffer,
        hash,
        zoom: zoom[0],
        minZoom,
        maxZoom,
        maxBounds,
        container: this.container!,
        center:
          fitBounds && center === defaultCenter
            ? this.calcCenter(fitBounds)
            : center,
        style,
        scrollZoom,
        attributionControl,
        customAttribution,
        interactive,
        dragRotate,
        pitchWithRotate,
        renderWorldCopies,
        trackResize,
        touchZoomRotate,
        doubleClickZoom,
        keyboard,
        dragPan,
        boxZoom,
        refreshExpiredTiles,
        logoPosition,
        bearingSnap,
        failIfMajorPerformanceCaveat,
        antialias,
        transformRequest,
      };

      if (bearing) {
        if (!Array.isArray(bearing)) {
          throw new Error(
            "bearing need to be an array type of length 1 for reliable update"
          );
        }

        opts.bearing = bearing[0];
      }

      if (pitch) {
        if (!Array.isArray(pitch)) {
          throw new Error(
            "pitch need to be an array type of length 1 for reliable update"
          );
        }

        opts.pitch = pitch[0];
      }

      // This is a hack to allow injecting the map instance, which assists
      // in testing and theoretically provides a means for users to inject
      // their own map instance.
      let map = this.state.map;

      if (!map) {
        map = new MapboxGl.Map(opts);
        this.setState({ map });
      }

      if (fitBounds) {
        map.fitBounds(fitBounds, fitBoundsOptions, { fitboundUpdate: true });
      }

      // tslint:disable-next-line:no-any
      map.on("load", (evt: React.SyntheticEvent<any>) => {
        if (this._isMounted) {
          this.setState({ ready: true });
        }

        if (onStyleLoad) {
          onStyleLoad(map!, evt);
        }
      });

      this.listeners = listenEvents(events, this.props, map);
    }

    public componentWillUnmount() {
      const { map } = this.state;
      this._isMounted = false;

      if (map) {
        map.remove();
      }
    }

    public componentDidUpdate(prevProps: Props & Events) {
      const { map } = this.state;
      if (!map) {
        return null;
      }

      // Update event listeners
      this.listeners = updateEvents(this.listeners, this.props, map);

      const center = map.getCenter();
      const zoom = map.getZoom();
      const bearing = map.getBearing();
      const pitch = map.getPitch();

      const didZoomUpdate =
        prevProps.zoom !== this.props.zoom &&
        (this.props.zoom && this.props.zoom[0]) !== zoom;

      const didCenterUpdate =
        prevProps.center !== this.props.center &&
        ((this.props.center && this.props.center[0]) !== center.lng ||
          (this.props.center && this.props.center[1]) !== center.lat);

      const didBearingUpdate =
        prevProps.bearing !== this.props.bearing &&
        (this.props.bearing && this.props.bearing[0]) !== bearing;

      const didPitchUpdate =
        prevProps.pitch !== this.props.pitch &&
        (this.props.pitch && this.props.pitch[0]) !== pitch;

      if (this.props.maxBounds) {
        const didMaxBoundsUpdate = prevProps.maxBounds !== this.props.maxBounds;

        if (didMaxBoundsUpdate) {
          map.setMaxBounds(this.props.maxBounds);
        }
      }

      if (this.props.fitBounds) {
        const { fitBounds } = prevProps;

        const didFitBoundsUpdate =
          fitBounds !== this.props.fitBounds || // Check for reference equality
          this.props.fitBounds.length !== (fitBounds && fitBounds.length) || // Added element
          !!fitBounds.filter((c, i) => {
            // Check for equality
            const nc = this.props.fitBounds && this.props.fitBounds[i];
            return c[0] !== (nc && nc[0]) || c[1] !== (nc && nc[1]);
          })[0];

        if (
          didFitBoundsUpdate ||
          !isEqual(prevProps.fitBoundsOptions, this.props.fitBoundsOptions)
        ) {
          map.fitBounds(this.props.fitBounds, this.props.fitBoundsOptions, {
            fitboundUpdate: true,
          });
        }
      }

      if (
        didZoomUpdate ||
        didCenterUpdate ||
        didBearingUpdate ||
        didPitchUpdate
      ) {
        const mm: string = this.props.movingMethod || defaultMovingMethod;
        const { flyToOptions, animationOptions } = this.props;
        // @ts-ignore
        map[mm]({
          ...animationOptions,
          ...flyToOptions,
          zoom: didZoomUpdate && this.props.zoom ? this.props.zoom[0] : zoom,
          center: didCenterUpdate ? this.props.center : center,
          bearing: didBearingUpdate ? this.props.bearing : bearing,
          pitch: didPitchUpdate ? this.props.pitch : pitch,
        });
      }

      if (!isEqual(prevProps.style, this.props.style)) {
        map.setStyle(this.props.style);
      }

      return null;
    }

    public setRef = (x: HTMLElement | null) => {
      this.container = x!;
    };

    public render() {
      const {
        containerStyle,
        className,
        children,
        renderChildrenInPortal,
      } = this.props;

      const { ready, map } = this.state;

      if (renderChildrenInPortal) {
        const container =
          ready && map && typeof map.getCanvasContainer === "function"
            ? map.getCanvasContainer()
            : undefined;

        return (
          <MapContext.Provider value={map}>
            <div
              ref={this.setRef}
              className={className}
              style={{ ...containerStyle }}
            >
              {ready && container && createPortal(children, container)}
            </div>
          </MapContext.Provider>
        );
      }

      return (
        <MapContext.Provider value={map}>
          <div
            ref={this.setRef}
            className={className}
            style={{ ...containerStyle }}
          >
            {ready && children}
          </div>
        </MapContext.Provider>
      );
    }
  };
};

export default MapFactory;

And then using it

import MapFactory from "my-path-to-file";

const MapBase = MapFactory({...});

[akre54]

Running into this too. It looks like the issue is that when the callback is updated to a new function, that line in toListenOff only checks whether the prop is a function, not whether it's referentially the same as the old version. The fix is to pass the prevProps from cDU, or to move to hooks here.

Fixing this could have performance impacts for non-memoized functions, so might be useful to add an option.

For now, I'm using my own useEffect hook with a useRef to hack around the problem.

[akre54]

Created a PR at #948, but I don't love the solution. Any suggestions for how to improve it are welcome!