Deep comparison of parameters prop in Layer

[felixpalmer]

For #6750

Change List

• Addition of equal method for object propType
• parameters prop marked for deep comparison in Layer

[coveralls]

Coverage increased (+0.003%) to 82.066% when pulling d68d9f8 on felix/prop-type-object into f84198c on master.

[ibgreen]

In the functional programming model, one relies on a new object being minted when something changes, so shallow compare is enough.

deepEqual has some perf impact. That said the diffing cost for params objects would typically be small, and this does remove a possible "gotcha" for non-functional programmers.

Do we as a general rule want to support mutable properties across the api? Are there other props that need the same treatment.

[Pessimistress]

@ibgreen We do deep comparison in a few other cases, for example color values (number[4]). In this particular case Kepler could probably switch to using a constant parameters object to avoid the update. But there is a more general issue in TileLayer. When diffProps returns a false positive, a normal layer will just perform an unnecessary redraw, which is not terrible. TileLayer on the other hand will invalidate all layer cache and rerender all sublayers, which can be expensive.

I believe the problem lies in the original design that TileLayer automatically forwards all of its props to renderSubLayers. It is therefore impossible to determine if an arbitrary prop change is meant to affect the tile layer itself (tile fetching and culling), or the generation of sub layers. Since renderSubLayers may return anything, we cannot pre-define the prop types of TileLayer to offer the same level of optimization as other official layers.

A "proper" fix that aligns the TileLayer behavior with other layers is to avoid exposing TileLayer to the sub layer props:

/// BAD
new TileLayer({
  data: ...,
  getTileData: ...,
  maxZoom: 14,

  parameters: {},
  getLineWidth: f => ...,
  getFillColor: f => ...,
  getLineColor: lineColor,
  getPointRadius: 4,

  renderSubLayers: props => new GeoJsonLayer(props) 
});

/// GOOD
new TileLayer({
  data: ...,
  getTileData: ...,
  maxZoom: 14,

  renderSubLayers: props => new GeoJsonLayer(props, {
    parameters: {},
    getLineWidth: f => ...,
    getFillColor: f => ...,
    getLineColor: lineColor,
    getPointRadius: 4
  }),
  updateTriggers: {
    renderSubLayers: [lineColor]
  }
});

Both approach work with the current release, but the second is much more performant in React with managed view states.

We can update the documentation and examples to promote the latter, and deprecate the first use case in the next major release.

[ibgreen]

True, there is a concern about false positives (newly minted object with same props), as well as false negatives (same object with mutated props) which I was thinking about.

Yes perhaps it is be better to go all-in on deep comparisons. Could that allow us to not have to recommend such custom renderSubLayers (that seems tedious and error prone for end users).

[felixpalmer]

Would it make sense in that case to have the renderSubLayers be an abstract method to force the user to provide an implementation? If we are recommending one pattern over another, I think it is best to be explicit.