tomchentw / react-google-maps

React.js Google Maps integration component
https://tomchentw.github.io/react-google-maps/
MIT License
4.62k stars 938 forks source link

Fundamental performance problems with MarkerClusterer #836

Open mschipperheyn opened 6 years ago

mschipperheyn commented 6 years ago

I have tried and failed to resolve performance problems with having many markers using the MarkerClusterer. I now believe that the performance issue is an architectural limitation caused by the fact that for each Marker a <div></div> is rendered and performance decreases lineairly with increasing numbers of markers.

The MarkerClusterer is intended to deal with many markers and it's not working as advertised. I would recommend rewriting this and use the clusterer api more directly, avoiding a dom node for every marker. I can share the code I wrote based google-map-react as I was forced to move away from react-google-maps. Certainly not as elegant as it could be, but I was under a time crunch.

gergely-ujvari commented 6 years ago

:+1:

I was also forced to write my own component for dealing with multiple markers.

thunderbird3 commented 6 years ago

@mschipperheyn I am also about to use MarkerClusterer with google-map-react. If you happen to have your component as a snippet somewhere, it would be very helpful!

mschipperheyn commented 6 years ago
/* global google */
import React from 'react';
import PropTypes from 'prop-types';
import GoogleMapReact from 'google-map-react';
import MarkerClustererPlus from 'marker-clusterer-plus';
import styles from './map.css';

const clusterIconStyles = [
  {
    height: 90,
    textColor: '#ffffff',
    url: '/img/senseo-90.png',
    width: 90
  }
];

const markerClustererFactory = (map, markers) => {
  const mcOptions = {
    maxZoom: 15,
    gridSize: 60,
    enableRetinaIcons: true,
    averageCenter: true,
    styles: clusterIconStyles
  };
  return new MarkerClustererPlus(map, markers, mcOptions);
};

export default class Map extends React.PureComponent {
  static contextTypes = {
    goApiId: PropTypes.string.isRequired
  };

  static propTypes = {
    center: PropTypes.shape({
      lat: PropTypes.number.isRequired,
      lng: PropTypes.number.isRequired
    }),
    markers: PropTypes.arrayOf(
      PropTypes.shape({
        id: PropTypes.number.isRequired,
        lng: PropTypes.number.isRequired,
        lat: PropTypes.number.isRequired,
        rating: PropTypes.number.isRequired
      })
    ),
    zoom: PropTypes.number,
    highlight: PropTypes.number,
    onMarkerClick: PropTypes.func.isRequired
  };

  static defaultProps = {
    center: {
      lat: 52.092876,
      lng: 5.10448
    },
    markers: [],
    zoom: 8,
    highlight: -1
  };

  state = {
    draggable: true
  };

  componentWillMount() {
    const { goApiId } = this.context;
    this.mapConfig = {
      key: goApiId,
      v: '3.exp',
      libraries: ['geometry', 'drawing', 'places']
    };
  }

  componentDidMount() {
    if (typeof window !== 'undefined') {
      window.addEventListener('resize', this.updateWindowDimensions);
      this.updateWindowDimensions();
    }
  }

  componentDidUpdate(prevProps) {
    const { markers } = this.props;
    if (this.markerClusterer) {
      if (prevProps.markers !== markers) this.drawMarkers();
    } else if (typeof window !== 'undefined') {
      this.loadInterval = setInterval(() => {
        if (this.markerClusterer && markers.length > 0) {
          clearInterval(this.loadInterval);
          this.drawMarkers();
        }
      }, 1000);
    }
  }

  componentWillUnmount() {
    this.markerClusterer = null;
    this.map = null;
    if (typeof window !== 'undefined')
      window.removeEventListener('resize', this.updateWindowDimensions);
  }

  handleRenderMarkers = map => {
    const { markers } = this.props;
    this.map = map.map;
    if (!this.markerClusterer)
      this.markerClusterer = markerClustererFactory(map.map, markers);
    if (markers.length !== this.markerClusterer.getMarkers().length)
      this.drawMarkers();
  };

  updateWindowDimensions = () => {
    if (typeof window !== 'undefined')
      this.setState({
        draggable: window.innerWidth > 479.8
      });
  };

