[JS] Typings of the thing model

[vavido]

When accessing the properties of a feature in typescript, the compiler complains that

TS2339: Property 'xxx' does not exist on type 'object'.

The quickest workaround here is to add an //@ts-ignore above the line, but that can get bloated quickly.

Another quick solution would be to change the return type of get properies() in the Feature class to any.
A more "proper" approach could be to set the return type to a dictonary, (I assume the properties object is always a dictionary with string keys and arbitrary entries?), but that only moves the problem down one layer in the hierarchy.

What are your thoughts on this?

[vavido]

As a somewhat related sitenote, the current API of the Thing class is a bit weird, too. To access features of a thing, thing.features?.features? is needed because the features field is of Features type, which then conains a dictonary features that has the actual features.
Tbh, I don't fully understand the purpose of the Features class and the IndexedEntityModel class it inherits from, but assuming this hierarchy layer can not simply be removed, how about returning this._features.features; inside the features getter of the thing?

[ffendt]

I like the idea of pushing the "problem" down one layer for properties. The API knows it's a dict, so it can also show it like this. The layer below is domain logic of users and needs to be mapped to the correct data type for that domain by the user.

You're absolutely right about the Features object (and all other classes extending IndexedEntityModel). If we were about to change the features, I would suggest to also change all other occurrences of IndexedEntityModel. A simple solution would be to make the Features class an utility class with just static methods to parse to and from object. The currently known FeaturesType would be the concrete type stored in thing._features.
Would be nice though if there was only one interface Features combining the static methods and the dict. The last time I checked, I didn't find a suiteable and nice solution for adding static methods to an interface. Do you know of any?

[vavido]

Normally I would suggest just creating a class instead of an interface, but I think in the case of index signature types like FeatureType there is no way to do this.
Nvm, in theory one should just be able to add an index signature to a class. However, in the case of the Features class, this conflicts with the _elements property of the superclass IndexedEntityModel

I would advocate for simply creating seperate data classes/interfaces and static mapper functions/classes, as I personally find the current approach with things like class IndexedEntityModel<T extends IndexedEntityModel<T, V>, V> extends EntityModel<IndexedEntityModel<T, V>> very hard to understand.

[vavido]

I thought some more about it and I theoretically like the idea of having the toObject/fromObject methods directly on the corresponding classes, but in this case this leads to an extra class (Feature) that has essentially no purpose other than mapping objects but now also gets dragged along into the class hierarchy.
@ffendt What do you think about seperate mapper & data classes?

[ffendt]

Another idea that came up my mind was to build this like we have in the java client. That would be accessing each Feature in Features by a getter (getFeature(featureId)). But actually I like traversing the path with dot-notation.
So from my point of view it would be a good option to use separate mapper & data classes for the classes that currently implement IndexedEntityModel.

[vavido]

Hi @ffendt, thanks for your input, I'll try to create a clean mapper/data implementation and create a PR here soon

[vavido]

@ffendt I think I found a way to tidy up the sctructure without a need to seperate Mappers and Entity classes:
Typescript allows static methods on a record type, that means the IndexedEntityModel can be written as

export abstract class IndexedEntityModel<EntryType> implements Record<string, EntryType> {
  [key: string]: EntryType;
  
   static fromPlainObject<T>(...): IndexedEntityModel<T> | undefined {

And the concrete classes like this:

export class Features extends IndexedEntityModel<Feature> {

  public static fromObject(o: any): Features | undefined {
    if (o === undefined) {
      return o;
    }
    return IndexedEntityModel.fromPlainObject<Feature>(o, Feature.fromObject);
  }
}

So this means that calls like features.toObject() need to be refactored to Features.toObject(features).
What do you think about that?