phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Rewrite Screen without createModel and createView closures #922

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

Over in https://github.com/phetsims/joist/issues/886, @samreid and I came to an initial understanding (needs more investigation) that one of the only "fully correct" solutions for the TypeScript parameterization of Screen is to remove the notion of createView, which takes a TModel as a parameter.

For example, this patch is supportive of having AnyScreen's model type be TModel:

``` Subject: [PATCH] Rename beam => handle, see https://github.com/phetsims/center-and-variability/issues/182 --- Index: js/HomeScreen.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/HomeScreen.ts b/js/HomeScreen.ts --- a/js/HomeScreen.ts (revision 513e637f35c41068cc67c36b1788762c9b095377) +++ b/js/HomeScreen.ts (date 1683657285512) @@ -56,7 +56,7 @@ // at the time of construction, the Sim.screenProperty is not yet assigned (because it may itself include the // HomeScreen), so we must use a function to lazily get it after it is assigned () => new HomeScreenModel( getScreenProperty(), simScreens, activeSimScreensProperty, options.tandem.createTandem( 'model' ) ), - model => new HomeScreenView( simNameProperty, model, { + () => new HomeScreenView( simNameProperty, model, { warningNode: options.warningNode, tandem: options.tandem.createTandem( 'view' ) } ), Index: js/Screen.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/Screen.ts b/js/Screen.ts --- a/js/Screen.ts (revision 513e637f35c41068cc67c36b1788762c9b095377) +++ b/js/Screen.ts (date 1683657214069) @@ -30,7 +30,6 @@ import ScreenIcon from './ScreenIcon.js'; import ScreenView from './ScreenView.js'; import PickRequired from '../../phet-core/js/types/PickRequired.js'; -import IntentionalAny from '../../phet-core/js/types/IntentionalAny.js'; import Multilink from '../../axon/js/Multilink.js'; import TModel from './TModel.js'; import PatternStringProperty from '../../axon/js/PatternStringProperty.js'; @@ -72,7 +71,7 @@ // @joist-internal - This type is uses IntentionalAny to break the contravariance dependency that the createView function // has on Screen. Ideally we could use TModel instead, but that would involve a rewrite of how we pass closures into // Screen instead of the already created Model/View themselves. See https://github.com/phetsims/joist/issues/783#issuecomment-1231017213 -export type AnyScreen = Screen; +export type AnyScreen = Screen; // Parameterized on M=Model and V=View class Screen extends PhetioObject { @@ -91,7 +90,7 @@ public readonly createKeyboardHelpNode: null | ( ( tandem: Tandem ) => Node ); // joist-internal public readonly pdomDisplayNameProperty: TReadOnlyProperty; private readonly createModel: () => M; - private readonly createView: ( model: M ) => V; + private readonly createView: () => V; private _model: M | null; private _view: V | null; @@ -104,7 +103,7 @@ documentation: 'Section of a simulation which has its own model and view.' } ); - public constructor( createModel: () => M, createView: ( model: M ) => V, providedOptions: ScreenOptions ) { + public constructor( createModel: () => M, createView: () => V, providedOptions: ScreenOptions ) { const options = optionize()( {
samreid commented 1 year ago

We also considered passing in an array of ()=>Screen. But @zepumph and I agree that this or the proposal above is not a good use of time. We would like to avoid this explicit "non-goal" of TypeScript in https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals#non-goals

[the goal is not to] Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.