Logic around properties $watch prevents source update if opacity AND visible are not set.

[cramatt]

Overview

The following user case will create a situation where a GeoJson layer does not update it's source when expected.

Use case

Consider the following use case:

<openlayers width="100%" height="450px">
  <ol-layer ng-if="vm.geoJson.source.geojson" ol-layer-properties="vm.geoJson"></ol-layer>
</openlayers>

function setAndUpdateGeoJson(data) {
  vm.geoJson = {
    name: 'A Geo Layer',
    source: {
      type: 'GeoJSON',
      geojson: {
        object: data
      }
    }
  };
}

setAndUpdateGeoJson(<FeatureCollection 1>)

setAndUpdateGeoJson(<FeatureCollection 2>)

Expected behavior

The map updates with the new GeoJson source.

Actual behavior

The map does not update.

Why?

This happens because of the logic around how properties are watched. When the watch callback fires, Openlayers directive first checks if two properties (opacity and visible) are defined. If visible is not defined, it's set to default value true and the watch callback returns, preventing any layer creation of updating. Since the properties object is now modified, the watcher fires again, and this time the callback checks if opacity is defined. If not, again it's set to default value 1 and returned early. The properties watcher now fires a 3rd time.

The problem is that now the watcher callback arguments, oldProperties and newProperties, now have the same source, and the logic here if (isDefined(oldProperties) && !equals(properties.source, oldProperties.source)) { evaluates to true, preventing the layer from being updated.

How to fix option 1

Set opacity and visible properties explicitly.

function setAndUpdateGeoJson(data) {
  vm.geoJson = {
    name: 'A Geo Layer',
    visible: true,
    opacity: 1,
    source: {
      type: 'GeoJSON',
      geojson: {
        object: data
      }
    }
  };
}

How to fix option 2

Break out the set and update function.

function setGeoJson(data) {
  vm.geoJson = {
    name: 'A Geo Layer',
    source: {
      type: 'GeoJSON',
      geojson: {
        object: data
      }
    }
  };
}

function updateGeoJson() {
  vm.geoJson.source.geojson = data;
}

What to do ?

I'm not actually sure this is a bug, it's more of a gotcha which I wanted to document for others who encounter it.

That being said, is it necessary to have the returns here and here? Seems like removing them would still work and would prevent this issue.