CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.98k stars 3.5k forks source link

TypeScript: createDefaultImageryProviderViewModels() and similar methods are missing #9084

Open sraimund opened 4 years ago

sraimund commented 4 years ago

First of all, thank you for adding TypeScript support. It's really nice to have the API documentation available in the IDEs. When adapting my code to the webpack-cesium-example, I noticed that import { createDefaultImageryProviderViewModels } from "cesium"; works, but the method is not included in the definitions file, thus the IDEs (e.g. WebStorm in the screenshot below) give a warning. I use createDefaultImageryProviderViewModels() to initially change the base map, as mentioned on stackoverflow.

In the bundled Cesium.js file, I saw that several methods (e.g. createMaterialPropertyDescriptor, createUniform) are missing in the TypeScript file, whereas others (e.g. createCommand, createGuid) are present. As I'm new to TypeScript, I cannot tell the reason or even a solution, but it would be helpful, when all exported methods would be included and eventually documented.

webstorm_warning

thw0rted commented 4 years ago

The TypeScript typings are created based on the JSDoc documentation tags that are included for (almost!) all the types and functions in the Cesium public API. Cesium did a great job properly annotating everything so that the plugin used to generate typings could correctly deduce the signature of function calls, class properties/methods, etc, but some parts were incomplete or inaccurate. (I'm not on the team, but I tried to help clean some of this up.)

The problem is, there's no exhaustive way to be certain that the types are accurate, other than creating tests that try to use every part of the API from a TS consumer, so that the type-checker will flag if they're used incorrectly. That would be incredibly time-consuming for a project of this scale, and no one on the core team uses Cesium from TypeScript, so it hasn't happened.

All that said, I just tracked down the source for the function in question and it's not actually missing a JSDoc tag, it's explicitly marked as @private. The team has decided that normal users shouldn't be calling this function, for some reason. You'd have to ask them why.

PS: if y'all are fixing this, please also note that the createCommand function lacks a return type annotation. That's how I came across this issue in the first place.

thw0rted commented 4 years ago

PS to my PS: I looked into it and this is more complex than I thought. Command is one of those cases where Cesium really wanted to have abstract classes before they arrived as a language feature -- as far as I can tell, nobody actually uses the Command function that's defined there, it only exists for the benefit of the doc generator. The problem is that it's tagged @constructor but is not callable, so if you have a Command and try to call it, the type checker will complain.

I tried fixing this by adding an @augments Function tag. This generates the correct typings (class Command extends Function {...}) with tsd-jsdoc so it's callable and has the right properties, but trips up the Typescript compiler due to a known issue where TS can understand @extends/@augments on proper ES6 classes but not ES5 newable functions. This might actually be OK because nobody is compiling the (JS) codebase with tsc -- you just get a red squiggle in VSCode, on a class nobody actually uses, and for some reason tsd-jsdoc doesn't complain.