Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Sim.js to TypeScript #795

Closed
pixelzoom opened this issue Mar 14, 2022 · 15 comments
Closed

Port Sim.js to TypeScript #795

pixelzoom opened this issue Mar 14, 2022 · 15 comments

Comments

@pixelzoom
Copy link
Contributor

I started on this, and ran into a whole bunch of places to add TODOs... Missing doc, use-before-definiton, etc. TODOs require an associated GitHub issue, so this is it.

@pixelzoom pixelzoom self-assigned this Mar 14, 2022
@pixelzoom
Copy link
Contributor Author

After 3+ hours trying to port Sim.js... I feel like I've had to make too many changes - symptomatic of the fact that Sim.js is a mess. And my sim is not behaving properly. The PhET menu is not layered correctly. And there may be other problems.

Here's a patch, in case someone else want to start from where I left off. Good luck.

Patch
Index: chipper/phet-types.d.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/phet-types.d.ts b/chipper/phet-types.d.ts
--- a/chipper/phet-types.d.ts	(revision 57fc66cc1632cb45bc644a6c19b82837d64fdc22)
+++ b/chipper/phet-types.d.ts	(date 1647282555785)
@@ -18,6 +18,14 @@
 declare var assertSlow: undefined | ( ( x: any, s?: string ) => void );
 declare var sceneryLog: undefined | any;
 declare var phet: any;
+declare var phetSplashScreen: { dispose: () => void };
+declare var phetSplashScreenDownloadComplete: () => void;
+declare var TWEEN: { update: ( dt: number ) => void };
+declare var phetio: {
+  PhetioIDUtils: {
+    HOME_SCREEN_COMPONENT_NAME: string
+  }
+};
 
 type RoundPushButtonOptions = {
   soundPlayer?: SoundClipPlayer,
@@ -59,7 +67,8 @@
 
 type QSMType = {
   getAll: ( a: any ) => any,
-  containsKey: ( key: string ) => boolean
+  containsKey: ( key: string ) => boolean,
+  warnings: { key: string, value: any, message: string }[]
 };
 
 declare var QueryStringMachine: QSMType;
\ No newline at end of file
Index: joist/js/Screen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/Screen.ts b/joist/js/Screen.ts
--- a/joist/js/Screen.ts	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/Screen.ts	(date 1647282210705)
@@ -20,7 +20,7 @@
 import { Shape } from '../../kite/js/imports.js';
 import optionize from '../../phet-core/js/optionize.js';
 import StringUtils from '../../phetcommon/js/util/StringUtils.js';
-import { Color, Path, Rectangle, Node } from '../../scenery/js/imports.js';
+import { IColor, Node, Path, Rectangle } from '../../scenery/js/imports.js';
 import PhetioObject, { PhetioObjectOptions } from '../../tandem/js/PhetioObject.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import IOType from '../../tandem/js/types/IOType.js';
@@ -51,7 +51,7 @@
 type ScreenSelfOptions = {
   name?: string | null;
   instrumentNameProperty?: true;
-  backgroundColorProperty?: Property<string> | Property<Color>;
+  backgroundColorProperty?: Property<IColor>;
   homeScreenIcon?: ScreenIcon | null;
   showUnselectedHomeScreenIconFrame?: boolean;
   navigationBarIcon?: ScreenIcon | null;
@@ -64,20 +64,20 @@
 
 // Parameterized on M=Model and V=View
 class Screen<M, V extends ScreenView> extends PhetioObject {
-  backgroundColorProperty: Property<string> | Property<Color>;
-  private readonly nameProperty: IReadOnlyProperty<string | null>;
+  backgroundColorProperty: Property<IColor>;
+  public readonly nameProperty: IReadOnlyProperty<string | null>;
   private readonly showScreenIconFrameForNavigationBarFill: string | null;
   private readonly homeScreenIcon: Node | null;
   private readonly navigationBarIcon: Node | null;
   private readonly showUnselectedHomeScreenIconFrame: boolean;
   private readonly keyboardHelpNode: Node | null; // joist-internal
   private readonly pdomDisplayNameProperty: DerivedProperty<string | null, [ string | null ]>;
-  private readonly maxDT: number;
+  public readonly maxDT: number;
   private readonly createModel: () => M;
   private readonly createView: ( model: M ) => V;
   private _model: M | null;
   private _view: V | null;
-  private readonly activeProperty: BooleanProperty;
+  public readonly activeProperty: BooleanProperty;
   public readonly descriptionContent: string;
 
   static HOME_SCREEN_ICON_ASPECT_RATIO: number;
@@ -98,7 +98,7 @@
       instrumentNameProperty: true,
 
       // {Property.<Color|string>} background color of the Screen
-      backgroundColorProperty: new Property( 'white' ),
+      backgroundColorProperty: new Property<IColor>( 'white' ),
 
       // {Node|null} icon shown on the home screen. If null, then a default is created.
       // For single-screen sims, there is no home screen and the default is OK.
Index: joist/js/ScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/ScreenView.ts b/joist/js/ScreenView.ts
--- a/joist/js/ScreenView.ts	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/ScreenView.ts	(date 1647282838046)
@@ -298,6 +298,12 @@
       0, 0, 1
     );
   }
+
+  /**
+   * Steps the view.
+   * @param dt - time step, in seconds
+   */
+  step( dt: number ): void {}
 }
 
 ScreenView.DEFAULT_LAYOUT_BOUNDS = DEFAULT_LAYOUT_BOUNDS;
Index: joist/js/selectScreens.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/selectScreens.js b/joist/js/selectScreens.js
--- a/joist/js/selectScreens.js	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/selectScreens.js	(date 1647282655001)
@@ -1,6 +1,7 @@
 // Copyright 2020-2021, University of Colorado Boulder
 
 import joist from './joist.js';
