typescript issues

[jony89]

Regard the new version 1.70 which should officially support TypeScript,

all results properties should be optional

example, if geodesic is instance of EllipsoidGeodesic the following :

geodesic.interpolateUsingSurfaceDistance(distance);

yields error :

error TS2554: Expected 2 arguments, but got 1.

we should be able to pass Color to prop that is a Property

new PolylineDashMaterialProperty({
  color: Color.WHITE,
});

yields the error :

error TS2322: Type 'Color' is not assignable to type 'Property'.

should not we support direct assign ?

if polyline is instance of PolylineGraphic the following :

polyline.width = 12;

yields the error :

error TS2322: Type '12' is not assignable to type 'Property'.

There are many examples of this.

There are now two cesium Typescript definitions

TypeScript community is using https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/cesium. why wouldn't we update the definition there instead of creating two definitions for cesium?

ATM, cesium 1.70 could break projects TypeScript definition, since cesium lib definition takes over the @types/cesium

@mramato

[OmarShehata]

Thanks for bringing up these issues @jony89. For this:

ATM, cesium 1.70 could break projects TypeScript definition, since cesium lib definition takes over the @types/cesium

You can continue to use your own as Matt mentions in this blog post:

If you’re using your own custom definitions or @types/cesium and you’re not yet ready to switch, you can delete Source/Cesium.d.ts after install. TypeScript tooling will then fall back to the next set of CesiumJS type definitions that it finds.

[jony89]

switch, you can delete Source/Cesium.d.ts after install.

I really should not touch node_modules at all. it's generally bad and will make me write some script for the CI process.

[mramato]

@jony89 thank you for taking the time to write up this issue and provide feedback and bug reports!

all results properties should be optional

For EllipsoidGeodesic.interpolateUsingSurfaceDistance, this is true and a bug in our code (technically in the JSDoc annotations). I will open a PR to fix it. However, this is not true for Cesium's API in general, we have a lot of required result parameters.

we should be able to pass Color to prop that is a Property

Absolutely. This is another issue that I will open a PR to fix. We did this for all of the XXXGraphics types, but missed it for these properties.

should not we support direct assign ?

This is an unfortunate limitation of TypeScript.

polyline.width = 12;

needs to be

polyline.width = new ConstantProperty(12);

Here's why: Cesium mimics a capability from C# known as implicit conversion. Basically the internal type of all Entity API properties is Property, but CesiumJS allows you to assign a primitive type and internally it will automatically turn this into a ConstantProperty (for constant values). Even if plain JavaScript where polyline.width = 12 works, typeof polyline.width will not be a number because polyline.width is actually instanceof ConstantProperty. We would love to be able to make the TypeScript type of the setter to be Property|number and the getter to be Property, but TypeScript specifically disallows this. Lots of people have complained about this and I think the closest issue on the TS side is here: microsoft/TypeScript#2521

We want to play by the TS rules as much as possible and make the definition match whatever we would have done if we were writing TS directly. We are looking at ways of turning Property into a generic, but I don't think we'll even be able to make polyline.width = 12 work without the TS compiler adding new features.

TypeScript community is using https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/cesium. why wouldn't we update the definition there instead of creating two definitions for cesium?

At least some of the developers that support @types/cesium also contribute to Cesium and they are all part of the cesium community. Since TypeScript is now officially supported and maintained, it is much easier to keep everything in the cesium repository. It allows us to maintain high quality definitions with as little friction as possible and make sure as Cesium changes, the TS types change in lock step. We also reached out to some of the maintainers of @types/cesium to make sure they could provide feedback to the process and they are welcome to contribute to the new definitions. We did not want to try to "take over" the existing project, but our goal is to make the official TypeScript definitions the best ones available and we fully expect that at some point @types/cesium will be deprecated.

ATM, cesium 1.70 could break projects TypeScript definition, since cesium lib definition takes over the @types/cesium

As @OmarShehata mentioned, deleting the file is one easy way to ensure it isn't used, which you can do with a prepare or postinstall step as part of your build scripts. However, I looked up alternative ways as well. I should have been a little less lazy and included them in the blog and CHANGES.

If you want to continue to use @types/cesium or a custom typings file, you can add the below to your tsconfig.json's compilerOptions settings.

"paths": {
  "cesium": ["node_modules/@types/cesium"]
}

Basically this overrides the types field in the cesium module and points to whatever types you want to use.

Thanks again for your feedback. I'll keep this issue open until the actionable items (the definition issues you brought up) are fixed, hopefully later today.