Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster children don't re-render on state updates #31

Open
jaydenseric opened this issue Apr 14, 2020 · 7 comments
Open

Cluster children don't re-render on state updates #31

jaydenseric opened this issue Apr 14, 2020 · 7 comments

Comments

@jaydenseric
Copy link

Describe the bug

<Marker> children of <Cluster> don't re-render on state updates.

To Reproduce

  1. Create a map with clustered markers.
  2. Add a local state boolean to the component containing the map.
  3. Make the marker children conditionally render content based on the boolean.
  4. Add a button to your component that toggles the state boolean.
  5. Click the button, and notice that the children in the makers doesn't re-render and change.

Expected behavior

<Cluster> children should re-render when state changes.

Desktop:

  • OS: [e.g. iOS] macOS
  • Browser [e.g. chrome, safari] Chrome
  • Version [e.g. 22] 81

Additional context

At first I thought it might be because <Cluster> is a React.PureComponent, but when I manually reached into node_modules and changed it to React.Component it made no difference.

@jasonleehodges
Copy link

I am also finding this issue. I have the location of the markers updating periodically and they do not re-render on the map.

@WeZinc
Copy link

WeZinc commented Jun 10, 2020

same here :|

@Bur0
Copy link

Bur0 commented Jul 8, 2020

Did you find a solution ?

@jaydenseric
Copy link
Author

@Bur0 I can't remember all the details, but here is a dump of the components I came up with:

import { MapContext } from '@urbica/react-map-gl'
import PropTypes from 'prop-types'
import { useContext } from 'react'
import useSupercluster from 'use-supercluster'

/**
 * An alternative to the [`@urbica/react-map-gl-cluster`](https://github.com/urbica/react-map-gl-cluster)
 * component [`Cluster`](https://urbica.github.io/react-map-gl-cluster/#/Cluster).
 * @param {object} options Options.
 * @param {Array<object>} options.points Supercluster points; an array of [GeoJSON Feature](https://tools.ietf.org/html/rfc7946#section-3.2) objects. Each feature’s `geometry` must be a [GeoJSON Point](https://tools.ietf.org/html/rfc7946#section-3.1.2).
 * @param {object} [options.options] Supercluster options.
 * @param {Function} options.children React children render function.
 * @returns {ReactNode} React children containing map markers.
 * @see [GitHub issue for `Cluster` children not re-rendering on state updates](https://github.com/urbica/react-map-gl-cluster/issues/31).
 */
export default function MapCluster({ points, options, children }) {
  const map = useContext(MapContext)
  const { supercluster, clusters } = useSupercluster({
    points,
    bounds: map.getBounds().toArray().flat(),
    zoom: map.getZoom(),
    options,
  })

  return children(clusters, supercluster)
}

MapCluster.propTypes = {
  points: PropTypes.arrayOf(
    PropTypes.exact({
      type: PropTypes.oneOf(['Feature']).isRequired,
      geometry: PropTypes.exact({
        type: PropTypes.oneOf(['Point']).isRequired,
        coordinates: PropTypes.arrayOf([PropTypes.number.isRequired])
          .isRequired,
      }),
      properties: PropTypes.object,
    })
  ),
  options: PropTypes.object,
  children: PropTypes.func.isRequired,
}
import { MapContext } from '@urbica/react-map-gl'
import mapboxgl from 'mapbox-gl'
import PropTypes from 'prop-types'
import { PureComponent } from 'react'
import { createPortal } from 'react-dom'

const mapboxPointPropType = PropTypes.exact({
  x: PropTypes.number.isRequired,
  y: PropTypes.number.isRequired,
})

