Date: 2020-08-07 recorded, decision made earlier
Superseded by 0012-do-not-use-require-for-cesium-in-ts on 2024-09-28
Cesium recently implemented their own typescript definitions that replaces the old community-run @types/cesium. As a result we've also stopped using our own typescript definitions (previously defined in lib/ThirdParty/terriajs-cesium/index.d.ts). However we've encountered a few problems when importing some of the declared modules in Cesium.d.ts in terriajs-cesium in that there are occasions where a function is not included as part of the declaration in Cesium.d.ts and so requires module augmentation.
Here's an example (note I'll be using this example all throughout the document to explain the methods I've tried and other relevant details): looking at lib/Models/GltfCatalogItem.ts, if we import the module Axis from terriajs-cesium using the standard way (import Axis from 'terriajs-cesium/Source/Scene/Axis') we face the following error:
Property 'fromName' does not exist on type 'typeof Axis'.
for parts of the code that uses the fromName function, such as (line 57):
return Axis.fromName(this.upAxis);This error comes up because the module declared in Cesium.d.ts only exports the enum X, Y and Z. So parts of the code that references Axis.Y, or one of the other enums, would work just fine, but other parts of the code that reference functions defined in terriajs-cesium/Source/Scene/Axis but not in the module, such as Axis.fromName, would break TS type checking, even though technically everything works fine in good old JS.
-
Kevin had tried to merge the enums declared in
Cesium.d.tsinto a locally declared class in the following way:declare module "terriajs-cesium/Source/Scene/Axis" { export default class Axis { static fromName(name: string): number; } }
but as a result of this, we face the error
TypeError: /home/regina/release/TerriaMap-mobx/packages/terriajs/lib/Models/GltfCatalogItem.ts: Duplicate declaration "Axis"and also another error relating to after digging through this problem we found that we can't do a declaration merge on classes, which Kevin confirmed:
Kevin Ring 2:18 PM hmm seems classes can't be declaration merged with classes. TIL
I've also tried to locally declare an Axis module as a class consisting of the enum types
X,Y,Zand thefromNamefunction. This didn't work because class declarations require a function implementation for every function defined in the class. Given that the functions are already implemented in Cesium it would be silly to have to re-implement the function again in this repo for various reasons - so this wasn't a good way of solving the problem. -
I've tried to use namespaces instead of using classes to see if this would fix the problem, so I tried the following:
declare module "terriajs-cesium/Source/Scene/Axis" { export default namespace Axis { let X: number; let Y: number; let Z: number; function fromName(name: string): number; } }
So I tried declaring the above in
lib/ThirdParty/terriajs-cesium-extra/index.d.ts(due to needing to refer to theAxisnamespace in two different filesGltfCatalogItem.tsandGtfsCatalogItem.ts). I found that TS can't use namespaces as a type and screams the following error:Cannot use namespace 'Axis' as a type.Given that this problem is mainly a type checking problem, rather than an implementation problem (i.e. everything runs fine if we were to use
require, which by default casts it to typeany), this isn't a viable solution. -
The final method I tried was to use interfaces, which was the suggested method based on the TS declaration merging docs. Even though the modules we are importing aren't actually interfaces (
Axisis supposed to be a class I believe), it still somewhat made sense given that we are only trying to tell TS that the functionfromNameexists in the moduleAxis, and this could be done using an interface (also +1 for not having to re-implement the function, unlike declaring a new class). So I created a declaration of the interfaceAxisinlib/ThirdParty/terriajs-cesium-extra/index.d.tsas follows:declare interface Axis { X: number; Y: number; Z: number; fromName(name: string): number; }
This is where I found out that when importing modules using
import Axis from "...", TS will not make use of the Axis interface declared inindex.d.ts, and instead retains the type that was declared in terriajs-cesium (Cesium.d.ts). This results in the Axis object to still be of typeenumand I face the same problem that I encountered above (exact same errors). This is where I figured usingrequireinstead would do the trick, because it allows us to cast the Axis object to be of type Axis interface that's declared in ourindex.d.ts.
We'll use require over import for very special cases such as these, because we want TS to look at the type declared in lib/ThirdParty/terriajs-cesium-extra/index.d.ts. Given that require by default will cast the object to type any, we also should cast the object to the desired type/interface declared in index.d.ts.
- Inconsistency - we have to use two different ways of importing. Maybe it might(?) undesirably encourage people to use
requireto easily avoid type checking errors, even though it should really only be used if there are no other ways to fix the problem. And hopefully people don't just see it as "an alternative" way of importing because we should still encourage usingimportoverrequire - We have to define the
interfacealong with its fields and methods names ourselves, rather than relying on Cesium's up-to-date type definitions, which means we would need to (manually?) keep it up-to-date with the Cesium's implementations. There would be a problem, say, if they remove thefromNamefunction from theAxisclass and this isn't reflected in our type declarations file - our code becomes technically incorrect and would throw runtime errors, but TS would not detect this.