From 2d7db715c270d1bd08e88f321efdee21b7f8aee6 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Thu, 19 Sep 2019 07:03:31 -0700 Subject: [PATCH] Start typechecking more runtime objects --- modern/src/runtime/AnimatedLayer.ts | 15 +-- modern/src/runtime/Group.ts | 2 +- modern/src/runtime/GuiObject.ts | 141 +++++++++++++++------------- modern/src/runtime/Layout.ts | 4 +- modern/src/runtime/MakiObject.ts | 5 +- modern/src/types.ts | 2 +- modern/src/utils.ts | 2 +- tsconfig.json | 5 +- 8 files changed, 100 insertions(+), 76 deletions(-) diff --git a/modern/src/runtime/AnimatedLayer.ts b/modern/src/runtime/AnimatedLayer.ts index ec92fe05..9f6eabda 100644 --- a/modern/src/runtime/AnimatedLayer.ts +++ b/modern/src/runtime/AnimatedLayer.ts @@ -9,9 +9,9 @@ class AnimatedLayer extends Layer { constructor(node, parent, annotations, store) { super(node, parent, annotations, store); - this._setAttributeDefaults(this.attributes); - this._convertAttributeTypes(this.attributes); - this._initializeStartAndEnd(this.attributes); + this._setAttributeDefaults(); + this._convertAttributeTypes(); + this._initializeStartAndEnd(); this._playing = this.attributes.autoplay; this._frameNum = this.attributes.start || 0; @@ -20,7 +20,8 @@ class AnimatedLayer extends Layer { this._setupAnimationLoop(); } - _setAttributeDefaults(attributes: Object): void { + _setAttributeDefaults(): void { + const { attributes } = this; if (attributes.autoplay == null) { attributes.autoplay = "0"; } @@ -32,7 +33,8 @@ class AnimatedLayer extends Layer { } } - _convertAttributeTypes(attributes: Object): void { + _convertAttributeTypes(): void { + const { attributes } = this; if (attributes.autoplay != null) { attributes.autoplay = !!Number(attributes.autoplay); } @@ -50,7 +52,8 @@ class AnimatedLayer extends Layer { } } - _initializeStartAndEnd(attributes: Object): void { + _initializeStartAndEnd(): void { + const { attributes } = this; if (attributes.start != null && attributes.end != null) { return; } diff --git a/modern/src/runtime/Group.ts b/modern/src/runtime/Group.ts index a71d8e5c..b98f74fc 100644 --- a/modern/src/runtime/Group.ts +++ b/modern/src/runtime/Group.ts @@ -27,7 +27,7 @@ class Group extends GuiObject { return; } - oncreateobject(newobj): void { + oncreateobject(newobj: GuiObject): void { this.js_trigger("onCreateObject", newobj); } diff --git a/modern/src/runtime/GuiObject.ts b/modern/src/runtime/GuiObject.ts index 4394fdfc..efbe07c1 100644 --- a/modern/src/runtime/GuiObject.ts +++ b/modern/src/runtime/GuiObject.ts @@ -4,45 +4,71 @@ import { findParentNodeOfType, unimplementedWarning, } from "../utils"; -import * as MakiSelectors from "../MakiSelectors"; +import { ModernStore, XmlNode } from "../types"; + +type TargetParams = { + alpha?: number; + x?: number; + y?: number; + w?: number; + h?: number; +}; + +type TransitionParams = { + alpha?: { start: number; end: number }; + x?: { start: number; end: number }; + y?: { start: number; end: number }; + w?: { start: number; end: number }; + h?: { start: number; end: number }; +}; + +type AnimationPropKeys = "alpha" | "x" | "y" | "w" | "h"; + +const ANIMATION_PROP_KEYS: AnimationPropKeys[] = ["alpha", "x", "y", "w", "h"]; class GuiObject extends MakiObject { visible: boolean; _targetAnimationSpeed: number; - _startParams: Object; - _targetParams: Object; - _transitionParams: Object; + _targetParams: TargetParams; + _transitionParams: TransitionParams; _targetAnimationStartTime: number; - _targetAnimationCancelID: number; - constructor(node, parent, annotations, store) { + _targetAnimationCancelID: number | null; + constructor( + node: XmlNode, + parent: MakiObject, + annotations: Object = {}, + store: ModernStore + ) { super(node, parent, annotations, store); - this._setAttributeDefaults(this.attributes); - this._convertAttributeTypes(this.attributes); + this._setAttributeDefaults(); + this._convertAttributeTypes(); this.visible = true; - this._startParams = {}; + this._targetAnimationSpeed = 0; this._targetParams = {}; this._transitionParams = {}; - this._selectorCache = new Map(); + this._targetAnimationStartTime = 0; + this._targetAnimationCancelID = null; } - _setAttributeDefaults(attributes) { - if (attributes.alpha == null) { - attributes.alpha = "255"; + _setAttributeDefaults() { + if (this.attributes.alpha == null) { + this.attributes.alpha = "255"; } - if (attributes.x == null) { - attributes.x = "0"; + if (this.attributes.x == null) { + this.attributes.x = "0"; } - if (attributes.y == null) { - attributes.y = "0"; + if (this.attributes.y == null) { + this.attributes.y = "0"; } - if (attributes.ghost == null) { - attributes.ghost = "0"; + if (this.attributes.ghost == null) { + this.attributes.ghost = "0"; } } - _convertAttributeTypes(attributes) { + _convertAttributeTypes() { + const { attributes } = this; if (attributes.alpha != null) { attributes.alpha = Number(attributes.alpha); } @@ -57,24 +83,6 @@ class GuiObject extends MakiObject { } } - _useUidSelector(selector) { - // TODO: use memoize for this - if (!this._selectorCache.has(selector)) { - this._selectorCache.set(selector, selector(this._uid)); - } - return this._selectorCache.get(selector)(this._store.getState()); - } - - _compareToUidSelector(value, selector) { - const selectorValue = this._useUidSelector(selector); - if (selectorValue !== value) { - console.error( - `Maki state ${value} is out of sync with tree state ${selectorValue}` - ); - } - return value; - } - /** * getclassname() * @@ -86,10 +94,11 @@ class GuiObject extends MakiObject { } findobject(id: string) { - return findDescendantByTypeAndId(this, null, id); + const self: MakiObject = this; + return findDescendantByTypeAndId(self, null, id); } - init(newRoot): void { + init(newRoot: MakiObject): void { this.parent = newRoot; newRoot.js_addChild(this); } @@ -107,8 +116,10 @@ class GuiObject extends MakiObject { return this.parent; } - getparentlayout() { - return findParentNodeOfType(this, new Set(["layout"])); + // TODO: Make this return a `Layout` once `Layout` is typed. + getparentlayout(): MakiObject | null { + const self: MakiObject = this; + return findParentNodeOfType(self, new Set(["layout"])); } show(): void { @@ -122,10 +133,7 @@ class GuiObject extends MakiObject { } gettop(): number { - return this._compareToUidSelector( - Number(this.attributes.y) || 0, - MakiSelectors.getTop - ); + return Number(this.attributes.y) || 0; } getleft(): number { @@ -270,17 +278,17 @@ class GuiObject extends MakiObject { (currentTime - this._targetAnimationStartTime) / this._targetAnimationSpeed; if (progress > 1) { - this._startParams = {}; this._targetParams = {}; this.ontargetreached(); return; } - ["alpha", "x", "y", "w", "h"].forEach(attr => { - if (this._transitionParams[attr]) { - this.attributes[attr] = - this._targetParams[attr] * progress + - this._startParams[attr] * (1 - progress); + ANIMATION_PROP_KEYS.forEach(attr => { + const transition = this._transitionParams[attr]; + if (transition == null) { + return; } + const { start, end } = transition; + this.attributes[attr] = end * progress + start * (1 - progress); }); this.js_trigger("js_update"); this._targetAnimationLoop(); @@ -290,14 +298,18 @@ class GuiObject extends MakiObject { gototarget(): void { this._transitionParams = {}; - ["alpha", "x", "y", "w", "h"].forEach(attr => { - if (this._targetParams[attr] != null) { - this._transitionParams[attr] = true; - this._startParams[attr] = - this.attributes[attr] != null - ? this.attributes[attr] - : this._targetParams[attr]; + ANIMATION_PROP_KEYS.forEach(attr => { + const target = this._targetParams[attr]; + if (target == null) { + return; } + + const attribute = this.attributes[attr]; + + this._transitionParams[attr] = { + start: attribute != null ? Number(attribute) : target, + end: target, + }; }); this._targetAnimationStartTime = window.performance.now(); @@ -309,9 +321,10 @@ class GuiObject extends MakiObject { } canceltarget(): void { - window.cancelAnimationFrame(this._targetAnimationCancelID); + if (this._targetAnimationCancelID != null) { + window.cancelAnimationFrame(this._targetAnimationCancelID); + } this._targetAnimationCancelID = null; - this._startParams = {}; this._targetParams = {}; } @@ -339,12 +352,12 @@ class GuiObject extends MakiObject { return; } - bringabove(guiobj) { + bringabove(guiobj: GuiObject) { unimplementedWarning("bringabove"); return; } - bringbelow(guiobj) { + bringbelow(guiobj: GuiObject) { unimplementedWarning("bringbelow"); return; } @@ -529,7 +542,7 @@ class GuiObject extends MakiObject { y: number, p1: number, p2: number, - source + source: GuiObject ): number { unimplementedWarning("onaction"); this.js_trigger("onAction", action, param, x, y, p1, p2, source); diff --git a/modern/src/runtime/Layout.ts b/modern/src/runtime/Layout.ts index acb67787..58543ccd 100644 --- a/modern/src/runtime/Layout.ts +++ b/modern/src/runtime/Layout.ts @@ -1,5 +1,6 @@ import Group from "./Group"; import { findParentNodeOfType, unimplementedWarning } from "../utils"; +import MakiObject from "./MakiObject"; class Layout extends Group { /** @@ -13,7 +14,8 @@ class Layout extends Group { } getcontainer() { - return findParentNodeOfType(this, new Set(["container"])); + const self: MakiObject = this; + return findParentNodeOfType(self, new Set(["container"])); } ondock(): void { diff --git a/modern/src/runtime/MakiObject.ts b/modern/src/runtime/MakiObject.ts index b2069415..3a7843e1 100644 --- a/modern/src/runtime/MakiObject.ts +++ b/modern/src/runtime/MakiObject.ts @@ -7,7 +7,10 @@ class MakiObject { name: string; _uid: number; _store: ModernStore; - attributes: Object; + // TODO: This should really just be `string | undefined` and we should handle + // 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; _emitter: Emitter; children: MakiObject[]; diff --git a/modern/src/types.ts b/modern/src/types.ts index 5f7b4a72..72b17009 100644 --- a/modern/src/types.ts +++ b/modern/src/types.ts @@ -3,7 +3,7 @@ export type MakiTree = any; export type XmlNode = { children: XmlNode[]; - attributes: { id?: string; file?: string }; + attributes: { [key: string]: string | undefined }; name: string; uid: number; // TODO: They don't start with these. Do we need types for each stage? }; diff --git a/modern/src/utils.ts b/modern/src/utils.ts index 3120b805..76108c0a 100644 --- a/modern/src/utils.ts +++ b/modern/src/utils.ts @@ -243,7 +243,7 @@ export function findParentOrCurrentNodeOfType< export function findDescendantByTypeAndId< T extends { children: T[]; name: string; attributes?: { id?: string } } ->(node: T, type: string, id: string): T | null { +>(node: T, type: string | null, id: string): T | null { if (node.children.length === 0) { return null; } diff --git a/tsconfig.json b/tsconfig.json index 75bb7f4e..279aff7f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,7 +18,10 @@ "js/**/*.tsx", "modern/src/utils.ts", "modern/src/Emitter.ts", - "modern/src/runtime/MakiObject.ts" + "modern/src/runtime/MakiObject.ts", + "modern/src/runtime/GuiObject.ts", + "modern/src/runtime/Layout.ts", + "modern/src/runtime/Group.ts" ], "exclude": ["node_modules", "**/*.spec.ts", "demo/built"] }