Typescript errors when setting color of PointGraphics


As can be seen in the screendump, typescript complains when trying to set the color of a PointGraphics object directly, and is happy when wrapping the color in a new ColorMaterialProperty.
However the latter doesn’t work, the point has no color, and the former will work despite the error that is displayed. (which is still a problem since the application won’t start properly when there are tsc errors)
Is there a problem with the typescript definitions?
I’m using Cesium 1.83 in an Angular 10 project.

Update: I can “fix” this by changing line 22184 in the file node_modules/cesium/Source/Cesium.d.ts
to: color: Color | Property | undefined;
A similar issue occurs with BillBoardGraphics.

@markchagers

Thank you for the information. Out of curiosity, does a similar “fix” work for BillBoardGraphics?

-Sam

Yep that works also.
Obviously that is not a real fix. There seems to be a disconnect here between the official typescript definitions and the actual functionality implemented by the CesiumJS lib. Having the latter behave the way the typescript definitions suggest would be preferable IMO, but failing that, loosening the type requirements for the color property would at least give us a useable workaround.
Should I file a bug somewhere for this?
Update: After more investigation, it now seems to be a common problem with all “properties” that accept a color. (i.e. LabelGraphics.fillColor)

update2: I have submitted an issue in github: Typescript errors when setting color properties of various Cesium objects · Issue #9667 · CesiumGS/cesium · GitHub

@markchagers

Thank you for your thorough work and for submitting an issue on GitHub. We currently have a lot of issues that we want to prioritize for our next release but we will get to this issue ASAP. However, I would be happy to review a pull request if you have the time and energy to submit a solution to the problem.

-Sam

I’d be happy to do that, but i’d need a little guidance on the best way to fix this.
Specifically: will color properties generally accept only a Color value, or is the intent to have them accept a ColorMaterialProperty or either? Currently a ColorMaterialProperty doesn’t work.

@markchagers

Color should accept a color value (r,g,b,a). ColorMaterialProperty accepts a Color, which it maps to solid color Material uniforms.

-Sam

@sam.rothstein I think changing the type annotation in the LabelGraphics.js file fixes my issue, since the typescript definition file is generated from the jsDoc entries:

  /**
   * Gets or sets the Property specifying the fill {@link Color}.
   * @memberof LabelGraphics.prototype
   * @type {Color|undefined}  // <= EDITED LINE
   */
  fillColor: createPropertyDescriptor("fillColor"),

However that raises a few questions:
1 Should the typescript annotations be fixed to correspond with the implementation (as above), or should the implementation be fixed to comply with the type annotations?
2 does this affect anything else in the Cesium code (I’m not familiar enough with the code base to be sure I don’t break something accidentally).
3 Doesn’t it interfere with any plans to fix the actual implementation so that using a ColorMaterialProperty here will work?
4 How many other properties are there in all of Cesium where a similar situation occurs?

@markchagers

Thank you for the questions. I am going to check in with a few other Cesium developers and get back to you as soon as possible.

-Sam

@sam.rothstein I have created a pull request PR for the simplest solution: changing the type annotations so that the expected value for these properties is a Cesium.Color.
Going the other route: changing the implementation to accept a property as suggested by the type annotations may be a better solution but is a bit beyond my expertise at the moment.

1 Like