tiled icon indicating copy to clipboard operation
tiled copied to clipboard

Implement Type Guards for Scripting API

Open TehCupcakes opened this issue 1 year ago • 8 comments

Is your feature request related to a problem? Please describe.

Layers and objects have various boolean properties to indicate what type of instance they are. In TypeScript, these are represented by different types. However, because these are just booleans and not type guard functions, TS cannot do narrowing on its own. That means that even after we run a condition to check isTileMap, intellisense only knows that we have an Asset, and properties on TileMap are not accessible without type errors.

AssetTypeError

Describe the solution you'd like

The API should include functions with type predicates in order to automatically narrow the type when an appropriate value is checked. Changing these to functions could be a breaking change to users (although easy to remediate). I believe the underlying JS implementation could use a getter to maintain backwards compatibility with property syntax (asset.isTileMap) while also allowing function syntax (asset.isTileMap()). However, TS would only allow one to be defined if they share the same name and does not support type predicates on getters, so it would have to be treated as a method in the TS definitions:

isTileMap(): this is TileMap;

TypeGuardSolution

Describe alternatives you've considered

You can work around the less specific type by manually asserting the type after the condition, using as. However, this is not type safe! There is no guarantee that you would get the type which you believe should be coming through. It leaves you susceptible to incorrect logical conditions, as well as not being adaptive for updates to the API which could break the expected type.

AssertTileMapType

I implemented a type-safe workaround by creating the following abstract helper class with type guard functions. This adds some bloat to my extension which could instead be implemented within the scripting API for a better user experience. (In this case I created an abstract class to avoid extending Asset or Layer directly, and just make static calls. The real implementation would put these functions directly on the class prototype, and need not include an input argument.)

export abstract class Guard {
  // ASSET
  static isTileMap(a: Asset): a is TileMap {
    return a.isTileMap;
  }

  static isTileSet(a: Asset): a is Tileset {
    return a.isTileset;
  }

  // LAYER
  static isObjectLayer(l: Layer): l is ObjectGroup {
    return l.isObjectLayer;
  }
  static isGroupLayer(l: Layer): l is GroupLayer {
    return l.isGroupLayer;
  }
  static isImageLayer(l: Layer): l is ImageLayer {
    return l.isImageLayer;
  }
  static isTileLayer(l: Layer): l is TileLayer {
    return l.isTileLayer;
  }
}

TehCupcakes avatar Jul 07 '23 21:07 TehCupcakes