dlr-eoc / ukis-frontend-libraries

A collection of angular ui-components, services, interfaces... to help you create geospatial mapping applications for the web.
Apache License 2.0
17 stars 4 forks source link

Make certain at runtime that layers added to LayersService are actually instances of `Layer` #127

Open MichaelLangbein opened 2 years ago

MichaelLangbein commented 2 years ago

Description

Consider this case:

const layer1 = new RasterLayer(paras);
const layer2 = { ... layer1 };        // This is a common mistake. What user should have done here is `new RasterLayer(layer1);`
layersService.add(layer1);
layersService.add(layer2);             // no error here
layer2.visible = !layer2.visible;
layersService.updateLayer(layer2);    // error here

This causes an error:

layer with id: <....> you want to update not in storeItems!

This error is thrown because updateLayer calls isInLayergroups which checks if the passed layer is an instance of Layer

if (layergroup instanceof Layer || layergroup instanceof LayerGroup)

Since layer2 was created using object-spread, it does not pass this test.

Relevant Package

This feature request is for @dlr-eoc/services-layers

Describe alternatives you've considered

boeckMt commented 2 years ago

Creating instances and Shallow-cloning objets is a different thing which should not be mistaken.

But yes we could check this on add Layer, and give hint like ... is not an instance of Layer

Also in my VS Code I get the hint

Argument of type '{ type: TRasterLayertype; url: string; subdomains?: string[]; params?: IRasterLayerParams; tileSize?: number; name: string; id: string; opacity: number; visible: boolean; ... 20 more ...; cssClass?: string; }' is not assignable to parameter of type 'Layer'. Property 'time' is missing in type '{ type: TRasterLayertype; url: string; subdomains?: string[]; params?: IRasterLayerParams; tileSize?: number; name: string; id: string; opacity: number; visible: boolean; ... 20 more ...; cssClass?: string; }' but required in type 'Layer'.

So I can't add the layer.


Feel free to do a PR :)

This could be somewhere in isInLayergroups because this function is called before add Layer or Group.

public isInLayergroups(layergroup: Layer | LayerGroup | string, groups?: Array<Layer | LayerGroup>): boolean{
...
  if (layergroup instanceof Layer || layergroup instanceof LayerGroup) {
      id = layergroup.id;
    } else {
      id = layergroup;
    }
...
}

I can't remember why we allowed to add strings here... If we remove it we could use the else for this check.


We could also use this opportunity to do https://github.com/dlr-eoc/ukis-frontend-libraries/issues/74

FYI @voinSR @Stef-Firestone @lucas-angermann

boeckMt commented 2 years ago

Depends on: https://github.com/dlr-eoc/ukis-frontend-libraries/issues/74