/**
 * This React component is an evolution of the [`@urbica/react-map-gl`](https://github.com/urbica/react-map-gl)
 * component [`Marker`](https://urbica.github.io/react-map-gl/#/Components/Marker):
 *
 * - Removed the static `displayName` property as it gets added via Babel.
 * - Removed the drag related functionality as it’s not needed.
 * - New prop `anchor` that default to `bottom`.
 * - New prop `zIndex`.
 * - Misc. simplifications.
 * @see [GitHub issue to add a `Marker` component `zIndex` prop](https://github.com/urbica/react-map-gl/issues/286).
 */
export default class MapMarker extends PureComponent {
  static defaultProps = {
    anchor: 'bottom',
  }

  static propTypes = {
    latitude: PropTypes.number.isRequired,
    longitude: PropTypes.number.isRequired,
    anchor: PropTypes.oneOf([
      'top-left',
      'top',
      'top-right',
      'right',
      'bottom-right',
      'bottom',
      'bottom-left',
      'left',
      'center',
    ]),
    offset: PropTypes.oneOfType([
      PropTypes.number,
      PropTypes.arrayOf(PropTypes.number.isRequired),
      mapboxPointPropType,
      PropTypes.exact({
        'top-left': mapboxPointPropType.isRequired,
        top: mapboxPointPropType.isRequired,
        'top-right': mapboxPointPropType.isRequired,
        right: mapboxPointPropType.isRequired,
        'bottom-right': mapboxPointPropType.isRequired,
        bottom: mapboxPointPropType.isRequired,
        'bottom-left': mapboxPointPropType.isRequired,
        left: mapboxPointPropType.isRequired,
        center: mapboxPointPropType.isRequired,
      }),
    ]),
    zIndex: PropTypes.number,
    children: PropTypes.node.isRequired,
  }

  static contextType = MapContext

  constructor(props) {
    super(props)
    this.element =
      // eslint-disable-next-line no-undef
      document.createElement('div')
    this.element.style.zIndex = this.props.zIndex
  }

  componentDidMount() {
    this.marker = new mapboxgl.Marker(this.element, {
      offset: this.props.offset,
      anchor: this.props.anchor,
    })
    this.marker
      .setLngLat([this.props.longitude, this.props.latitude])
      .addTo(this.context)
  }

  componentDidUpdate(prevProps) {
    if (
      prevProps.latitude !== this.props.latitude ||
      prevProps.longitude !== this.props.longitude
    )
      this.marker.setLngLat([this.props.longitude, this.props.latitude])

    if (prevProps.zIndex !== this.props.zIndex)
      this.element.style.zIndex = this.props.zIndex
  }

  componentWillUnmount() {
    this.marker.remove()
  }

  getMarker() {
    return this.marker
  }

  render() {
    return createPortal(this.props.children, this.element)
  }
}

I can't remember why, but the MapMarker component wouldn't work as a functional component.

@spencerpauly
Copy link

You can fix this by using a deep compare instead of shallow compare against the old children. I fixed it locally by using deep equal. Also less performant I'm guessing though, so possibly being able to choose only certain fields that you want to compare might be a solution too. Or have a toggle prop to switch between deep or shallow compare for different use-cases.

@spencerpauly
Copy link

How would you like to fix this @stepankuzmin? I can submit a PR w/ the change if you'd like.

@spencerpauly
Copy link

Nevermind. Best way I found to do this was just change the keys of the markers whenever the data in those markers changes. So here's an example:

<Cluster radius={40} extent={512} nodeSize={64} component={ClusterMarker}>
    {points.map(point => (
      <Marker
        key={`${point.id}-${point.isActive.toString()}`}
        longitude={point.geometry.coordinates[0]}
        latitude={point.geometry.coordinates[1]}
      >
        <MyCustomMarkerThatIWantToRerenderWhenActive isActive={point.isActive} />
      </Marker>
    ))}
  </Cluster>

That way it should only redraw these markers whenever one of their "isActive" states changes. This is much more performant than the deep equality check I was doing for a while.

Also useSupercluster uses a deep equality check and it works, but I've found it's really not that performant and gives me odd issues where the markers vanish when I teleport to another part of the map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants