Merge pull request #7283 from zoran995/fix/cesium-type-augmentation

fix: augment cesium types and use import instead of require in ts files
TerriaJS · Sep 30, 2024 · 0d9a8ef · 0d9a8ef
2 parents fa7c7f0 + a9a9083
commit 0d9a8ef
Show file tree

Hide file tree

Showing 15 changed files with 82 additions and 70 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -8,6 +8,7 @@
   - Error services now instantiated externally to terriajs
 
 - Fix remaining lint warnings
+- Augment cesium types and start using import instead of require in ts files
 - [The next improvement]
 
 #### 8.7.6 - 2024-08-22

diff --git a/architecture/0002-require-instead-of-import.md b/architecture/0002-require-instead-of-import.md
@@ -4,7 +4,7 @@ Date: 2020-08-07 recorded, decision made earlier
 
 ## Status
 
-Accepted
+Superseded by [0012-do-not-use-require-for-cesium-in-ts](./0012-do-not-use-require-for-cesium-in-ts.md) on 2024-09-28
 
 ## Context
 

diff --git a/architecture/0010-never-support-ie11.md b/architecture/0010-never-support-ie11.md
@@ -4,7 +4,7 @@ Date: 2022-05-12
 
 ## Status
 
-Proposed
+Accepted
 
 ## Context
 

diff --git a/architecture/0011-configurable-search-providers.md b/architecture/0011-configurable-search-providers.md
@@ -4,7 +4,7 @@ Date: 2021-01-19
 
 ## Status
 
-Proposed
+Accepted
 
 ## Context
 

diff --git a/architecture/0012-do-not-use-require-for-cesium-in-ts.md b/architecture/0012-do-not-use-require-for-cesium-in-ts.md
@@ -0,0 +1,34 @@
+# 2. Using `imports` instead of `require` + type casting statements
+
+Date: 2024-09-28
+
+## Status
+
+Proposed
+
+## Context
+
+A decision was made in [0002-require-instead-of-import](./0002-require-instead-of-import.md) to use non-exported terriajs-cesium features using `require` + type casting statements due to inability to augment namespace and enum imports.
+
+Recently we found a new way to augment terriajs-cesium typescript definition in case of enums and namespaces. When we directly augment `terriajs-module` typescript will properly augment the namespaces and enums can be augmented as namespaces making everything works as should on type level.
+
+```ts
+declare module terriajs-cesium {
+  namespace FeatureDetection {
+    function isChrome(): boolean;
+    function isEdge(): boolean;
+    function isInternetExplorer(): boolean;
+    function isFirefox(): boolean;
+    function internetExplorerVersion(): number[];
+    function chromeVersion(): number[];
+  }
+
+  namespace Axis {
+    function fromName(name: string): number;
+  }
+}
+```
+
+## Decission
+
+Augment `terriajs-cesium` type definition
diff --git a/lib/Map/Leaflet/LeafletDataSourceDisplay.ts b/lib/Map/Leaflet/LeafletDataSourceDisplay.ts
@@ -1,21 +1,19 @@
+import L from "leaflet";
 import BoundingSphere from "terriajs-cesium/Source/Core/BoundingSphere";
+import EventHelper from "terriajs-cesium/Source/Core/EventHelper";
+import JulianDate from "terriajs-cesium/Source/Core/JulianDate";
+import createGuid from "terriajs-cesium/Source/Core/createGuid";
+import defaultValue from "terriajs-cesium/Source/Core/defaultValue";
+import destroyObject from "terriajs-cesium/Source/Core/destroyObject";
 import BoundingSphereState from "terriajs-cesium/Source/DataSources/BoundingSphereState";
 import CustomDataSource from "terriajs-cesium/Source/DataSources/CustomDataSource";
 import DataSource from "terriajs-cesium/Source/DataSources/DataSource";
 import DataSourceCollection from "terriajs-cesium/Source/DataSources/DataSourceCollection";
-import defaultValue from "terriajs-cesium/Source/Core/defaultValue";
 import Entity from "terriajs-cesium/Source/DataSources/Entity";
 import EntityCluster from "terriajs-cesium/Source/DataSources/EntityCluster";
-import EventHelper from "terriajs-cesium/Source/Core/EventHelper";
 import isDefined from "../../Core/isDefined";
