Improve typing of .ts files in /modern (#950)

* Improve typing of Actions

* Make typing of MakiObjects more explicit

* Make parent optional

* Allow group to be initialized with all nulls

* Whitelist all runtime ts files for type checking

* Typecheck the entire modern codebase!

* Ignore typescript

modern/src/Actions.ts

@@ -74,37 +74,42 @@ export function gotSkinZip(zip: JSZip, store: ModernStore): Thunk {
 
     dispatch(setXmlTree(xmlTree));
 
-    const makiTree = await initialize(zip, xmlTree);
+    const makiTree: MakiObject = await initialize(zip, xmlTree);
     // Execute scripts
     await Utils.asyncTreeFlatMap(makiTree, async (node: MakiObject) => {
-      switch (node.name) {
-        case "groupdef": {
-          // removes groupdefs from consideration (only run scripts when actually referenced by group)
-          return {};
-        }
-        case "script": {
-          // TODO: stop ignoring standardframe
-          if (node.attributes.file.endsWith("standardframe.maki")) {
-            return node;
-          }
-          const scriptGroup = Utils.findParentNodeOfType(
-            node,
-            new Set(["group", "WinampAbstractionLayer", "WasabiXML"])
-          );
-          node.system = new System(scriptGroup, store);
-          const script = await Utils.readUint8array(zip, node.attributes.file);
-          run({
-            runtime,
-            data: script,
-            system: node.system,
-            log: false,
-          });
-          return node;
-        }
-        default: {
-          return node;
-        }
+      if (!(node instanceof JsScript)) {
+        return node;
       }
+      const scriptPath = node.getScriptPath();
+      if (scriptPath == null) {
+        return node;
+      }
+      if (scriptPath.endsWith("standardframe.maki")) {
+        // TODO: stop ignoring standardframe
+        return node;
+      }
+
+      // removes groupdefs from consideration (only run scripts when actually referenced by group)
+      if (Utils.findParentNodeOfType(node, new Set(["groupdef"]))) {
+        return node;
+      }
+      const scriptGroup = Utils.findParentNodeOfType(
+        node,
+        new Set(["group", "winampabstractionlayer", "wasabixml"])
+      );
+      node.system = new System(scriptGroup, store);
+      const script = await Utils.readUint8array(zip, scriptPath);
+      if (script == null) {
+        console.warn(`Unable to find script at ${scriptPath}`);
+        return node;
+      }
+      run({
+        runtime,
+        data: script,
+        system: node.system,
+        log: false,
+      });
+      return node;
     });
 
     dispatch(setMakiTree(makiTree));

modern/src/runtime/GuiObject.ts

@@ -33,7 +33,11 @@ class GuiObject extends MakiObject {
   _transitionParams: TransitionParams;
   _targetAnimationStartTime: number;
   _targetAnimationCancelID: number | null;
-  constructor(node: XmlNode, parent: MakiObject, annotations: Object = {}) {
+  constructor(
+    node: XmlNode | null,
+    parent: MakiObject | null,
+    annotations: Object = {}
+  ) {
     super(node, parent, annotations);
 
     this._setAttributeDefaults();

modern/src/runtime/JsScript.ts

@@ -13,6 +13,11 @@ class JsScript extends MakiObject {
   getclassname() {
     return "Script";
   }
+
+  getScriptPath(): string | null {
+    const { file } = this.attributes;
+    return file == null ? null : String(file);
+  }
 }
 
 export default JsScript;

modern/src/runtime/MakiObject.ts

@@ -10,14 +10,14 @@ class MakiObject {
   // type conversion differently. Having one type that holds both the pre and
   // post type coerced values is too confusing.
   attributes: { [key: string]: string | number | boolean | undefined };
-  parent: MakiObject;
+  parent: MakiObject | null;
   _emitter: Emitter;
   children: MakiObject[];
   js_annotations: Object;
 
   constructor(
     node: XmlNode | null,
-    parent: MakiObject,
+    parent: MakiObject | null,
     annotations: Object = {}
   ) {
     if (node) {
@@ -61,6 +61,9 @@ class MakiObject {
   }
 
   js_delete() {
+    if (this.parent == null) {
+      return;
+    }
     this.parent.js_removeChild(this);
     this.parent.js_trigger("js_update");
   }

modern/src/runtime/PopupMenu.ts

@@ -17,9 +17,12 @@ type Command =
 class PopupMenu extends MakiObject {
   _commands: Command[];
   _guiParent: Group;
+  parent: MakiObject;
   js_selectCommand: (id: number) => void;
+
   constructor(node: XmlNode, parent: MakiObject, annotations: Object) {
     super(node, parent, annotations);
+    this.parent = parent;
 
     if (!(parent instanceof Group)) {
       throw new Error(

modern/src/runtime/System.ts

@@ -12,16 +12,17 @@ import Layout from "./Layout";
 import GuiObject from "./GuiObject";
 
 class System extends MakiObject {
-  _scriptGroup: Group;
+  _scriptGroup: MakiObject;
   _root: MakiObject;
   _store: ModernStore;
   _privateInt: Map<string, Map<string, number>>;
   _privateString: Map<string, Map<string, string>>;
-  constructor(scriptGroup: Group | null, store: ModernStore) {
+  constructor(scriptGroup: MakiObject | null, store: ModernStore) {
     super(null, null);
     this._store = store;
 
-    this._scriptGroup = scriptGroup == null ? new Group() : scriptGroup;
+    this._scriptGroup =
+      scriptGroup == null ? new Group(null, null) : scriptGroup;
     this._root = this._scriptGroup;
     while (this._root.parent) {
       this._root = this._root.parent;
@@ -121,6 +122,7 @@ class System extends MakiObject {
   getprivateint(section: string, item: string, defvalue: number): number {
     if (
       !this._privateInt.has(section) ||
+      // @ts-ignore We know this section exists
       !this._privateInt.get(section).has(item)
     ) {
       return defvalue;
@@ -635,6 +637,7 @@ class System extends MakiObject {
   getprivatestring(section: string, item: string, defvalue: string): string {
     if (
       !this._privateString.has(section) ||
+      // @ts-ignore We know this section exists
       !this._privateString.get(section).has(item)
     ) {
       return defvalue;

modern/src/utils.ts

@@ -226,16 +226,18 @@ export function findParent<T extends { parent: T | null }>(
 }
 
 // Operations on trees
-export function findParentNodeOfType<
-  T extends { parent: T | null; name: string }
->(node: T, type: Set<string>): T | null {
-  return findParent(node, n => type.has(n.name));
+export function findParentNodeOfType(
+  node: MakiObject,
+  type: Set<string>
+): MakiObject | null {
+  return findParent(node, n => type.has(n.name.toLowerCase()));
 }
 
-export function findParentOrCurrentNodeOfType<
-  T extends { parent: T | null; name: string }
->(node: T, type: Set<string>): T | null {
-  if (type.has(node.name)) {
+export function findParentOrCurrentNodeOfType(
+  node: MakiObject,
+  type: Set<string>
+): MakiObject | null {
+  if (type.has(node.name.toLowerCase())) {
     return node;
   }
   return findParentNodeOfType(node, type);
@@ -285,9 +287,7 @@ function findDirectDescendantById<
   );
 }
 
-function* iterateLexicalScope<
-  T extends { children: T[]; parent: T | undefined }
->(node: T): IterableIterator<T> {
+function* iterateLexicalScope(node: MakiObject): IterableIterator<MakiObject> {
   let currentNode = node;
   while (currentNode.parent) {
     const { parent } = currentNode;
@@ -304,10 +304,10 @@ function* iterateLexicalScope<
 }
 
 // Search up the tree for a node that is in `node`'s lexical that matches `predicate`.
-function findInLexicalScope<T extends { children: T[]; parent: T | undefined }>(
-  node: T,
-  predicate: (node: T) => boolean
-): T | null {
+function findInLexicalScope(
+  node: MakiObject,
+  predicate: (node: MakiObject) => boolean
+): MakiObject | null {
   for (const child of iterateLexicalScope(node)) {
     if (predicate(child)) {
       return child;

tsconfig.json

@@ -16,67 +16,13 @@
     "js/**/*.js",
     "js/**/*.ts",
     "js/**/*.tsx",
-    "modern/src/Emitter.ts",
-    "modern/src/runtime/AlbumArtLayer.ts",
-    "modern/src/runtime/AnimatedLayer.ts",
-    "modern/src/runtime/BitList.ts",
-    "modern/src/runtime/Browser.ts",
-    "modern/src/runtime/Button.ts",
-    "modern/src/runtime/CfgGroup.ts",
-    "modern/src/runtime/CheckBox.ts",
-    "modern/src/runtime/Component.ts",
-    "modern/src/runtime/ComponentBucket.ts",
-    "modern/src/runtime/Config.ts",
-    "modern/src/runtime/ConfigAttribute.ts",
-    "modern/src/runtime/ConfigItem.ts",
-    "modern/src/runtime/Container.ts",
-    "modern/src/runtime/DropDownList.ts",
-    "modern/src/runtime/Edit.ts",
-    "modern/src/runtime/EqVis.ts",
-    "modern/src/runtime/FeedWatcher.ts",
-    "modern/src/runtime/Form.ts",
-    "modern/src/runtime/Frame.ts",
-    "modern/src/runtime/Group.ts",
-    "modern/src/runtime/GroupList.ts",
-    "modern/src/runtime/GuiList.ts",
-    "modern/src/runtime/GuiObject.ts",
-    "modern/src/runtime/GuiTree.ts",
-    "modern/src/runtime/JsElements.ts",
-    "modern/src/runtime/JsGammaSet.ts",
-    "modern/src/runtime/JsGroupDef.ts",
-    "modern/src/runtime/JsWinampAbstractionLayer.ts",
-    "modern/src/runtime/Layer.ts",
-    "modern/src/runtime/Layout.ts",
-    "modern/src/runtime/LayoutStatus.ts",
-    "modern/src/runtime/List.ts",
-    "modern/src/runtime/MakiObject.ts",
-    "modern/src/runtime/Map.ts",
-    "modern/src/runtime/Menu.ts",
-    "modern/src/runtime/MenuButton.ts",
-    "modern/src/runtime/MouseRedir.ts",
-    "modern/src/runtime/PlDir.ts",
-    "modern/src/runtime/PlEdit.ts",
-    "modern/src/runtime/PopupMenu.ts",
-    "modern/src/runtime/QueryList.ts",
-    "modern/src/runtime/Region.ts",
-    "modern/src/runtime/Slider.ts",
-    "modern/src/runtime/Status.ts",
-    // "modern/src/runtime/System.ts",
-    "modern/src/runtime/TabSheet.ts",
-    "modern/src/runtime/Text.ts",
-    "modern/src/runtime/Timer.ts",
-    "modern/src/runtime/Title.ts",
-    "modern/src/runtime/ToggleButton.ts",
-    "modern/src/runtime/TreeItem.ts",
-    "modern/src/runtime/Vis.ts",
-    "modern/src/runtime/Wac.ts",
-    "modern/src/runtime/WindowHolder.ts",
-    // "modern/src/Actions.ts",
-    "modern/src/utils.ts",
-    "modern/src/initializeStateTree.ts",
-    "modern/src/Selectors.ts",
-    // "modern/src/Store.ts",
-    "modern/src/types.ts"
+    "modern/**/*.ts",
+    "modern/**/*.tsx"
   ],
-  "exclude": ["node_modules", "**/*.spec.ts", "demo/built"]
+  "exclude": [
+    "node_modules",
+    "**/*.spec.ts",
+    "demo/built",
+    "modern/src/Dashboard.tsx"
+  ]
 }