  drawMarkers = () => {
    const { markers, highlight, onMarkerClick } = this.props;
    if (!this.markerClusterer) return;
    google.maps.Marker.prototype.isDraggable = () => false;
    const googleMarkers = markers.map(({ id, lng, lat }) => {
      const marker = new google.maps.Marker({
        map: this.map,
        position: new google.maps.LatLng(lat, lng),
        draggable: false,
        icon:
          id === highlight
            ? '/img/senseo-pin-featured.png'
            : '/img/senseo-pin.png'
      });
      google.maps.event.addListener(marker, 'click', () => {
        onMarkerClick(id);
      });
      return marker;
    });
    this.markerClusterer.clearMarkers();
    this.markerClusterer.addMarkers(googleMarkers, true);
    this.markerClusterer.repaint();
  };

  render() {
    const { zoom, center } = this.props;
    return (
      <div className={styles.mapContainer}>
        <GoogleMapReact
          bootstrapURLKeys={this.mapConfig}
          defaultCenter={center}
          defaultZoom={zoom}
          onGoogleApiLoaded={this.handleRenderMarkers}
          yesIWantToUseGoogleMapApiInternals
        />
      </div>
    );
  }
}
JvB94 commented 6 years ago

Hi, I had the same problem. I was helped to hand over the prop "noRedraw" to the marker. The problem is that otherwise for every marker that is added or deleted the entire card is reloaded.

ariccio commented 3 years ago

@JvB94 do you mean noClustererRedraw? I can't find a noRedraw anywhere in the repo!

ariccio commented 3 years ago

Hmmm, no, it looks like it's making single markers disappear sometimes?

ariccio commented 3 years ago

My own marker drawing code is a bit janky due to excessive reloads from the server, but it does seem like banning all redraws caused a bug where non-clustered points disappeared.

If you're coming ere from google, what you actually probably want is to do something like this as long as you know how many markers you're going to add ahead of time:

    let noClustererRedraw = undefined;
    if (index === (placesSize - 1)) {
        noClustererRedraw = false;
    }
    else {
        noClustererRedraw = true;
    }
    return (
        <Marker position={pos} key={/*code for key item*/} clusterer={clusterer} onClick={clickHandler} noClustererRedraw={noClustererRedraw} />
    )

Depending, of course, on your own personal code style preferences. I prefer if/else to inline ternary operators. In a language were function calls weren't so miserably slow, I'd also factor that out to a function :)

Hector1567XD commented 3 years ago

My own marker drawing code is a bit janky due to excessive reloads from the server, but it does seem like banning all redraws caused a bug where non-clustered points disappeared.

If you're coming ere from google, what you actually probably want is to do something like this as long as you know how many markers you're going to add ahead of time:

    let noClustererRedraw = undefined;
    if (index === (placesSize - 1)) {
        noClustererRedraw = false;
    }
    else {
        noClustererRedraw = true;
    }
    return (
        <Marker position={pos} key={/*code for key item*/} clusterer={clusterer} onClick={clickHandler} noClustererRedraw={noClustererRedraw} />
    )

Depending, of course, on your own personal code style preferences. I prefer if/else to inline ternary operators. In a language were function calls weren't so miserably slow, I'd also factor that out to a function :)

Thank you!!

This solution worked for me!, but I had to also add a variation to the key of the last marker to make sure that it would redraw when the array changed but kept the last marker.

I could suggest to anyone who needs it:

    let noClustererRedraw;
    let markerKey = /*code for key item*/;

    if (index === (placesSize - 1)) {
        noClustererRedraw = false;
        markerKey = markerKey+'-redrawer-'+uniqid();
    }else{
        noClustererRedraw = true;
    }

    return (
        <Marker 
                position={pos} 
                key={markerKey} 
                clusterer={clusterer} 
                onClick={clickHandler} 
                noClustererRedraw={noClustererRedraw} 
        />
    )
TeddySabatierLimpide commented 1 year ago

Hi ! Thans for the great tips, for me it's laggy when I remove markers and maps redraw every clusters on my react app. How would you do ?

releshreyas commented 1 year ago

Documented a solution in https://github.com/JustFly1984/react-google-maps-api/issues/2849#issuecomment-1645506350 - maybe this works for you.