-import JulianDate from "terriajs-cesium/Source/Core/JulianDate";
-import L from "leaflet";
 import LeafletScene from "./LeafletScene";
 
-const createGuid = require("terriajs-cesium/Source/Core/createGuid").default;
-const destroyObject =
-  require("terriajs-cesium/Source/Core/destroyObject").default;
-
 interface Visualizer {
   update(time: JulianDate): boolean;
   destroy(): void;

diff --git a/lib/Map/Leaflet/LeafletSelectionIndicator.ts b/lib/Map/Leaflet/LeafletSelectionIndicator.ts
@@ -1,26 +1,19 @@
+import L from "leaflet";
 import Cartographic from "terriajs-cesium/Source/Core/Cartographic";
-import CesiumMath from "terriajs-cesium/Source/Core/Math";
 import EasingFunction from "terriajs-cesium/Source/Core/EasingFunction";
 import Ellipsoid from "terriajs-cesium/Source/Core/Ellipsoid";
-import L from "leaflet";
+import CesiumMath from "terriajs-cesium/Source/Core/Math";
+import TweenCollection from "terriajs-cesium/Source/Scene/TweenCollection";
 import isDefined from "../../Core/isDefined";
 
 import Leaflet from "../../Models/Leaflet";
 
-const TweenCollection =
-  require("terriajs-cesium/Source/Scene/TweenCollection").default;
 const selectionIndicatorUrl = require("../../../wwwroot/images/NM-LocationTarget.svg");
 
 interface Tween {
   cancelTween(): void;
 }
 
-interface TweenCollection {
-  length: number;
-  add(args: any): Tween;
-  update(): void;
-}
-
 const cartographicScratch = new Cartographic();
 
 export default class LeafletSelectionIndicator {

diff --git a/lib/Map/Leaflet/LeafletVisualizer.ts b/lib/Map/Leaflet/LeafletVisualizer.ts
@@ -1,28 +1,25 @@
+import L, { LatLngBounds, LatLngBoundsLiteral, PolylineOptions } from "leaflet";
 import AssociativeArray from "terriajs-cesium/Source/Core/AssociativeArray";
 import Cartesian2 from "terriajs-cesium/Source/Core/Cartesian2";
 import Cartesian3 from "terriajs-cesium/Source/Core/Cartesian3";
-import CesiumMath from "terriajs-cesium/Source/Core/Math";
 import Color from "terriajs-cesium/Source/Core/Color";
-import DataSource from "terriajs-cesium/Source/DataSources/DataSource";
+import destroyObject from "terriajs-cesium/Source/Core/destroyObject";
 import Ellipsoid from "terriajs-cesium/Source/Core/Ellipsoid";
-import Entity from "terriajs-cesium/Source/DataSources/Entity";
-import EntityCollection from "terriajs-cesium/Source/DataSources/EntityCollection";
-import EntityCluster from "terriajs-cesium/Source/DataSources/EntityCluster";
-import isDefined from "../../Core/isDefined";
 import JulianDate from "terriajs-cesium/Source/Core/JulianDate";
-import L, { LatLngBounds, PolylineOptions, LatLngBoundsLiteral } from "leaflet";
-import LeafletScene from "./LeafletScene";
+import CesiumMath from "terriajs-cesium/Source/Core/Math";
 import PolygonHierarchy from "terriajs-cesium/Source/Core/PolygonHierarchy";
-import PolylineGlowMaterialProperty from "terriajs-cesium/Source/DataSources/PolylineGlowMaterialProperty";
+import Rectangle from "terriajs-cesium/Source/Core/Rectangle";
+import writeTextToCanvas from "terriajs-cesium/Source/Core/writeTextToCanvas";
+import DataSource from "terriajs-cesium/Source/DataSources/DataSource";
+import Entity from "terriajs-cesium/Source/DataSources/Entity";
+import EntityCluster from "terriajs-cesium/Source/DataSources/EntityCluster";
+import EntityCollection from "terriajs-cesium/Source/DataSources/EntityCollection";
 import PolylineDashMaterialProperty from "terriajs-cesium/Source/DataSources/PolylineDashMaterialProperty";
+import PolylineGlowMaterialProperty from "terriajs-cesium/Source/DataSources/PolylineGlowMaterialProperty";
 import Property from "terriajs-cesium/Source/DataSources/Property";
-import Rectangle from "terriajs-cesium/Source/Core/Rectangle";
+import isDefined from "../../Core/isDefined";
 import { getLineStyleLeaflet } from "../../Models/Catalog/Esri/esriLineStyle";
-
-const destroyObject =
-  require("terriajs-cesium/Source/Core/destroyObject").default;
-const writeTextToCanvas =
-  require("terriajs-cesium/Source/Core/writeTextToCanvas").default;
+import LeafletScene from "./LeafletScene";
 
 interface PointDetails {
   layer?: L.CircleMarker;
@@ -1345,7 +1342,7 @@ function convertEntityPositionsToLatLons(positions: Cartesian3[]): L.LatLng[] {
 export default class LeafletVisualizer {
   visualizersCallback(
     leafletScene: LeafletScene,
-    _entityCluster: EntityCluster,
+    _entityCluster: EntityCluster | undefined,
     dataSource: DataSource
   ) {
     const entities = dataSource.entities;

diff --git a/lib/ModelMixins/GltfMixin.ts b/lib/ModelMixins/GltfMixin.ts
@@ -9,6 +9,7 @@ import ConstantProperty from "terriajs-cesium/Source/DataSources/ConstantPropert
 import CustomDataSource from "terriajs-cesium/Source/DataSources/CustomDataSource";
 import Entity from "terriajs-cesium/Source/DataSources/Entity";
 import ModelGraphics from "terriajs-cesium/Source/DataSources/ModelGraphics";
+import Axis from "terriajs-cesium/Source/Scene/Axis";
 import HeightReference from "terriajs-cesium/Source/Scene/HeightReference";
 import AbstractConstructor from "../Core/AbstractConstructor";
 import proxyCatalogItemUrl from "../Models/Catalog/proxyCatalogItemUrl";
@@ -19,11 +20,6 @@ import MappableMixin from "./MappableMixin";
 import ShadowMixin from "./ShadowMixin";
 import Resource from "terriajs-cesium/Source/Core/Resource";
 
-// We want TS to look at the type declared in lib/ThirdParty/terriajs-cesium-extra/index.d.ts
-// and import doesn't allows us to do that, so instead we use require + type casting to ensure
-// we still maintain the type checking, without TS screaming with errors
-const Axis: Axis = require("terriajs-cesium/Source/Scene/Axis").default;
-
 type BaseType = Model<GltfTraits>;
 
 export interface GltfTransformationJson {

diff --git a/lib/Models/AugmentedVirtuality.ts b/lib/Models/AugmentedVirtuality.ts
@@ -1,22 +1,20 @@
 import {
   action,
   computed,
+  makeObservable,
   observable,
-  runInAction,
-  makeObservable
+  runInAction
 } from "mobx";
 import CesiumCartesian3 from "terriajs-cesium/Source/Core/Cartesian3";
 import Cartographic from "terriajs-cesium/Source/Core/Cartographic";
 import EllipsoidTerrainProvider from "terriajs-cesium/Source/Core/EllipsoidTerrainProvider";
 import CesiumMath from "terriajs-cesium/Source/Core/Math";
 import CesiumMatrix3 from "terriajs-cesium/Source/Core/Matrix3";
+import sampleTerrainMostDetailed from "terriajs-cesium/Source/Core/sampleTerrainMostDetailed";
 import Camera from "terriajs-cesium/Source/Scene/Camera";
 import Scene from "terriajs-cesium/Source/Scene/Scene";
 import Terria from "./Terria";
 
-const sampleTerrainMostDetailed =
-  require("terriajs-cesium/Source/Core/sampleTerrainMostDetailed").default;
-
 interface EventLoopState {
   intervalId?: any;
 }

diff --git a/lib/Models/Catalog/Gtfs/GtfsCatalogItem.ts b/lib/Models/Catalog/Gtfs/GtfsCatalogItem.ts
@@ -22,6 +22,7 @@ import Entity from "terriajs-cesium/Source/DataSources/Entity";
 import ModelGraphics from "terriajs-cesium/Source/DataSources/ModelGraphics";
 import PointGraphics from "terriajs-cesium/Source/DataSources/PointGraphics";
 import PropertyBag from "terriajs-cesium/Source/DataSources/PropertyBag";
+import Axis from "terriajs-cesium/Source/Scene/Axis";
 import ColorBlendMode from "terriajs-cesium/Source/Scene/ColorBlendMode";
 import HeightReference from "terriajs-cesium/Source/Scene/HeightReference";
 import ShadowMode from "terriajs-cesium/Source/Scene/ShadowMode";
@@ -49,11 +50,6 @@ import {
 import prettyPrintGtfsEntityField from "./prettyPrintGtfsEntityField";
 import VehicleData from "./VehicleData";
 
-// We want TS to look at the type declared in lib/ThirdParty/terriajs-cesium-extra/index.d.ts
-// and import doesn't allows us to do that, so instead we use require + type casting to ensure
-// we still maintain the type checking, without TS screaming with errors
-const Axis: Axis = require("terriajs-cesium/Source/Scene/Axis").default;
-
 interface RectangleExtent {
   east: number;
   south: number;

diff --git a/lib/Models/Catalog/Ows/WebProcessingServiceCatalogFunctionJob.ts b/lib/Models/Catalog/Ows/WebProcessingServiceCatalogFunctionJob.ts
@@ -10,6 +10,7 @@ import {
   toJS
 } from "mobx";
 import Mustache from "mustache";
+import createGuid from "terriajs-cesium/Source/Core/createGuid";
 import URI from "urijs";
 import { JsonObject } from "../../../Core/Json";
 import TerriaError from "../../../Core/TerriaError";
@@ -36,8 +37,6 @@ import proxyCatalogItemUrl from "../proxyCatalogItemUrl";
 
 const executeWpsTemplate = require("./ExecuteWpsTemplate.xml");
 
-const createGuid = require("terriajs-cesium/Source/Core/createGuid").default;
-
 class WpsLoadableStratum extends LoadableStratum(
   WebProcessingServiceCatalogFunctionJobTraits
 ) {

diff --git a/lib/Models/Cesium.ts b/lib/Models/Cesium.ts
@@ -197,7 +197,7 @@ export default class Cesium extends GlobeOrMap {
 
     // Workaround for Firefox bug with WebGL and printing:
     // https://bugzilla.mozilla.org/show_bug.cgi?id=976173
-    const firefoxBugOptions = (FeatureDetection as any).isFirefox()
+    const firefoxBugOptions = FeatureDetection.isFirefox()
       ? {
           contextOptions: {
             webgl: { preserveDrawingBuffer: true }

diff --git a/lib/Models/Leaflet.ts b/lib/Models/Leaflet.ts
@@ -167,7 +167,7 @@ export default class Leaflet extends GlobeOrMap {
     this.dataSourceDisplay = new LeafletDataSourceDisplay({
       scene: this.scene,
       dataSourceCollection: this.dataSources,
-      visualizersCallback: this._leafletVisualizer.visualizersCallback as any // TODO: fix type error
+      visualizersCallback: this._leafletVisualizer.visualizersCallback
     });
 
     this._eventHelper = new EventHelper();

diff --git a/lib/ThirdParty/terriajs-cesium-extra/index.d.ts b/lib/ThirdParty/terriajs-cesium-extra/index.d.ts
@@ -2,6 +2,7 @@ declare module "terriajs-cesium/Source/Scene/TweenCollection" {
   export default class TweenCollection {
     get length(): number;
     add(options: any): any;
+    update(time?: number): void;
   }
 }
 
@@ -44,25 +45,24 @@ declare module "terriajs-cesium/Source/Core/PolygonGeometryLibrary";
 
 declare module "terriajs-cesium/Source/DataSources/getElement";
 
-declare interface Axis {
-  X: number;
-  Y: number;
-  Z: number;
-  fromName(name: string): number;
-}
-
-declare interface FeatureDetection {
-  isEdge(): boolean;
-  isInternetExplorer(): boolean;
-  internetExplorerVersion(): number[];
-  chromeVersion(): number[];
-}
-
 // This is a workaround for Cesium's incorrect type declaration for raiseEvent.
 declare module "terriajs-cesium" {
   export interface Event {
     raiseEvent(...arguments: any[]): void;
   }
+
+  namespace FeatureDetection {
+    function isChrome(): boolean;
+    function isEdge(): boolean;
+    function isInternetExplorer(): boolean;
+    function isFirefox(): boolean;
+    function internetExplorerVersion(): number[];
+    function chromeVersion(): number[];
+  }
+
+  namespace Axis {
+    function fromName(name: string): number;
+  }
 }
 
 // Begin Generated Declarations