alex3165 / react-leaflet-draw

React component for leaflet-draw on top of react-leaflet
228 stars 152 forks source link

`draw.rectangle` equality check in `componentDidUpdate` broken #30

Open vjpr opened 7 years ago

vjpr commented 7 years ago
    if (isEqual(this.props.draw, prevProps.draw) || this.props.position !== prevProps.position) {
      return false;
    }

If draw is set to something, this equality check will fail causing re-rendering every time there is an update.

$ prevProps.draw
rectangle: shapeOptions: clickable: truecolor: "#3388ff"fill: truefillColor: nullfillOpacity: 0.2opacity: 0.5showArea: truestroke: trueweight: 4__proto__: Object__proto__: Object__proto__: Object

$ this.props.draw
rectangle: shapeOptions: {clickable: true}__proto__: Object__proto__: Object

The issue is in updateDrawControls. Looks like a reference to this.props.draw is being passed directly to options.draw which is then mutated by leaflet-draw.

Props should be immutable.

  updateDrawControls = () => {
    const { layerContainer } = this.context;
    const { draw, edit, position } = this.props;
    const options = {
      edit: {
        ...edit,
        featureGroup: layerContainer
      }
    };

    if (draw) {
      options.draw = draw;
    }

    if (position) {
      options.position = position;
    }

    this.leafletElement = new L.Control.Draw(options); // eslint-disable-line
  };
alex3165 commented 7 years ago

Hey @vjpr thanks for the issue, it seems like a critical issue. I am not spending much time maintaining this library to be honest so if you want to go ahead with a fix for this it is welcome. Otherwise I will try to find some time to analyse / fix this later next week.