[Feature Request] Isolate Google Maps data layer usage for easier overrides

[acorncom]

Is your feature request related to a problem? Please describe.
As mentioned in #724 (comment)

Not directly related to your issue, but I noticed that maybe using the global this._map.data object is not the best approach for the adapter and maybe it would be better to 'namespace' Terra Draw data to avoid clashes using new google.maps.Data().

our Google Maps adapter assumes full and complete ownership of the default map.data GeoJson layer in Google Maps when setting up. It would be helpful for future integrations to have more control over how that works as more folks migrate to using TerraDraw from Google Maps as part of the drawing manager deprecation.

Describe your proposed idea for the solution to this problem
The suggestion above was to fully namespace Terra Draw's data layer, presumably either initializing on load

export class TerraDrawGoogleMapsAdapter extends TerraDrawExtend.TerraDrawBaseAdapter {
  constructor(
    config: {
      lib: typeof google.maps;
      map: google.maps.Map;
    } & TerraDrawExtend.BaseAdapterConfig,
  ) {
    super(config);
    this._lib = config.lib;
    this._map = config.map;

    this._data = new google.maps.Data();
    this._data.setMap(this._map);
  }

  private _data: google.maps.Data;
}

or a getter to lazily initialize the data layer

export class TerraDrawGoogleMapsAdapter extends TerraDrawExtend.TerraDrawBaseAdapter {
...

  private _data: google.maps.Data;

  private get data() {
    if (!this._data) {
      this._data = new google.maps.Data();
      this._data.setMap(this._map);
    }
  }

}

Thinking about it, my inclination would be to default to continuing to use the default data layer as we do but set things up to allow consumers the ability to more strictly shift to a fully separate data layer if needed:

export class TerraDrawGoogleMapsAdapter extends TerraDrawExtend.TerraDrawBaseAdapter {

  protected data() {
    return this._map.data;
  }

  // with updates to usage like
  private clearLayers() {
    if (this._layers) {
      this.data().forEach((feature) => {
        const id = feature.getId() as string;
        const hasFeature = this.renderedFeatureIds.has(id);
        if (hasFeature) {
          this.data().remove(feature);
        }
      });
    this.renderedFeatureIds = new Set();
  }

}

This approach allows us to continue working with the existing data layer that's already setup/managed by Google, but gives consumers the ability to more strictly enforce separation and/or has greater access to the internal data library for registering events or the like if their app needs it.

I've done two integrations with TerraDraw in the last few months (both with Google Maps). One of those (a more established app) already used its own distinct data layer for key mapping so the default TD layer approach wasn't an issue. In the second, newer one, we probably would have used the map.data layer for the main drawing surface and shifted TD to drawing on to a different data layer using a custom adapter if that had been an option. Because we didn't, that app left the map.data layer to TD and setup its own data layer to control/manage as well.

Interested in your take!

[binod-adhikari]

Thanks for raising this feature request! We’re currently working on integrating Terra Draw into one of our legacy AngularJS applications, which relies on map.data for managing its data. As you pointed out, the most practical approach at the moment seems to be setting up a separate data layer and allowing Terra Draw to handle map.data directly.

[JamesLMilner]

@acorncom @binod-adhikari thanks both for your input - I was writing a long explanation of the approach I had in mind but thought it might just be easier to share a PR and see what you thought. The gist of it is we default to map.data but provide an option to the adapter to allow an isolated data layer: #756

Thanks!

[acorncom]

@JamesLMilner yeah, I like what you're doing there, especially the use of a protected method to give any subclasses of the adapter the ability to access the isolated data layer if needed. 👍

[JamesLMilner]

@acorncom thanks - it was based off the code you provided. I can merge it soon unless there's any objections here.