+import Screen from './Screen.js'; // eslint-disable-line
 
 /**
  * Given an array of all possible screens that a sim can have, select and order them according to the relevant query
@@ -21,7 +22,7 @@
  * @param {number[]} screensQueryParameter - from phet.chipper.queryParameters.screens
  * @param {boolean} screensQueryParameterProvided
  * @param {function( selectedSimScreens ):HomeScreen} createHomeScreen
- * @returns {{homeScreen:HomeScreen|null, initialScreen:Screen, selectedSimScreens:Screen[], screens:Screen[]}} - duck-typed for tests
+ * @returns {{homeScreen:HomeScreen|null, initialScreen:Screen, selectedSimScreens:Screen[], screens:Screen[], allScreensCreated: boolean}} - duck-typed for tests
  * @throws Error if incompatible data is provided
  */
 const selectScreens = ( allSimScreens,
Index: joist/js/SimDisplay.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/SimDisplay.js b/joist/js/SimDisplay.js
--- a/joist/js/SimDisplay.js	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/SimDisplay.js	(date 1647278866435)
@@ -166,7 +166,7 @@
 
   /**
    * Handle synthetic input event fuzzing
-   * @private
+   * @public
    */
   fuzzInputEvents() {
 
Index: joist/js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts
--- a/joist/js/Sim.ts	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/Sim.ts	(date 1647282838055)
@@ -1,5 +1,7 @@
 // Copyright 2013-2022, University of Colorado Boulder
-// @ts-nocheck
+
+//TODO https://github.com/phetsims/joist/issues/795 revisit uses of '!'
+
 /**
  * Main class that represents one simulation.
  * Provides default initialization, such as polyfills as well.
@@ -12,9 +14,10 @@
 
 import animationFrameTimer from '../../axon/js/animationFrameTimer.js';
 import BooleanProperty from '../../axon/js/BooleanProperty.js';
-import createObservableArray from '../../axon/js/createObservableArray.js';
+import createObservableArray, { ObservableArray } from '../../axon/js/createObservableArray.js';
 import DerivedProperty from '../../axon/js/DerivedProperty.js';
 import Emitter from '../../axon/js/Emitter.js';
+import IReadOnlyProperty from '../../axon/js/IReadOnlyProperty.js';
 import NumberProperty from '../../axon/js/NumberProperty.js';
 import Property from '../../axon/js/Property.js';
 import stepTimer from '../../axon/js/stepTimer.js';
@@ -24,14 +27,16 @@
 import Random from '../../dot/js/Random.js';
 import DotUtils from '../../dot/js/Utils.js'; // eslint-disable-line default-import-match-filename
 import merge from '../../phet-core/js/merge.js';
+import optionize from '../../phet-core/js/optionize.js';
 import platform from '../../phet-core/js/platform.js';
+import PickOptional from '../../phet-core/js/types/PickOptional.js';
 import StringUtils from '../../phetcommon/js/util/StringUtils.js';
 import BarrierRectangle from '../../scenery-phet/js/BarrierRectangle.js';
 import { animatedPanZoomSingleton, globalKeyStateTracker, Node, Utils, voicingUtteranceQueue } from '../../scenery/js/imports.js';
 import '../../sherpa/lib/game-up-camera-1.0.0.js';
 import soundManager from '../../tambo/js/soundManager.js';
 import PhetioAction from '../../tandem/js/PhetioAction.js';
-import PhetioObject from '../../tandem/js/PhetioObject.js';
+import PhetioObject, { PhetioObjectOptions } from '../../tandem/js/PhetioObject.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import NumberIO from '../../tandem/js/types/NumberIO.js';
 import audioManager from './audioManager.js';
@@ -46,12 +51,14 @@
 import MemoryMonitor from './MemoryMonitor.js';
 import NavigationBar from './NavigationBar.js';
 import packageJSON from './packageJSON.js';
+import PreferencesConfiguration from './preferences/PreferencesConfiguration.js';
 import PreferencesManager from './preferences/PreferencesManager.js';
 import Profiler from './Profiler.js';
 import QueryParametersWarningDialog from './QueryParametersWarningDialog.js';
 import Screen from './Screen.js';
 import ScreenSelectionSoundGenerator from './ScreenSelectionSoundGenerator.js';
 import ScreenshotGenerator from './ScreenshotGenerator.js';
+import ScreenView from './ScreenView.js';
 import selectScreens from './selectScreens.js';
 import SimDisplay from './SimDisplay.js';
 import SimInfo from './SimInfo.js';
@@ -80,54 +87,221 @@
 
 assert && assert( typeof phet.chipper.brand === 'string', 'phet.chipper.brand is required to run a sim' );
 
+//TODO https://github.com/phetsims/joist/issues/795 any because we don't have a model base class or interface
+type AnyScreen = Screen<any, ScreenView>;
+
+type ModalNode = Node & {
+  hide: () => void;
+  layout: ( bounds: Bounds2 ) => void;
+};
+
+type SelfOptions = {
+
+  // credits, see AboutDialog for format
+  credits: any; //TODO https://github.com/phetsims/joist/issues/795 need a type for credits object literal
+
+  // creates the content for the Options dialog
+  createOptionsDialogContent: null | ( ( tandem: Tandem ) => Node );
+
+  // a {Node} placed onto the home screen
+  homeScreenWarningNode: null | Node;
+
+  // true when this sim supports a keyboard help button on the navigation bar that shows keyboard help
+  // content. This content is specific to each screen, see Screen.keyboardHelpNode for more info.
+  hasKeyboardHelpContent: boolean;
+
+  // If a PreferencesConfiguration is provided the sim will include the PreferencesDialog and a button in the
+  // NavigationBar to open it.
+  preferencesConfiguration: PreferencesConfiguration | null;
+
+  // Passed to SimDisplay, but a top level option for API ease.
+  webgl: boolean;
+
+  // Passed to the SimDisplay
+  simDisplayOptions: any; //TODO https://github.com/phetsims/joist/issues/795 SimDisplayOptions
+
+} & PickOptional<PhetioObject, 'phetioState' | 'phetioReadOnly' | 'tandem'>;
+
+type SimOptions = SelfOptions & PhetioObjectOptions;
+
 class Sim extends PhetioObject {
 
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly simNameProperty: Property<string>; // joist-internal
+
+  // the name displayed by the sim at runtime
+  public readonly displayedSimNameProperty: IReadOnlyProperty<string>;
+
+  // Indicates that sim construction has completed, and that all screen models and views have been created.
+  // This was added for PhET-iO but can be used by any client. This does not coincide with the end of the Sim
+  // constructor, because Sim has asynchronous steps that finish after the constructor is completed.
+  public readonly isConstructionCompleteProperty: Property<boolean>;
+
+  // Stores the effective window dimensions that the simulation will be taking up
+  public readonly dimensionProperty: Property<Dimension2>;
+
+  // When the sim is active, scenery processes inputs and stepSimulation(dt) runs from the system clock.
+  // Set to false for when the sim will be paused.
+  public readonly activeProperty: Property<boolean>;
+
+  // Indicates whether the browser tab containing the simulation is currently visible
+  public readonly browserTabVisibleProperty: Property<boolean>;
+
+  // Indicates that PhET-iO is currently setting the state of the simulation.
+  // See PhetioStateEngine for details. This must be declared before soundManager.initialized is called.
+  public readonly isSettingPhetioStateProperty: IReadOnlyProperty<boolean>;
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly screenProperty: Property<AnyScreen>;
+
+  // joist-internal
+  // How the home screen and navbar are scaled. This scale is based on the
+  // HomeScreen's layout bounds to support a consistently sized nav bar and menu. If this scale was based on the
+  // layout bounds of the current screen, there could be differences in the nav bar across screens.
+  public readonly scaleProperty: Property<number>;
+
+  // joist-internal
+  // global bounds for the entire simulation. null before first resize.
+  public readonly boundsProperty: Property<Bounds2 | null>;
+
+  // joist-internal, read-only
+  // global bounds for the screen-specific part (excludes the navigation bar), null before first resize
+  public readonly screenBoundsProperty: Property<Bounds2 | null>;
+
+  // Action that indicates when the sim resized. This Action is implemented so it can be automatically played back.
+  public readonly resizeAction: PhetioAction<[ number, number ]>;
+
+  // Action that steps the simulation. This Action is implemented so it can be automatically played back for PhET-iO
+  // record/playback.  Listen to this Action if you have an action that happens during the simulation step.
+  public readonly stepSimulationAction: PhetioAction<[ number ]>;
+
+  // Emitter that indicates when a frame starts.
+  // Listen to this Emitter if you have an action that must be performed before the step begins.
+  public readonly frameStartedEmitter: Emitter;
+
+  // Emitter that indicates when a frame ends.
+  // Listen to this Emitter if you have an action that must be performed after the step completes.
+  public readonly frameEndedEmitter: Emitter
+
+  public readonly homeScreen: HomeScreen | null;
+
+  // all screens that appear in the runtime of this sim, with the homeScreen first if it was created
+  public readonly screens: AnyScreen[];
+
+  // order list of non-Home screens that appear in this runtime of the sim
+  public readonly simScreens: AnyScreen[];
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  private readonly display: SimDisplay;
+
+  public readonly navigationBar: NavigationBar; // joist-internal
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly lookAndFeel: LookAndFeel;
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly version: string;   // joist-internal
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly credits: any;   // joist-internal
+
+  // root node for the Display
+  public readonly rootNode: Node;
+
+  // layer for popups, dialogs, and their backgrounds and barriers
+  private readonly topLayer: Node;
+
+  // joist-internal
+  // Semi-transparent black barrier used to block input events when a dialog (or other popup)
+  // is present, and fade out the background.
+  public readonly barrierRectangle: BarrierRectangle;
+
+  //TODO https://github.com/phetsims/joist/issues/795
+  // list of nodes that are "modal" and hence block input with the barrierRectangle, used by modal dialogs and the PhetMenu.
+  private readonly modalNodeStack: ObservableArray<ModalNode>;
+
+  // number of animation frames that have occurred
+  private frameCounter: number;
+
+  // whether the window has resized since our last updateDisplay()
+  private resizePending: boolean;
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly locale: string;
+
+  // Keep track of the previous time for computing dt, and initially signify that time hasn't been recorded yet.
+  private lastStepTime: number;
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  private lastAnimationFrameTime: number;
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  private destroyed: boolean;
+
+  // if true, add support specific to accessible technology that work with touch devices.
+  public readonly supportsGestureDescription: boolean
+
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly hasKeyboardHelpContent: boolean;  // joist-internal
+
+  // true if all possible screens are present (order-independent)
+  public readonly allScreensCreated: boolean;
+
+  // create this only after all other members have been set on Sim
+  public readonly simInfo: SimInfo;
+
+  public readonly memoryMonitor: MemoryMonitor;
+
+  // If Preferences are available through a PreferencesConfiguration, this type will be added to the Sim to manage
+  // the state of features that can be enabled/disabled through user preferences.
+  public readonly preferencesManager: PreferencesManager | null;
+
+  // The Toolbar is not created unless requested with a PreferencesConfiguration.
+  private readonly toolbar: Toolbar | null;
+
+  // Creates content for the Options dialog.
+  public readonly createOptionsDialogContent: null | ( ( tandem: Tandem ) => Node );
+
+  // joist-internal
+  //TODO https://github.com/phetsims/joist/issues/795 document
+  public readonly updateBackground: () => void;
+
+  // joist-internal
+  // Bind the animation loop so it can be called from requestAnimationFrame with the right this.
+  public boundRunAnimationLoop: () => void;
+
   /**
-   * @param {string} name - the name of the simulation, to be displayed in the navbar and homescreen
-   * @param {Screen[]} allSimScreens - the possible screens for the sim in order of declaration (does not include the home screen)
-   * @param {Object} [options] - see below for options
+   * @param name - the name of the simulation, to be displayed in the navbar and homescreen
+   * @param allSimScreens - the possible screens for the sim, in the order that they appear on the homescreen and navbar
+   * @param providedOptions
    */
-  constructor( name, allSimScreens, options ) {
+  constructor( name: string, allSimScreens: AnyScreen[], providedOptions: SimOptions ) {
 
     window.phetSplashScreenDownloadComplete();
 
     assert && assert( allSimScreens.length >= 1, 'at least one screen is required' );
 
-    options = merge( {
+    const options = optionize<SimOptions, SelfOptions, PhetioObjectOptions>( {
 
-      // credits, see AboutDialog for format
+      // SelfOptions
+      //TODO https://github.com/phetsims/joist/issues/795
+      // @ts-ignore
       credits: {},
-
-      // {null|function(tandem:Tandem):Node} creates the content for the Options dialog
       createOptionsDialogContent: null,
-
-      // a {Node} placed onto the home screen (if available)
       homeScreenWarningNode: null,
-
-      // {boolean} - true when this sim supports a keyboard help button on the navigation bar that shows keyboard help
-      // content. This content is specific to each screen, see Screen.keyboardHelpNode for more info.
       hasKeyboardHelpContent: false,
 
-      // {VibrationManager|null} - Responsible for managing vibration feedback for a sim. Experimental, and
-      // not used frequently. The vibrationManager instance is passed through options so that tappi doesn't have to
-      // become a dependency for all sims yet. If this gets more use, this will likely change.
+      //TODO https://github.com/phetsims/joist/issues/794
       vibrationManager: null,
-
-      // {PreferencesConfiguration|null} - If a PreferencesConfiguration is provided the sim will
-      // include the PreferencesDialog and a button in the NavigationBar to open it.
       preferencesConfiguration: null,
-
-      // Passed to SimDisplay, but a top level option for API ease.
       webgl: SimDisplay.DEFAULT_WEBGL,
-
-      // Passed to the SimDisplay
       simDisplayOptions: {},
 
-      // phet-io
+      // PhetioObjectOptions
       phetioState: false,
       phetioReadOnly: true,
       tandem: Tandem.ROOT
-    }, options );
+    }, providedOptions );
 
     assert && assert( !options.simDisplayOptions.webgl, 'use top level sim option instead of simDisplayOptions' );
 
@@ -140,10 +314,21 @@
 
     super( options );
 
-    // @public (read-only PhetMenu) {null|function(tandem:Tandem):Node}
+    //TODO https://github.com/phetsims/joist/issues/795 which of these fields can be initialized in the constructor
+    // instead of finishInit? So they can be readonly and not have null values.
+
+    // Bind the animation loop so it can be called from requestAnimationFrame with the right this.
+    this.boundRunAnimationLoop = this.runAnimationLoop.bind( this );
+
+    this.modalNodeStack = createObservableArray();
+
+    //TODO https://github.com/phetsims/joist/issues/795 document
+    // Moved these here from finishInt, is that OK?
+    this.lastStepTime = -1;
+    this.lastAnimationFrameTime = -1;
+
     this.createOptionsDialogContent = options.createOptionsDialogContent;
 
-    // @public {Property.<string>} (joist-internal)
     this.simNameProperty = new StringProperty( name, {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'simNameProperty' ),
       phetioFeatured: true,
@@ -157,21 +342,16 @@
       throw new Error( 'playbackModeEnabledProperty cannot be changed after Sim construction has begun' );
     } );
 
-    // @public {Property} that indicates sim construction completed, and that all screen models and views have been created.
-    // This was added for PhET-iO but can be used by any client. This does not coincide with the end of the Sim
-    // constructor (because Sim has asynchronous steps that finish after the constructor is completed)
-    this.isConstructionCompleteProperty = new Property( false );
+    this.isConstructionCompleteProperty = new BooleanProperty( false );
     assert && this.isConstructionCompleteProperty.lazyLink( isConstructionComplete => {
       assert && assert( isConstructionComplete, 'Sim construction should never uncomplete' );
     } );
 
-    // @public {Property.<Dimension2>} - Stores the effective window dimensions that the simulation will be taking up
     this.dimensionProperty = new Property( new Dimension2( 0, 0 ), {
       useDeepEquality: true
     } );
 
-    // @public - Action that indicates when the sim resized.  This Action is implemented so it can be automatically played back.
-    this.resizeAction = new PhetioAction( ( width, height ) => {
+    this.resizeAction = new PhetioAction<[ number, number ]>( ( width: number, height: number ) => {
       assert && assert( width > 0 && height > 0, 'sim should have a nonzero area' );
 
       this.dimensionProperty.value = new Dimension2( width, height );
@@ -201,7 +381,10 @@
 
       // Layout each of the screens
       _.each( this.screens, m => m.view.layout( availableScreenBounds ) );
-      this.topLayer.children.forEach( child => child.layout && child.layout( availableScreenBounds ) );
+      this.topLayer.children.forEach( ( child: Node ) => {
+        // @ts-ignore
+        child.layout && child.layout( availableScreenBounds );
+      } );
 
       // Fixes problems where the div would be way off center on iOS7
       if ( platform.mobileSafari ) {
@@ -213,16 +396,18 @@
       this.boundsProperty.value = new Bounds2( 0, 0, width, height );
       this.screenBoundsProperty.value = availableScreenBounds.copy();
 
+      const animatedPanZoomListener = animatedPanZoomSingleton.listener!;
+
       // set the scale describing the target Node, since scale from window resize is applied to each ScreenView,
       // (children of the PanZoomListener targetNode)
-      animatedPanZoomSingleton.listener.setTargetScale( scale );
+      animatedPanZoomListener.setTargetScale( scale );
 
       // set the bounds which accurately describe the panZoomListener targetNode, since it would otherwise be
       // inaccurate with the very large BarrierRectangle
-      animatedPanZoomSingleton.listener.setTargetBounds( this.boundsProperty.value );
+      animatedPanZoomListener.setTargetBounds( this.boundsProperty.value );
 
       // constrain the simulation pan bounds so that it cannot be moved off screen
-      animatedPanZoomSingleton.listener.setPanBounds( this.boundsProperty.value );
+      animatedPanZoomListener.setPanBounds( this.boundsProperty.value );
     }, {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'resizeAction' ),
       parameters: [
@@ -245,21 +430,14 @@
     // relatively awkward, it is possible to listen for a callback when a frame begins, when a frame is being processed
     // or after the frame is complete.  See https://github.com/phetsims/joist/issues/534
 
-    // @public Emitter that indicates when a frame starts.  Listen to this Emitter if you have an action that must be
-    // performed before the step begins.
     this.frameStartedEmitter = new Emitter();
 
-    // @public Emitter that indicates when a frame ends.  Listen to this Emitter if you have an action that must be
-    // performed after the step completes.
     this.frameEndedEmitter = new Emitter( {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'frameEndedEmitter' ),
       phetioHighFrequency: true
     } );
 
-    // @public {Action} Action that steps the simulation. This Action is implemented so it can be automatically
-    // played back for PhET-iO record/playback.  Listen to this Action if you have an action that happens during the
-    // simulation step.
-    this.stepSimulationAction = new PhetioAction( dt => {
+    this.stepSimulationAction = new PhetioAction<[ number ]>( dt => {
       this.frameStartedEmitter.emit();
 
       // increment this before we can have an exception thrown, to see if we are missing frames
@@ -305,19 +483,19 @@
       if ( phet.chipper.queryParameters.supportsPanAndZoom ) {
 
         // animate the PanZoomListener, for smooth panning/scaling
-        animatedPanZoomSingleton.listener.step( dt );
+        animatedPanZoomSingleton.listener!.step( dt );
       }
 
       // if provided, update the vibrationManager which tracks time sequences of on/off vibration
+      // @ts-ignore TODO https://github.com/phetsims/joist/issues/794
       if ( this.vibrationManager ) {
+        // @ts-ignore TODO https://github.com/phetsims/joist/issues/794
         this.vibrationManager.step( dt );
       }
 
       // View step is the last thing before updateDisplay(), so we can do paint updates there.
       // See https://github.com/phetsims/joist/issues/401.
-      if ( screen.view.step ) {
-        screen.view.step( dt );
-      }
+      screen.view.step( dt );
 
       // Do not update the display while PhET-iO is customizing, or it could show the sim before it is fully ready for display.
       if ( !( Tandem.PHET_IO_ENABLED && !phet.phetio.phetioEngine.isReadyForDisplay ) ) {
@@ -352,28 +530,20 @@
       QueryStringMachine.containsKey( 'initialScreen' ),
       screensQueryParameter,
       QueryStringMachine.containsKey( 'screens' ),
-      selectedSimScreens => {
-        return new HomeScreen( this.simNameProperty, () => this.screenProperty, selectedSimScreens, Tandem.ROOT.createTandem( window.phetio.PhetioIDUtils.HOME_SCREEN_COMPONENT_NAME ), {
-          warningNode: options.homeScreenWarningNode
-        } );
+      ( selectedSimScreens: AnyScreen[] ) => {
+        return new HomeScreen( this.simNameProperty, () => this.screenProperty, selectedSimScreens,
+          Tandem.ROOT.createTandem( window.phetio.PhetioIDUtils.HOME_SCREEN_COMPONENT_NAME ), {
+            warningNode: options.homeScreenWarningNode
+          } );
       }
     );
 
-    // @public (read-only) {HomeScreen|null}
     this.homeScreen = screenData.homeScreen;
-
-    // @public (read-only) {Screen[]} - the ordered list of sim-specific screens that appear in this runtime of the sim
     this.simScreens = screenData.selectedSimScreens;
-
-    // @public (read-only) {Screen[]} - all screens that appear in the runtime of this sim, with the homeScreen first if
-    // it was created
     this.screens = screenData.screens;
-
-    // @public (read-only) {boolean} - true if all possible screens are present (order-independent)
     this.allScreensCreated = screenData.allScreensCreated;
 
-    // @public {Property.<Screen>}
-    this.screenProperty = new Property( screenData.initialScreen, {
+    this.screenProperty = new Property<AnyScreen>( screenData.initialScreen, {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'screenProperty' ),
       phetioFeatured: true,
       phetioDocumentation: 'Determines which screen is selected in the simulation',
@@ -381,9 +551,8 @@
       phetioType: Property.PropertyIO( Screen.ScreenIO )
     } );
 
-    // @public - the displayed name in the sim. This depends on what screens are shown this runtime (effected by query parameters).
     this.displayedSimNameProperty = new DerivedProperty( [ this.simNameProperty, this.simScreens[ 0 ].nameProperty ],
-      ( simName, screenName ) => {
+      ( simName: string, screenName: string | null ) => {
         const isMultiScreenSimDisplayingSingleScreen = this.simScreens.length === 1 && allSimScreens.length !== this.simScreens.length;
 
         // update the titleText based on values of the sim name and screen name
@@ -404,8 +573,6 @@
         }
       } );
 
-    // @public - When the sim is active, scenery processes inputs and stepSimulation(dt) runs from the system clock.
-    // Set to false for when the sim will be paused.
     this.activeProperty = new BooleanProperty( true, {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'activeProperty' ),
       phetioFeatured: true,
@@ -426,28 +593,16 @@
       this.browserTabVisibleProperty.set( document.visibilityState === 'visible' );
     }, false );
 
-    // @public (joist-internal, read-only) - How the home screen and navbar are scaled. This scale is based on the
-    // HomeScreen's layout bounds to support a consistently sized nav bar and menu. If this scale was based on the
-    // layout bounds of the current screen, there could be differences in the nav bar across screens.
     this.scaleProperty = new NumberProperty( 1 );
-
-    // @public (joist-internal, read-only) {Property.<Bounds2>|null} - global bounds for the entire simulation. null
-    //                                                                 before first resize
-    this.boundsProperty = new Property( null );
+    this.boundsProperty = new Property<Bounds2 | null>( null );
+    this.screenBoundsProperty = new Property<Bounds2 | null>( null );
 
-    // @public (joist-internal, read-only) {Property.<Bounds2>|null} - global bounds for the screen-specific part
-    //                                                            (excludes the navigation bar), null before first resize
-    this.screenBoundsProperty = new Property( null );
-
-    // @public
     this.lookAndFeel = new LookAndFeel();
     assert && assert( window.phet.joist.launchCalled, 'Sim must be launched using simLauncher, ' +
                                                       'see https://github.com/phetsims/joist/issues/142' );
 
-    // @private
     this.destroyed = false;
 
-    // @public {MemoryMonitor}
     this.memoryMonitor = new MemoryMonitor();
 
     // public (read-only) {boolean} - if true, add support specific to accessible technology that work with touch devices.
@@ -459,12 +614,10 @@
     assert && assert( !window.phet.joist.sim, 'Only supports one sim at a time' );
     window.phet.joist.sim = this;
 
-    // @public (read-only) {Property.<boolean>} - if PhET-iO is currently setting the state of the simulation.
-    // See PhetioStateEngine for details. This must be declared before soundManager.initialized is called.
     this.isSettingPhetioStateProperty = Tandem.PHET_IO_ENABLED ?
                                         new DerivedProperty(
                                           [ phet.phetio.phetioEngine.phetioStateEngine.isSettingStateProperty ],
-                                          _.identity ) :
+                                          ( isSettingState: boolean ) => isSettingState ) :
                                         new BooleanProperty( false );
 
     // commented out because https://github.com/phetsims/joist/issues/553 is deferred for after GQIO-oneone
@@ -488,21 +641,22 @@
     // @private {null|VibrationManager} - The singleton instance of VibrationManager. Experimental and not frequently
     // used. If used more generally, reference will no longer be needed as joist will have access to vibrationManager
     // through when tappi becomes a sim lib.
+    // @ts-ignore TODO https://github.com/phetsims/joist/issues/794
     this.vibrationManager = options.vibrationManager;
+    // @ts-ignore TODO https://github.com/phetsims/joist/issues/794
     if ( this.vibrationManager ) {
+      // @ts-ignore TODO https://github.com/phetsims/joist/issues/794
       this.vibrationManager.initialize( this.browserTabVisibleProperty, this.activeProperty );
     }
 
     // Make ScreenshotGenerator available globally so it can be used in preload files such as PhET-iO.
     window.phet.joist.ScreenshotGenerator = ScreenshotGenerator;
 
-    this.version = packageJSON.version; // @public (joist-internal)
-    this.credits = options.credits; // @public (joist-internal)
+    this.version = packageJSON.version;
+    this.credits = options.credits;
 
-    // @private - number of animation frames that have occurred
     this.frameCounter = 0;
 
-    // @private {boolean} - Whether the window has resized since our last updateDisplay()
     this.resizePending = true;
 
     // @public - Make our locale available
@@ -514,9 +668,6 @@
       $( 'title' ).html( name );
     }
 
-    // @public {PreferencesManager} - If Preferences are available through a PreferencesConfiguration,
-    // this type will be added to the Sim to manage the state of features that can be enabled/disabled
-    // through user preferences.
     this.preferencesManager = null;
 
     // @private {Toolbar|null} - The Toolbar is not created unless requested with a PreferencesConfiguration.
@@ -533,33 +684,45 @@
 
       // when the Toolbar positions update, resize the sim to fit in the available space
       this.toolbar.rightPositionProperty.lazyLink( () => {
-        this.resize( this.boundsProperty.value.width, this.boundsProperty.value.height );
+        assert && assert( this.boundsProperty.value ); // Bounds2 | null
+        const bounds = this.boundsProperty.value!;
+        this.resize( bounds.width, bounds.height );
       } );
     }
 
-    // @private
     this.display = new SimDisplay( options.simDisplayOptions );
+    this.topLayer = new Node();
+    this.display.simulationRoot.addChild( this.topLayer );
 
-    // @public - root node for the Display
+    this.barrierRectangle = new BarrierRectangle(
+      this.modalNodeStack, {
+        fill: 'rgba(0,0,0,0.3)',
+        pickable: true,
+        tandem: Tandem.GENERAL_VIEW.createTandem( 'barrierRectangle' ),
+        phetioDocumentation: 'Semi-transparent barrier used to block input events when a dialog is shown, also fades out the background'
+      } );
+    this.topLayer.addChild( this.barrierRectangle );
+
     this.rootNode = this.display.rootNode;
 
     Helper.initialize( this, this.display );
 
-    Property.multilink( [ this.activeProperty, phet.joist.playbackModeEnabledProperty ], ( active, playbackModeEnabled ) => {
+    Property.multilink( [ this.activeProperty, phet.joist.playbackModeEnabledProperty ],
+      ( active: boolean, playbackModeEnabled: boolean ) => {
 
-      // If in playbackMode is enabled, then the display must be interactive to support PDOM event listeners during
-      // playback (which often come directly from sim code and not from user input).
-      if ( playbackModeEnabled ) {
-        this.display.interactive = true;
-        globalKeyStateTracker.enabled = true;
-      }
-      else {
+        // If in playbackMode is enabled, then the display must be interactive to support PDOM event listeners during
+        // playback (which often come directly from sim code and not from user input).
+        if ( playbackModeEnabled ) {
+          this.display.interactive = true;
+          globalKeyStateTracker.enabled = true;
+        }
+        else {
 
-        // When the sim is inactive, make it non-interactive, see https://github.com/phetsims/scenery/issues/414
-        this.display.interactive = active;
-        globalKeyStateTracker.enabled = active;
-      }
-    } );
+          // When the sim is inactive, make it non-interactive, see https://github.com/phetsims/scenery/issues/414
+          this.display.interactive = active;
+          globalKeyStateTracker.enabled = active;
+        }
+      } );
 
     document.body.appendChild( this.display.domElement );
 
@@ -570,6 +733,7 @@
 
     // @public (joist-internal)
     this.updateBackground = () => {
+      // @ts-ignore
       this.lookAndFeel.backgroundColorProperty.value = this.screenProperty.value.backgroundColorProperty.value;
     };
 
@@ -583,7 +747,8 @@
     // See https://github.com/phetsims/scenery/issues/218
     this.screenProperty.lazyLink( ( newScreen, oldScreen ) => oldScreen.view.interruptSubtreeInput() );
 
-    // @public (read-only) {SimInfo} - create this only after all other members have been set on Sim
+    // create this only after all other members have been set on Sim
+    //TODO https://github.com/phetsims/joist/issues/795 document why "after"
     this.simInfo = new SimInfo( this );
 
     // Set up PhET-iO, must be done after phet.joist.sim is assigned
@@ -611,9 +776,8 @@
    * Update the views of the sim. This is meant to run after the state has been set to make sure that all view
    * elements are in sync with the new, current state of the sim. (even when the sim is inactive, as in the state
    * wrapper).
-   * @private
    */
-  updateViews() {
+  private updateViews(): void {
 
     // Trigger layout code
     this.resizeToWindow();
@@ -625,10 +789,9 @@
   }
 
   /**
-   * @param {Screen[]} screens
-   * @private
+   * TODO https://github.com/phetsims/joist/issues/795 document
    */
-  finishInit( screens ) {
+  private finishInit( screens: AnyScreen[] ): void {
 
     _.each( screens, screen => {
       screen.view.layerSplit = true;
@@ -661,28 +824,9 @@
 
         // Zoom out again after changing screens so we don't pan to the center of the focused ScreenView,
         // and so user has an overview of the new screen, see https://github.com/phetsims/joist/issues/682.
-        animatedPanZoomSingleton.listener.resetTransform();
+        animatedPanZoomSingleton.listener!.resetTransform();
       }
     } );
-
-    // layer for popups, dialogs, and their backgrounds and barriers
-    this.topLayer = new Node();
-    this.display.simulationRoot.addChild( this.topLayer );
-
-    // @private list of nodes that are "modal" and hence block input with the barrierRectangle.  Used by modal dialogs
-    // and the PhetMenu
-    this.modalNodeStack = createObservableArray(); // {Node} with node.hide()
-
-    // @public (joist-internal) Semi-transparent black barrier used to block input events when a dialog (or other popup)
-    // is present, and fade out the background.
-    this.barrierRectangle = new BarrierRectangle(
-      this.modalNodeStack, {
-        fill: 'rgba(0,0,0,0.3)',
-        pickable: true,
-        tandem: Tandem.GENERAL_VIEW.createTandem( 'barrierRectangle' ),
-        phetioDocumentation: 'Semi-transparent barrier used to block input events when a dialog is shown, also fades out the background'
-      } );
-    this.topLayer.addChild( this.barrierRectangle );
 
     // Fit to the window and render the initial scene
     // Can't synchronously do this in Firefox, see https://github.com/phetsims/vegas/issues/55 and
@@ -695,6 +839,7 @@
         this.resizePending = true;
       }
     };
+
     $( window ).resize( resizeListener );
     window.addEventListener( 'resize', resizeListener );
     window.addEventListener( 'orientationchange', resizeListener );
@@ -704,14 +849,6 @@
     // Kick off checking for updates, if that is enabled
     updateCheck.check();
 
-    // @private - Keep track of the previous time for computing dt, and initially signify that time hasn't been recorded yet.
-    this.lastStepTime = -1;
-    this.lastAnimationFrameTime = -1;
-
-    // @public (joist-internal)
-    // Bind the animation loop so it can be called from requestAnimationFrame with the right this.
-    this.boundRunAnimationLoop = this.runAnimationLoop.bind( this );
-
     // show any query parameter warnings in a dialog
     if ( QueryStringMachine.warnings.length ) {
       const warningDialog = new QueryParametersWarningDialog( QueryStringMachine.warnings, {
@@ -728,64 +865,81 @@
    * Adds a popup in the global coordinate frame. If the popup is model, it displays a semi-transparent black input
    * barrier behind it. A modal popup prevent the user from interacting with the reset of the application until the
    * popup is hidden. Use hidePopup() to hide the popup.
-   * @param {Node} popup - the popup, must implemented node.hide(), called by hidePopup
-   * @param {boolean} isModal - whether popup is modal
-   * @public
+   * @param popup - the popup, must implemented node.hide(), called by hidePopup
+   * @param isModal - whether popup is modal
    */
-  showPopup( popup, isModal ) {
-    assert && assert( popup );
-    assert && assert( !!popup.hide, 'Missing popup.hide() for showPopup' );
-    assert && assert( !this.topLayer.hasChild( popup ), 'popup already shown' );
+  public showPopup( popup: ModalNode, isModal: boolean ): void {
+
+    // These fields are not initialized until finishInit is called.
+    const modalStack = this.modalNodeStack!;
+    const topLayer = this.topLayer!;
+
+    assert && assert( !topLayer.hasChild( popup ), 'popup already shown' );
     if ( isModal ) {
       this.rootNode.interruptSubtreeInput();
-      this.modalNodeStack.push( popup );
+      modalStack.push( popup );
     }
     if ( popup.layout ) {
-      popup.layout( this.screenBoundsProperty.value );
+      assert && assert( this.screenBoundsProperty.value, 'expected screenBoundsProperty to have a value by now' );
+      popup.layout( this.screenBoundsProperty.value! );
     }
-    this.topLayer.addChild( popup );
+    topLayer.addChild( popup );
   }
 
   /*
    * Hides a popup that was previously displayed with showPopup()
-   * @param {Node} popup
-   * @param {boolean} isModal - whether popup is modal
-   * @public
+   * @param popup
+   * @param isModal - whether popup is modal
    */
-  hidePopup( popup, isModal ) {
-    assert && assert( popup && this.modalNodeStack.includes( popup ) );
-    assert && assert( this.topLayer.hasChild( popup ), 'popup was not shown' );
+  public hidePopup( popup: ModalNode, isModal: boolean ): void {
+
+    // These fields are not initialized until finishInit is called.
+    const modalStack = this.modalNodeStack!;
+    const topLayer = this.topLayer!;
+
+    assert && assert( popup && modalStack.includes( popup ) );
+    assert && assert( topLayer.hasChild( popup ), 'popup was not shown' );
     if ( isModal ) {
-      this.modalNodeStack.remove( popup );
+      modalStack.remove( popup );
     }
-    this.topLayer.removeChild( popup );
+    topLayer.removeChild( popup );
   }
 
   /**
+   * TODO https://github.com/phetsims/joist/issues/795 document
    * @public (joist-internal)
    */
-  resizeToWindow() {
+  public resizeToWindow(): void {
     this.resizePending = false;
     this.resize( window.innerWidth, window.innerHeight ); // eslint-disable-line bad-sim-text
   }
 
-  // @public (joist-internal, phet-io)
-  resize( width, height ) {
+  /**
+   * TODO https://github.com/phetsims/joist/issues/795 document
+   * @param width
+   * @param height
+   * @public (joist-internal, phet-io)
+   */
+  public resize( width: number, height: number ): void {
     this.resizeAction.execute( width, height );
   }
 
   // Destroy a sim so that it will no longer consume any resources. Formerly used in Smorgasbord.  May not be used by
   // anything else at the moment.
 
-  // @public (joist-internal)
-  start() {
+  /**
+   * TODO https://github.com/phetsims/joist/issues/795 document
+   * @public (joist-internal)
+   */
+  public start(): void {
 
     // In order to animate the loading progress bar, we must schedule work with setTimeout
     // This array of {function} is the work that must be completed to launch the sim.
-    const workItems = [];
+    type WorkItem = () => void;
+    const workItems: WorkItem[] = [];
 
     // Schedule instantiation of the screens
-    this.screens.forEach( screen => {
+    this.screens.forEach( ( screen: AnyScreen ) => {
       workItems.push( () => {
 
         // Screens may share the same instance of backgroundProperty, see joist#441
@@ -800,7 +954,7 @@
     } );
 
     // loop to run startup items asynchronously so the DOM can be updated to show animation on the progress bar
-    const runItem = i => {
+    const runItem = ( i: number ) => {
       setTimeout( // eslint-disable-line bad-sim-text
         () => {
           workItems[ i ]();
@@ -814,7 +968,7 @@
           if ( document.getElementById( 'progressBarForeground' ) ) {
 
             // Grow the progress bar foreground to the right based on the progress so far.
-            document.getElementById( 'progressBarForeground' ).setAttribute( 'width', `${progress * PROGRESS_BAR_WIDTH}` );
+            document.getElementById( 'progressBarForeground' )!.setAttribute( 'width', `${progress * PROGRESS_BAR_WIDTH}` );
           }
           if ( i + 1 < workItems.length ) {
             runItem( i + 1 );
@@ -897,14 +1051,22 @@
     runItem( 0 );
   }
 
-  // @public (joist-internal)
-  destroy() {
+  /**
+   * TODO https://github.com/phetsims/joist/issues/795 document
+   * TODO why isn't this dispose?
+   * @public (joist-internal)
+   */
+  public destroy(): void {
     this.destroyed = true;
     this.display.domElement.parentNode && this.display.domElement.parentNode.removeChild( this.display.domElement );
   }
 
-  // @private - Bound to this.boundRunAnimationLoop so it can be run in window.requestAnimationFrame
-  runAnimationLoop() {
+  /**
+   * TODO https://github.com/phetsims/joist/issues/795 document
+   *
+   * Bound to this.boundRunAnimationLoop so it can be run in window.requestAnimationFrame
+   */
+  private runAnimationLoop(): void {
     if ( !this.destroyed ) {
       window.requestAnimationFrame( this.boundRunAnimationLoop );
     }
@@ -932,8 +1094,10 @@
     }
   }
 
-  // @private - run a single frame including model, view and display updates
-  stepOneFrame() {
+  /**
+   * run a single frame including model, view and display updates
+   */
+  private stepOneFrame(): void {
 
     // Compute the elapsed time since the last frame, or guess 1/60th of a second if it is the first frame
     const currentTime = Date.now();
@@ -948,10 +1112,9 @@
 
   /**
    * Update the simulation model, view, scenery display with an elapsed time of dt.
-   * @param {number} dt in seconds
-   * @public (phet-io)
+   * @param dt in seconds
    */
-  stepSimulation( dt ) {
+  public stepSimulation( dt: number ): void {
     this.stepSimulationAction.execute( dt );
   }
 
@@ -959,10 +1122,9 @@
    * Hide or show all accessible content related to the sim ScreenViews, and navigation bar. This content will
    * remain visible, but not be tab navigable or readable with a screen reader. This is generally useful when
    * displaying a pop up or modal dialog.
-   * @param {boolean} visible
-   * @public
+   * @param visible
    */
-  setAccessibleViewsVisible( visible ) {
+  public setAccessibleViewsVisible( visible: boolean ) {
     for ( let i = 0; i < this.screens.length; i++ ) {
       this.screens[ i ].view.pdomVisible = visible;
     }
@@ -979,7 +1141,7 @@
  * @param {number} currentTime - milliseconds, current time.  Passed in instead of computed so there is no "slack" between measurements
  * @returns {number} - seconds
  */
-function getDT( lastTime, currentTime ) {
+function getDT( lastTime: number, currentTime: number ): number {
 
   // Compute the elapsed time since the last frame, or guess 1/60th of a second if it is the first frame
   return ( lastTime === -1 ) ? 1 / 60 :
Index: joist/tsconfig-module.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/tsconfig-module.json b/joist/tsconfig-module.json
--- a/joist/tsconfig-module.json	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/tsconfig-module.json	(date 1647282050057)
@@ -17,6 +17,7 @@
     "../chipper/phet-types.d.ts",
     "../chipper/node_modules/@types/lodash/index.d.ts",
     "../chipper/node_modules/@types/qunit/index.d.ts",
+    "../chipper/node_modules/@types/jquery/index.d.ts",
 
     "js/**/*",
     "sounds/**/*",
Index: joist/tsconfig.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/tsconfig.json b/joist/tsconfig.json
--- a/joist/tsconfig.json	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/tsconfig.json	(date 1647281938617)
@@ -9,6 +9,7 @@
     "../chipper/phet-types.d.ts",
     "../chipper/node_modules/@types/lodash/index.d.ts",
     "../chipper/node_modules/@types/qunit/index.d.ts",
+    "../chipper/node_modules/@types/jquery/index.d.ts",
 
     "js/**/*tests.js", // Cannot use **test** like we do elsewhere because of UpdaTESTate
     "js/**/*Tests.js", // Cannot use **test** like we do elsewhere because of UpdaTESTate
Index: joist/js/LookAndFeel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/LookAndFeel.js b/joist/js/LookAndFeel.js
--- a/joist/js/LookAndFeel.js	(revision fb857afab90f61e0b78a9f3fe8e8a08adce82964)
+++ b/joist/js/LookAndFeel.js	(date 1647282050055)
@@ -15,7 +15,7 @@
 
   constructor() {
 
-    // @public background color for the currently selected screen, which will be set on the Display as its backgroundColor
+    // @public {IColor} background color for the currently selected screen, which will be set on the Display as its backgroundColor
     this.backgroundColorProperty = new Property( 'black' );
 
     // @public (joist-internal) {boolean} - True if the navigation bar background is black

@pixelzoom pixelzoom removed their assignment Mar 14, 2022
@zepumph zepumph self-assigned this Mar 14, 2022
@zepumph
Copy link
Member

zepumph commented Mar 14, 2022

Thanks for taking a first pass! Sounds, horrible.

@zepumph
Copy link
Member

zepumph commented Apr 14, 2022

See #776 (comment)
by @samreid for the most recent conversion patch.

I will review this this week.

samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit that referenced this issue Apr 16, 2022
samreid added a commit to phetsims/geometric-optics that referenced this issue Apr 16, 2022
samreid added a commit to phetsims/number-play that referenced this issue Apr 16, 2022
samreid added a commit to phetsims/number-play that referenced this issue Apr 16, 2022
samreid added a commit to phetsims/chipper that referenced this issue Apr 16, 2022
samreid added a commit to phetsims/query-string-machine that referenced this issue Apr 16, 2022
@samreid
Copy link
Member

samreid commented Apr 16, 2022

I pushed changes to joist centered around Sim.ts. There were complications around Colors, some places expecting Color|string, some expecting just Color and some expecting just string. There were expectations throughout joist that colors were === 'black' or === 'white' or !==, so I converted some files to *.ts to help catch errors in those areas. It seemed best to promote LookAndFeel to use Color.

The quick start panel in CT (henceforth "CTQ") is green. Local testing seems OK. Dialogs show up in the right z-order. Sim logic runs. Keyboard help dialog appears. Tab navigation works. Type checking is passing. Lint is passing. Studio is working.

I'll mark this issue as blocks-publication in case anyone tries to publish something Monday. This is scheduled for review Tuesday.

UPDATE I have several any ts-ignore and TODO to investigate before this is ready for review.

@samreid samreid self-assigned this Apr 16, 2022
@zepumph
Copy link
Member

zepumph commented Apr 25, 2022

  • I like unknown! But we call model.step. So that would need an ignore or something. I tried this patch and I got really close to liking it, but can you help me out with the one error I'm getting with this.homeScreen === screen.
Index: js/IModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/IModel.ts b/js/IModel.ts
new file mode 100644
--- /dev/null	(date 1650910310505)
+++ b/js/IModel.ts	(date 1650910310505)
@@ -0,0 +1,13 @@
+// Copyright 2022, University of Colorado Boulder
+
+/**
+ * Model interface as it pertains to joist's implementation.
+ * @author Michael Kauzmann (PhET Interactive Simulations)
+ */
+
+type IModel = {
+
+  step?: ( dt: number ) => void;
+}
+
+export default IModel;
\ No newline at end of file
Index: js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/Sim.ts b/js/Sim.ts
--- a/js/Sim.ts	(revision f170b3c44238382daed60ac1b05d90c017f4bf6e)
+++ b/js/Sim.ts	(date 1650910310501)
@@ -5,7 +5,7 @@
  * Provides default initialization, such as polyfills as well.
  * If the simulation has only one screen, then there is no homescreen, home icon or screen icon in the navigation bar.
  *
- * The type for the contained Screen instances is Screen<any,any> since we do not want to parameterize Sim<[{M1,V1},{M2,V2}]
+ * The type for the contained Screen instances is Screen<IModel,ScreenView> since we do not want to parameterize Sim<[{M1,V1},{M2,V2}]
  * etc.
  *
  * @author Sam Reid (PhET Interactive Simulations)
@@ -67,6 +67,7 @@
 import ScreenView from './ScreenView.js';
 import Popupable from '../../sun/js/Popupable.js';
 import PickOptional from '../../phet-core/js/types/PickOptional.js';
+import IModel from './IModel.js';
 
 // constants
 const PROGRESS_BAR_WIDTH = 273;
@@ -160,14 +161,14 @@
 
   // the ordered list of sim-specific screens that appear in this runtime of the sim
   // REVIEW: 3. Is it time to create a Model supertype or a IModel interface that can be used for type safety here?
-  private readonly simScreens: Screen<any, ScreenView>[];
+  private readonly simScreens: Screen<IModel, ScreenView>[];
 
   // all screens that appear in the runtime of this sim, with the homeScreen first if it was created
-  private readonly screens: Screen<any, ScreenView>[];
+  private readonly screens: Screen<IModel, ScreenView>[];
 
   // the displayed name in the sim. This depends on what screens are shown this runtime (effected by query parameters).
   private readonly displayedSimNameProperty: IReadOnlyProperty<string>;
-  readonly screenProperty: Property<Screen<any, ScreenView>>;
+  readonly screenProperty: Property<Screen<IModel, ScreenView>>;
 
   // true if all possible screens are present (order-independent)
   private readonly allScreensCreated: boolean;
@@ -265,7 +266,7 @@
    * @param allSimScreens - the possible screens for the sim in order of declaration (does not include the home screen)
    * @param [providedOptions] - see below for options
    */
-  constructor( name: string, allSimScreens: Screen<any, ScreenView>[], providedOptions?: SimOptions ) {
+  constructor( name: string, allSimScreens: Screen<IModel, ScreenView>[], providedOptions?: SimOptions ) {
 
     window.phetSplashScreenDownloadComplete();
 
@@ -508,7 +509,7 @@
     this.screens = screenData.screens;
     this.allScreensCreated = screenData.allScreensCreated;
 
-    this.screenProperty = new Property<Screen<any, ScreenView>>( screenData.initialScreen, {
+    this.screenProperty = new Property<Screen<IModel, ScreenView>>( screenData.initialScreen, {
       tandem: Tandem.GENERAL_MODEL.createTandem( 'screenProperty' ),
       phetioFeatured: true,
       phetioDocumentation: 'Determines which screen is selected in the simulation',
@@ -699,7 +700,7 @@
     animationFrameTimer.runOnNextTick( () => phet.joist.display.updateDisplay() );
   }
 
-  private finishInit( screens: Screen<any, ScreenView>[] ): void {
+  private finishInit( screens: Screen<IModel, ScreenView>[] ): void {
 
     _.each( screens, screen => {
       screen.view.layerSplit = true;

  • We could (a) separately manage an array of Node with layout, or run type assertions on them when we read them out of topLayer.children.

I don't really understand this, is it required that nodes in the topLayer have a layout function? because I didn't think it did, if so, that seems like a reasonable path.

That sounds reasonable to me, unless there is an ordering dependency (hopefully there isn't).

Good luck!

I reviewed everything else. Thanks!

pixelzoom added a commit that referenced this issue May 19, 2022
@zepumph
Copy link
Member

zepumph commented Jun 9, 2022

Oops, I didn't assign back. Sorry.

@zepumph zepumph assigned samreid and unassigned zepumph Jun 9, 2022
@samreid
Copy link
Member

samreid commented Jun 14, 2022

but can you help me out with the one error I'm getting with this.homeScreen === screen.

That is because the HomeScreenModel has no step function. I think it would be very safe to add. I also tried changing Screen's declaration to class Screen<M extends IModel, V extends ScreenView> extends PhetioObject { but that led to a lot of contravariance problems like so:

/Users/samreid/apache-document-root/main/number-play/js/number-play-main.ts(61,5): error TS2322: Type 'NumberPlayGameScreen' is not assignable to type 'Screen<IModel, ScreenView>'.
  Types of property 'createView' are incompatible.
    Type '(model: NumberPlayGameModel) => NumberPlayGameScreenView' is not assignable to type '(model: IModel) => ScreenView'.
      Types of parameters 'model' and 'model' are incompatible.
        Type 'IModel' is missing the following properties from type 'NumberPlayGameModel': subitizeLevels, countingLevels, levels, levelProperty, reset

How do you want to proceed on this part?

, is it required that nodes in the topLayer have a layout function?

It's optional at the moment, like so:

      this.topLayer.children.forEach( child => {

        // @ts-ignore TODO: See https://github.com/phetsims/joist/issues/795
        // REVIEW: I'm really not sure how to handle this one, let's talk about it.
        child.layout && child.layout( availableScreenBounds );
      } );

@zepumph
Copy link
Member

zepumph commented Aug 11, 2022

How do you want to proceed on this part?

Will this be sorted out in #783?

I like this solution for topLayer after trying a couple of options (like augmenting the Node interface).

It doesn't get us fully there, but helps in the case where you make this change, note that Sim knows that the API for layout should be a bounds2 parameter:

Index: js/Popupable.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/Popupable.ts b/js/Popupable.ts
--- a/js/Popupable.ts	(revision 1677fc56a62a51a3fe7a28b58c44f9123c7b990d)
+++ b/js/Popupable.ts	(date 1660238463778)
@@ -123,7 +123,7 @@
       } );
     }
 
-    public layout( bounds: Bounds2 ): void {
+    public layout( scale: number, bounds: Bounds2 ): void {
       if ( this.layoutBounds ) {
         this.popupParent.matrix = ScreenView.getLayoutMatrix( this.layoutBounds, bounds );
       }

Please review.

@samreid
Copy link
Member

samreid commented Aug 27, 2022

Good fix, thanks! Let's go on hold until #783 is complete.

@samreid
Copy link
Member

samreid commented Sep 5, 2022

In #783 we decided to stick with

type TModel = {
  step: ( dt: number ) => void;
};

and

// Parameterized on M=Model and V=View
// The IntentionalAny in the model type is due to https://github.com/phetsims/joist/issues/783#issuecomment-1231017213
class Screen<M extends TModel = IntentionalAny, V extends ScreenView = ScreenView> extends PhetioObject {

Regarding the patch in #795 (comment), I didn't fully understand it, but since Sim doesn't have a scale to pass, perhaps it should remain omitted?

Back to @zepumph for discussion/review. I'm OK to close if you agree.

@samreid samreid assigned zepumph and unassigned samreid Sep 5, 2022
@zepumph
Copy link
Member

zepumph commented Sep 6, 2022

I didn't make too much sense. Let me re-explain:

d708083 is the recommended fix for a ts-ignore

It works so-so in my opinion, but is probably good enough. Note the type error when you apply this patch:

Index: js/Popupable.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/Popupable.ts b/js/Popupable.ts
--- a/js/Popupable.ts	(revision 1677fc56a62a51a3fe7a28b58c44f9123c7b990d)
+++ b/js/Popupable.ts	(date 1660238463778)
@@ -123,7 +123,7 @@
       } );
     }
 
-    public layout( bounds: Bounds2 ): void {
+    public layout( scale: number, bounds: Bounds2 ): void {
       if ( this.layoutBounds ) {
         this.popupParent.matrix = ScreenView.getLayoutMatrix( this.layoutBounds, bounds );
       }

TS2322: Type '(scale: number, bounds: Bounds2) => void' is not assignable to type '(bounds: Bounds2) => void'.

So if you try to add the wrong type signature for the layout method to the top layer, we fail. Will you take a final review of that commit and let me know if you recommend anything further? Please feel free to close.

@zepumph zepumph assigned samreid and unassigned zepumph Sep 6, 2022
@samreid
Copy link
Member

samreid commented Sep 6, 2022

Yes, that matches my understanding and I reproduced that type error by changing that layout method. I think we are in good shape to close.

@samreid samreid closed this as completed Sep 6, 2022
samreid added a commit to phetsims/perennial that referenced this issue Oct 22, 2024
zepumph pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants