Screen is incorrectly parameterized #886
Assignees
Labels
Comments
GravityAndOrbitsModel implements TModel because it matches the interface. TypeScript is structurally typed rather than nominally typed, so because
If I remove the
True, but I'm not sure what kind of problems that could create.
The code comment says: // The IntentionalAny in the model type is due to https://github.com/phetsims/joist/issues/783#issuecomment-1231017213
class Screen<M extends TModel = TModel, V extends ScreenView = ScreenView> extends PhetioObject {which refers to: #783 (comment) I tested |
That's a poor reason to omit
That is what I was advocating. Yeah, it'll work if you omit And it sounds like you're using IntentionalAny because doing it correctly is too much work. Or am I summarizing incorrectly? |
|
Changing class Screen<M extends TModel = IntentionalAny, V extends ScreenView = ScreenView> extends PhetioObject {to class Screen<M extends TModel, V extends ScreenView = ScreenView> extends PhetioObject {Introduces 80 type errors. I'm not worried about the sim-specific ones. But the ones in Joist would be much more difficult to fix.
That is a fair assessment, and in line with our philosophy about getting added value from TypeScript, without spending excessive time to get everything to be type-perfect. |
The change that you proposed was as follows, and it introduces 48 TS errors in joist. class Screen<M extends TModel, V extends ScreenView = ScreenView> extends PhetioObject {Changing to this reduces the number of TS errors to 27: class Screen<M extends TModel = TModel, V extends ScreenView = ScreenView> extends PhetioObject {Inspecting the above errors, 100% of them are related to HomeScreen. So there is undoubtedly a type problem with HomeScreen that should be addressed. And I don't see any reason to mask it with IntentionalAny. Back to @samreid for comment. |
|
There appear to be only 2 unique TS errors related to HomeScreen. Here they are, with examples: |
|
I'd start with this: - class HomeScreenModel {
+ class HomeScreenModel implements TModel { |
|
Here's my initial start on this issue: Subject: [PATCH] Move LinkableElement to a top-level type, improve documentation, use LinkableProperty and LinkableReadOnlyProperty, see https://github.com/phetsims/axon/issues/414
---
Index: main/acid-base-solutions/js/mysolution/MySolutionScreen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/acid-base-solutions/js/mysolution/MySolutionScreen.ts b/main/acid-base-solutions/js/mysolution/MySolutionScreen.ts
--- a/main/acid-base-solutions/js/mysolution/MySolutionScreen.ts (revision 58e33883fb21e5266e878f09ba45b8d55ad55b3f)
+++ b/main/acid-base-solutions/js/mysolution/MySolutionScreen.ts (date 1676415692457)
@@ -18,7 +18,7 @@
import MySolutionModel from './model/MySolutionModel.js';
import MySolutionScreenView from './view/MySolutionScreenView.js';
-export default class MySolutionScreen extends Screen {
+export default class MySolutionScreen extends Screen<MySolutionModel, MySolutionScreenView> {
public constructor( tandem: Tandem ) {
Index: main/joist/js/Screen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/Screen.ts b/main/joist/js/Screen.ts
--- a/main/joist/js/Screen.ts (revision 645f5bfedd59ebe8628b1ddf148fa56c1aea195a)
+++ b/main/joist/js/Screen.ts (date 1676415483098)
@@ -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';
@@ -74,7 +73,7 @@
// 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 {
+class Screen<M extends TModel = TModel, V extends ScreenView = ScreenView> extends PhetioObject {
public backgroundColorProperty: Property<Color> | Property<string> | Property<Color | string>;
Index: main/joist/js/HomeScreenModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/HomeScreenModel.ts b/main/joist/js/HomeScreenModel.ts
--- a/main/joist/js/HomeScreenModel.ts (revision 645f5bfedd59ebe8628b1ddf148fa56c1aea195a)
+++ b/main/joist/js/HomeScreenModel.ts (date 1676415429570)
@@ -14,8 +14,9 @@
import Tandem from '../../tandem/js/Tandem.js';
import joist from './joist.js';
import Screen from './Screen.js';
+import TModel from './TModel.js';
-class HomeScreenModel {
+class HomeScreenModel implements TModel {
public simScreens: Screen[]; // screens in the simulations that are not the HomeScreen
public screenProperty: Property<Screen>;
public selectedScreenProperty: Property<Screen>;
Index: main/acid-base-solutions/js/introduction/IntroductionScreen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/acid-base-solutions/js/introduction/IntroductionScreen.ts b/main/acid-base-solutions/js/introduction/IntroductionScreen.ts
--- a/main/acid-base-solutions/js/introduction/IntroductionScreen.ts (revision 58e33883fb21e5266e878f09ba45b8d55ad55b3f)
+++ b/main/acid-base-solutions/js/introduction/IntroductionScreen.ts (date 1676415610603)
@@ -18,7 +18,7 @@
import IntroductionModel from './model/IntroductionModel.js';
import IntroductionScreenView from './view/IntroductionScreenView.js';
-export default class IntroductionScreen extends Screen {
+export default class IntroductionScreen extends Screen<IntroductionModel, IntroductionScreenView> {
public constructor( tandem: Tandem ) {
After this change, there are 101 type errors: So I feel this is more of a collaboration issue. Please reach out to me on slack to schedule a time if you want to work on it together. |
|
Unlikely that I'll be able to join you on this until March. But I'll let you know if that changes. |
|
Thinking about this more... I did report this problem because I'm hitting it repeatedly, it's a problem in a foundational class, and it seems solvable. So I don't agree that masking the problem with |
pixelzoom
added a commit
to phetsims/graphing-slope-intercept
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/natural-selection
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/molecule-polarity
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/hookes-law
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/circuit-construction-kit-common
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/wilder
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/vegas
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/sound-waves
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/tambo
that referenced
this issue
Feb 15, 2023
pixelzoom
added a commit
to phetsims/sun
that referenced
this issue
Feb 15, 2023
|
In the above commits, I added missing |
|
I saw more notes about this problem in #783 (comment) |
|
After @pixelzoom commits above, I tried
Here's one I have no idea how to solve in NavigationBar:
Anyways, there are 95 other errors like that. I don't feel I have the expertise to develop a good solution here. We could reach out to #dev-public and see if someone else wants to attempt it? Or maybe even better to schedule it for the next sprint. But my suspicion is that we will conclude "it is not worth the time and baggage to add this level of type parametrization in Screen". But I would be happy to be wrong about that. |
|
I wrote to slack #dev-public:
I also listed it as a "proposed dev activity" in the 2023 iterations project board: https://github.com/orgs/phetsims/projects/65/views/7 |
|
Maybe something like this patch "AnyScreen" would be good? Subject: [PATCH] Remove completed TODO, see https://github.com/phetsims/center-and-variability/issues/45
---
Index: main/balancing-chemical-equations/js/game/GameScreen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/balancing-chemical-equations/js/game/GameScreen.ts b/main/balancing-chemical-equations/js/game/GameScreen.ts
--- a/main/balancing-chemical-equations/js/game/GameScreen.ts (revision f140cc1a1a9c553d91586ccf85c63633776c8268)
+++ b/main/balancing-chemical-equations/js/game/GameScreen.ts (date 1682946482460)
@@ -19,7 +19,7 @@
import GameModel from './model/GameModel.js';
import GameScreenView from './view/GameScreenView.js';
-export default class GameScreen extends Screen {
+export default class GameScreen extends Screen<GameModel, GameScreenView> {
public constructor( tandem: Tandem ) {
Index: main/joist/js/demo/DialogsScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/demo/DialogsScreenView.ts b/main/joist/js/demo/DialogsScreenView.ts
--- a/main/joist/js/demo/DialogsScreenView.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/demo/DialogsScreenView.ts (date 1682946518316)
@@ -14,7 +14,7 @@
import joist from '../joist.js';
import KeyboardHelpButton from '../KeyboardHelpButton.js';
import ScreenView, { ScreenViewOptions } from '../ScreenView.js';
-import Screen from '../Screen.js';
+import Screen, { AnyScreen } from '../Screen.js';
import Sim from '../Sim.js';
import NavigationBarPreferencesButton from '../preferences/NavigationBarPreferencesButton.js';
import PreferencesModel from '../preferences/PreferencesModel.js';
@@ -29,7 +29,7 @@
const keyboardHelpDialogContent = new BasicActionsKeyboardHelpSection();
- const fakeScreen = { createKeyboardHelpNode: () => keyboardHelpDialogContent, tandem: Tandem.OPTIONAL } as unknown as Screen;
+ const fakeScreen = { createKeyboardHelpNode: () => keyboardHelpDialogContent, tandem: Tandem.OPTIONAL } as unknown as AnyScreen;
const keyboardHelpButton = new KeyboardHelpButton(
[ fakeScreen ],
new Property( fakeScreen ),
Index: main/number-compare/js/common/view/numberCompareUtteranceQueue.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/number-compare/js/common/view/numberCompareUtteranceQueue.ts b/main/number-compare/js/common/view/numberCompareUtteranceQueue.ts
--- a/main/number-compare/js/common/view/numberCompareUtteranceQueue.ts (revision 9761d4fc4df82ff32f85f765a42d48d58df93909)
+++ b/main/number-compare/js/common/view/numberCompareUtteranceQueue.ts (date 1682946547954)
@@ -17,7 +17,7 @@
import numberComparePreferences from '../model/numberComparePreferences.js';
import Property from '../../../../axon/js/Property.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-import Screen from '../../../../joist/js/Screen.js';
+import Screen, { AnyScreen } from '../../../../joist/js/Screen.js';
class NumberCompareUtteranceQueue extends NumberSuiteCommonUtteranceQueue {
@@ -41,7 +41,7 @@
* to use for a given screen that the user is viewing. This is needed because selectedScreenProperty doesn't exist
* yet during the creation of this singleton.
*/
- public initialize( selectedScreenProperty: TReadOnlyProperty<Screen> ): void {
+ public initialize( selectedScreenProperty: TReadOnlyProperty<AnyScreen> ): void {
const speechDataProperty = new DerivedProperty(
[ this.compareScreenSpeechDataProperty, selectedScreenProperty ], ( compareScreenSpeechData, selectedScreen ) => {
Index: main/joist/js/selectScreens.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/selectScreens.ts b/main/joist/js/selectScreens.ts
--- a/main/joist/js/selectScreens.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/selectScreens.ts (date 1682945915838)
@@ -1,13 +1,13 @@
// Copyright 2020-2022, University of Colorado Boulder
import joist from './joist.js';
import HomeScreen from './HomeScreen.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
export type ScreenReturnType = {
homeScreen: HomeScreen | null;
- initialScreen: Screen;
- selectedSimScreens: Screen[];
- screens: Screen[];
+ initialScreen: AnyScreen;
+ selectedSimScreens: AnyScreen[];
+ screens: AnyScreen[];
allScreensCreated: boolean;
};
@@ -33,15 +33,15 @@
* @returns - duck-typed for tests
* @throws Error if incompatible data is provided
*/
-export default function selectScreens( allSimScreens: Screen[],
+export default function selectScreens( allSimScreens: AnyScreen[],
homeScreenQueryParameter: boolean,
homeScreenQueryParameterProvided: boolean,
initialScreenIndex: number,
initialScreenQueryParameterProvided: boolean,
screensQueryParameter: number[],
screensQueryParameterProvided: boolean,
- setupScreens: ( screens: Screen[] ) => void,
- createHomeScreen: ( screens: Screen[] ) => HomeScreen ): ScreenReturnType {
+ setupScreens: ( screens: AnyScreen[] ) => void,
+ createHomeScreen: ( screens: AnyScreen[] ) => HomeScreen ): ScreenReturnType {
if ( allSimScreens.length === 1 && homeScreenQueryParameterProvided && homeScreenQueryParameter ) {
const errorMessage = 'cannot specify homeScreen=true for single-screen sims';
Index: main/number-play/js/common/view/numberPlayUtteranceQueue.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/number-play/js/common/view/numberPlayUtteranceQueue.ts b/main/number-play/js/common/view/numberPlayUtteranceQueue.ts
--- a/main/number-play/js/common/view/numberPlayUtteranceQueue.ts (revision d7c72aec4c1b161c38745e531fdbb4e13c2a5549)
+++ b/main/number-play/js/common/view/numberPlayUtteranceQueue.ts (date 1682946560374)
@@ -16,7 +16,7 @@
import numberPlayPreferences from '../model/numberPlayPreferences.js';
import TProperty from '../../../../axon/js/TProperty.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import Screen from '../../../../joist/js/Screen.js';
+import Screen, { AnyScreen } from '../../../../joist/js/Screen.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
import StringProperty from '../../../../axon/js/StringProperty.js';
@@ -46,7 +46,7 @@
* to use for a given screen that the user is viewing. This is needed because selectedScreenProperty doesn't exist
* yet during the creation of this singleton.
*/
- public initialize( selectedScreenProperty: TReadOnlyProperty<Screen> ): void {
+ public initialize( selectedScreenProperty: TReadOnlyProperty<AnyScreen> ): void {
const speechDataProperty = new DerivedProperty(
[ this.tenScreenSpeechDataProperty, this.twentyScreenSpeechDataProperty, selectedScreenProperty ],
Index: main/joist/js/KeyboardHelpButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/KeyboardHelpButton.ts b/main/joist/js/KeyboardHelpButton.ts
--- a/main/joist/js/KeyboardHelpButton.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/KeyboardHelpButton.ts (date 1682945791859)
@@ -18,7 +18,7 @@
import JoistButton, { JoistButtonOptions } from './JoistButton.js';
import JoistStrings from './JoistStrings.js';
import KeyboardHelpDialog from './KeyboardHelpDialog.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
import PickRequired from '../../phet-core/js/types/PickRequired.js';
import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js';
@@ -31,7 +31,7 @@
class KeyboardHelpButton extends JoistButton {
- public constructor( screens: Screen[], screenProperty: Property<Screen>,
+ public constructor( screens: AnyScreen[], screenProperty: Property<AnyScreen>,
backgroundColorProperty: TReadOnlyProperty<Color>,
providedOptions: KeyboardHelpButtonOptions ) {
Index: main/joist/js/KeyboardHelpDialog.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/KeyboardHelpDialog.ts b/main/joist/js/KeyboardHelpDialog.ts
--- a/main/joist/js/KeyboardHelpDialog.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/KeyboardHelpDialog.ts (date 1682945814959)
@@ -18,7 +18,7 @@
import Dialog, { DialogOptions } from '../../sun/js/Dialog.js';
import joist from './joist.js';
import JoistStrings from './JoistStrings.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
// constants
const TITLE_MAX_WIDTH = 670;
@@ -32,7 +32,7 @@
export default class KeyboardHelpDialog extends Dialog {
private readonly disposeKeyboardHelpDialog: () => void;
- public constructor( screens: Screen[], screenProperty: Property<Screen>, providedOptions?: KeyboardHelpDialogOptions ) {
+ public constructor( screens: AnyScreen[], screenProperty: Property<AnyScreen>, providedOptions?: KeyboardHelpDialogOptions ) {
const options = optionize<KeyboardHelpDialogOptions, SelfOptions, DialogOptions>()( {
titleAlign: 'center',
Index: main/joist/js/HomeScreenModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/HomeScreenModel.ts b/main/joist/js/HomeScreenModel.ts
--- a/main/joist/js/HomeScreenModel.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/HomeScreenModel.ts (date 1682945738255)
@@ -13,14 +13,14 @@
import IntentionalAny from '../../phet-core/js/types/IntentionalAny.js';
import Tandem from '../../tandem/js/Tandem.js';
import joist from './joist.js';
-import Screen from './Screen.js';
+import Screen, { AnyScreen } from './Screen.js';
import TModel from './TModel.js';
class HomeScreenModel implements TModel {
- public simScreens: Screen[]; // screens in the simulations that are not the HomeScreen
- public screenProperty: Property<Screen>;
- public selectedScreenProperty: Property<Screen>;
- public readonly activeSimScreensProperty: ReadOnlyProperty<Screen[]>;
+ public simScreens: AnyScreen[]; // screens in the simulations that are not the HomeScreen
+ public screenProperty: Property<AnyScreen>;
+ public selectedScreenProperty: Property<AnyScreen>;
+ public readonly activeSimScreensProperty: ReadOnlyProperty<AnyScreen[]>;
/**
* @param screenProperty - the screen that is displayed to the user in the main area above the
@@ -28,7 +28,7 @@
* @param simScreens
* @param tandem
*/
- public constructor( screenProperty: Property<Screen<IntentionalAny, IntentionalAny>>, simScreens: Screen<IntentionalAny, IntentionalAny>[], activeSimScreensProperty: ReadOnlyProperty<Screen[]>, tandem: Tandem ) {
+ public constructor( screenProperty: Property<Screen<IntentionalAny, IntentionalAny>>, simScreens: Screen<IntentionalAny, IntentionalAny>[], activeSimScreensProperty: ReadOnlyProperty<AnyScreen[]>, tandem: Tandem ) {
this.simScreens = simScreens;
this.screenProperty = screenProperty;
Index: main/joist/js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/Sim.ts b/main/joist/js/Sim.ts
--- a/main/joist/js/Sim.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/Sim.ts (date 1682945952896)
@@ -50,7 +50,7 @@
import PreferencesModel from './preferences/PreferencesModel.js';
import Profiler from './Profiler.js';
import QueryParametersWarningDialog from './QueryParametersWarningDialog.js';
-import Screen from './Screen.js';
+import Screen, { AnyScreen } from './Screen.js';
import ScreenSelectionSoundGenerator from './ScreenSelectionSoundGenerator.js';
import ScreenshotGenerator from './ScreenshotGenerator.js';
import selectScreens from './selectScreens.js';
@@ -146,20 +146,20 @@
public readonly stepSimulationAction: PhetioAction<[ number ]>;
// the ordered list of sim-specific screens that appear in this runtime of the sim
- public readonly simScreens: Screen[];
+ public readonly simScreens: AnyScreen[];
// all screens that appear in the runtime of this sim, with the homeScreen first if it was created
- public readonly screens: Screen[];
+ public readonly screens: AnyScreen[];
// the displayed name in the sim. This depends on what screens are shown this runtime (effected by query parameters).
public readonly displayedSimNameProperty: TReadOnlyProperty<string>;
- public readonly selectedScreenProperty: Property<Screen>;
+ public readonly selectedScreenProperty: Property<AnyScreen>;
// true if all possible screens are present (order-independent)
private readonly allScreensCreated: boolean;
private availableScreensProperty!: Property<number[]>;
- public activeSimScreensProperty!: ReadOnlyProperty<Screen[]>;
+ public activeSimScreensProperty!: ReadOnlyProperty<AnyScreen[]>;
// 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.
@@ -256,7 +256,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
*/
- public constructor( simNameProperty: TReadOnlyProperty<string>, allSimScreens: Screen[], providedOptions?: SimOptions ) {
+ public constructor( simNameProperty: TReadOnlyProperty<string>, allSimScreens: AnyScreen[], providedOptions?: SimOptions ) {
window.phetSplashScreenDownloadComplete();
@@ -507,7 +507,7 @@
this.screens = screenData.screens;
this.allScreensCreated = screenData.allScreensCreated;
- this.selectedScreenProperty = new Property<Screen>( screenData.initialScreen, {
+ this.selectedScreenProperty = new Property<AnyScreen>( screenData.initialScreen, {
tandem: screensTandem.createTandem( 'selectedScreenProperty' ),
phetioFeatured: true,
phetioDocumentation: 'Determines which screen is selected in the simulation',
@@ -728,7 +728,7 @@
animationFrameTimer.runOnNextTick( () => phet.joist.display.updateDisplay() );
}
- private finishInit( screens: Screen[] ): void {
+ private finishInit( screens: AnyScreen[] ): void {
_.each( screens, screen => {
screen.view.layerSplit = true;
Index: main/beers-law-lab/js/common/view/BLLSim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/beers-law-lab/js/common/view/BLLSim.ts b/main/beers-law-lab/js/common/view/BLLSim.ts
--- a/main/beers-law-lab/js/common/view/BLLSim.ts (revision 93d4a82f5804d83f550ade53bdbb84c95e109044)
+++ b/main/beers-law-lab/js/common/view/BLLSim.ts (date 1682946503419)
@@ -8,7 +8,7 @@
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
import Sim, { SimOptions } from '../../../../joist/js/Sim.js';
-import Screen from '../../../../joist/js/Screen.js';
+import { AnyScreen } from '../../../../joist/js/Screen.js';
import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
import PickOptional from '../../../../phet-core/js/types/PickOptional.js';
import beersLawLab from '../../beersLawLab.js';
@@ -23,7 +23,7 @@
export default class BLLSim extends Sim {
- public constructor( simNameProperty: TReadOnlyProperty<string>, screens: Screen[], providedOptions?: BLLSimOptions ) {
+ public constructor( simNameProperty: TReadOnlyProperty<string>, screens: AnyScreen[], providedOptions?: BLLSimOptions ) {
const options = optionize<BLLSimOptions, SelfOptions, SimOptions>()( {
Index: main/joist/js/NavigationBarScreenButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/NavigationBarScreenButton.ts b/main/joist/js/NavigationBarScreenButton.ts
--- a/main/joist/js/NavigationBarScreenButton.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/NavigationBarScreenButton.ts (date 1682945864928)
@@ -21,7 +21,7 @@
import PushButtonModel from '../../sun/js/buttons/PushButtonModel.js';
import HighlightNode from './HighlightNode.js';
import joist from './joist.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
import PickRequired from '../../phet-core/js/types/PickRequired.js';
// constants
@@ -37,7 +37,7 @@
class NavigationBarScreenButton extends Voicing( Node ) {
private readonly buttonModel: PushButtonModel;
- public readonly screen: Screen;
+ public readonly screen: AnyScreen;
/**
* @param navigationBarFillProperty - the color of the navbar, as a string.
@@ -47,8 +47,8 @@
* @param navBarHeight
* @param [providedOptions]
*/
- public constructor( navigationBarFillProperty: TReadOnlyProperty<Color>, screenProperty: Property<Screen>,
- screen: Screen, simScreenIndex: number, navBarHeight: number,
+ public constructor( navigationBarFillProperty: TReadOnlyProperty<Color>, screenProperty: Property<AnyScreen>,
+ screen: AnyScreen, simScreenIndex: number, navBarHeight: number,
providedOptions: NavigationBarScreenButtonOptions ) {
assert && assert( screen.nameProperty.value, `name is required for screen ${simScreenIndex}` );
Index: main/joist/js/NavigationBar.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/NavigationBar.ts b/main/joist/js/NavigationBar.ts
--- a/main/joist/js/NavigationBar.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/NavigationBar.ts (date 1682945711853)
@@ -40,7 +40,7 @@
import Sim from './Sim.js';
import ReadOnlyProperty from '../../axon/js/ReadOnlyProperty.js';
import Bounds2 from '../../dot/js/Bounds2.js';
-import Screen from './Screen.js';
+import Screen, { AnyScreen } from './Screen.js';
import BooleanProperty from '../../axon/js/BooleanProperty.js';
// constants
@@ -246,7 +246,7 @@
} )!.width );
const maxScreenButtonHeight = _.maxBy( screenButtons, button => button.height )!.height;
- const screenButtonMap = new Map<Screen, Node>();
+ const screenButtonMap = new Map<AnyScreen, Node>();
screenButtons.forEach( screenButton => {
screenButtonMap.set( screenButton.screen, new AlignBox( screenButton, {
excludeInvisibleChildrenFromBounds: true,
Index: main/joist/js/ScreenSelectionSoundGenerator.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/ScreenSelectionSoundGenerator.ts b/main/joist/js/ScreenSelectionSoundGenerator.ts
--- a/main/joist/js/ScreenSelectionSoundGenerator.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/ScreenSelectionSoundGenerator.ts (date 1682945887269)
@@ -11,12 +11,12 @@
import SoundClip, { SoundClipOptions } from '../../tambo/js/sound-generators/SoundClip.js';
import screenSelection_mp3 from '../sounds/screenSelection_mp3.js';
import joist from './joist.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
import HomeScreen from './HomeScreen.js';
class ScreenSelectionSoundGenerator extends SoundClip {
- public constructor( screenProperty: ReadOnlyProperty<Screen>, homeScreen: HomeScreen | null, options?: SoundClipOptions ) {
+ public constructor( screenProperty: ReadOnlyProperty<AnyScreen>, homeScreen: HomeScreen | null, options?: SoundClipOptions ) {
super( screenSelection_mp3, options );
Index: main/joist/js/toolbar/Toolbar.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/toolbar/Toolbar.ts b/main/joist/js/toolbar/Toolbar.ts
--- a/main/joist/js/toolbar/Toolbar.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/toolbar/Toolbar.ts (date 1682945992734)
@@ -31,7 +31,7 @@
import VoicingToolbarAlertManager from './VoicingToolbarAlertManager.js';
import VoicingToolbarItem from './VoicingToolbarItem.js';
import LookAndFeel from '../LookAndFeel.js';
-import Screen from '../Screen.js';
+import Screen, { AnyScreen } from '../Screen.js';
// constants
const MAX_ANIMATION_SPEED = 250; // in view coordinates per second, assuming 60 fps
@@ -91,7 +91,7 @@
private readonly contentMargin: number;
private readonly disposeToolbar: () => void;
- public constructor( enabledProperty: TReadOnlyProperty<boolean>, selectedScreenProperty: TReadOnlyProperty<Screen>,
+ public constructor( enabledProperty: TReadOnlyProperty<boolean>, selectedScreenProperty: TReadOnlyProperty<AnyScreen>,
lookAndFeel: LookAndFeel, providedOptions?: ToolbarOptions ) {
const options = optionize<ToolbarOptions, SelfOptions, NodeOptions>()( {
Index: main/joist/js/HomeScreen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/HomeScreen.ts b/main/joist/js/HomeScreen.ts
--- a/main/joist/js/HomeScreen.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/HomeScreen.ts (date 1682945011801)
@@ -17,6 +17,7 @@
import joist from './joist.js';
import JoistStrings from './JoistStrings.js';
import Screen, { ScreenOptions } from './Screen.js';
+import IntentionalAny from '../../phet-core/js/types/IntentionalAny.js';
// constants
const homeStringProperty = JoistStrings.a11y.homeStringProperty;
@@ -32,9 +33,9 @@
public constructor(
simNameProperty: TReadOnlyProperty<string>,
- getScreenProperty: () => Property<Screen>,
- simScreens: Screen[],
- activeSimScreensProperty: ReadOnlyProperty<Screen[]>,
+ getScreenProperty: () => Property<Screen<IntentionalAny, IntentionalAny>>,
+ simScreens: Screen<IntentionalAny, IntentionalAny>[],
+ activeSimScreensProperty: ReadOnlyProperty<Screen<IntentionalAny, IntentionalAny>[]>,
providedOptions: HomeScreenOptions
) {
Index: main/joist/js/toolbar/VoicingToolbarAlertManager.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/toolbar/VoicingToolbarAlertManager.ts b/main/joist/js/toolbar/VoicingToolbarAlertManager.ts
--- a/main/joist/js/toolbar/VoicingToolbarAlertManager.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/toolbar/VoicingToolbarAlertManager.ts (date 1682946007972)
@@ -11,17 +11,17 @@
import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js';
import { SpeakableResolvedResponse } from '../../../utterance-queue/js/ResponsePacket.js';
import joist from '../joist.js';
-import Screen from '../Screen.js';
+import { AnyScreen } from '../Screen.js';
class VoicingToolbarAlertManager {
// The active Screen for the simulation, to generate Voicing descriptions that are related to the active screen.
- private readonly screenProperty: TReadOnlyProperty<Screen>;
+ private readonly screenProperty: TReadOnlyProperty<AnyScreen>;
/**
* @param screenProperty - indicates the active screen
*/
- public constructor( screenProperty: TReadOnlyProperty<Screen> ) {
+ public constructor( screenProperty: TReadOnlyProperty<AnyScreen> ) {
this.screenProperty = screenProperty;
}
Index: main/phet-io-test-sim/js/phet-io-test-sim/PhetIoTestSimScreen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/phet-io-test-sim/js/phet-io-test-sim/PhetIoTestSimScreen.ts b/main/phet-io-test-sim/js/phet-io-test-sim/PhetIoTestSimScreen.ts
--- a/main/phet-io-test-sim/js/phet-io-test-sim/PhetIoTestSimScreen.ts (revision 55cabc9caeb79617f555e942312ca617bb2e7c4d)
+++ b/main/phet-io-test-sim/js/phet-io-test-sim/PhetIoTestSimScreen.ts (date 1682946593523)
@@ -11,7 +11,7 @@
import PhetIoTestSimScreenView from './view/PhetIoTestSimScreenView.js';
import Tandem from '../../../tandem/js/Tandem.js';
-class PhetIoTestSimScreen extends Screen {
+class PhetIoTestSimScreen extends Screen<PhetIoTestSimModel, PhetIoTestSimScreenView> {
public constructor( tandem: Tandem ) {
Index: main/joist/js/SimInfo.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/SimInfo.ts b/main/joist/js/SimInfo.ts
--- a/main/joist/js/SimInfo.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/SimInfo.ts (date 1682945975031)
@@ -20,7 +20,7 @@
import ObjectLiteralIO from '../../tandem/js/types/ObjectLiteralIO.js';
import StringIO from '../../tandem/js/types/StringIO.js';
import joist from './joist.js';
-import Screen from './Screen.js';
+import { AnyScreen } from './Screen.js';
import packageJSON from './packageJSON.js';
import Sim from './Sim.js';
@@ -110,7 +110,7 @@
this.putInfo( 'simName', sim.simNameProperty.value );
this.putInfo( 'simVersion', sim.version );
this.putInfo( 'repoName', packageJSON.name );
- this.putInfo( 'screens', sim.screens.map( ( screen: Screen ) => {
+ this.putInfo( 'screens', sim.screens.map( ( screen: AnyScreen ) => {
const screenObject: ScreenState = {
// likely null for single screen sims, so use the sim name as a default
Index: main/number-suite-common/js/common/view/NumberSuiteCommonPreferencesNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/number-suite-common/js/common/view/NumberSuiteCommonPreferencesNode.ts b/main/number-suite-common/js/common/view/NumberSuiteCommonPreferencesNode.ts
--- a/main/number-suite-common/js/common/view/NumberSuiteCommonPreferencesNode.ts (revision 0b758f0455f7444575e254ad33bcfe62364b8352)
+++ b/main/number-suite-common/js/common/view/NumberSuiteCommonPreferencesNode.ts (date 1682946574838)
@@ -11,7 +11,7 @@
import optionize from '../../../../phet-core/js/optionize.js';
import numberSuiteCommon from '../../numberSuiteCommon.js';
import NumberSuiteCommonPreferences from '../model/NumberSuiteCommonPreferences.js';
-import Screen from '../../../../joist/js/Screen.js';
+import Screen, { AnyScreen } from '../../../../joist/js/Screen.js';
import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import SecondLanguageControl from './SecondLanguageControl.js';
@@ -65,7 +65,7 @@
/**
* Determines whether the sim is running with a screen of the specified type.
*/
- public static hasScreenType( constructor: new ( ...args: IntentionalAny[] ) => Screen ): boolean {
+ public static hasScreenType( constructor: new ( ...args: IntentionalAny[] ) => AnyScreen ): boolean {
return ( _.find( phet.joist.sim.screens, screen => screen instanceof constructor ) !== undefined );
}
}
Index: main/joist/js/HomeScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/HomeScreenView.ts b/main/joist/js/HomeScreenView.ts
--- a/main/joist/js/HomeScreenView.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/HomeScreenView.ts (date 1682945767576)
@@ -16,7 +16,7 @@
import joist from './joist.js';
import JoistStrings from './JoistStrings.js';
import ScreenView, { ScreenViewOptions } from './ScreenView.js';
-import Screen from './Screen.js';
+import Screen, { AnyScreen } from './Screen.js';
import HomeScreenModel from './HomeScreenModel.js';
import Property from '../../axon/js/Property.js';
import optionize from '../../phet-core/js/optionize.js';
@@ -35,7 +35,7 @@
class HomeScreenView extends ScreenView {
private homeScreenScreenSummaryIntroProperty!: TReadOnlyProperty<string>;
- private selectedScreenProperty: Property<Screen>;
+ private selectedScreenProperty: Property<AnyScreen>;
public screenButtons: HomeScreenButton[];
// NOTE: In https://github.com/phetsims/joist/issues/640, we attempted to use ScreenView.DEFAULT_LAYOUT_BOUNDS here.
@@ -91,7 +91,7 @@
const buttonGroupTandem = options.tandem.createTandem( 'buttonGroup' );
- this.screenButtons = _.map( model.simScreens, ( screen: Screen ) => {
+ this.screenButtons = _.map( model.simScreens, ( screen: AnyScreen ) => {
assert && assert( screen.nameProperty.value, `name is required for screen ${model.simScreens.indexOf( screen )}` );
assert && assert( screen.homeScreenIcon, `homeScreenIcon is required for screen ${screen.nameProperty.value}` );
Index: main/balancing-chemical-equations/js/balancing-chemical-equations-main.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/balancing-chemical-equations/js/balancing-chemical-equations-main.ts b/main/balancing-chemical-equations/js/balancing-chemical-equations-main.ts
--- a/main/balancing-chemical-equations/js/balancing-chemical-equations-main.ts (revision f140cc1a1a9c553d91586ccf85c63633776c8268)
+++ b/main/balancing-chemical-equations/js/balancing-chemical-equations-main.ts (date 1682946467197)
@@ -13,12 +13,10 @@
simLauncher.launch( () => {
- const screens = [
+ const sim = new Sim( BalancingChemicalEquationsStrings[ 'balancing-chemical-equations' ].titleStringProperty, [
new IntroductionScreen( Tandem.ROOT.createTandem( 'introductionScreen' ) ),
new GameScreen( Tandem.ROOT.createTandem( 'gameScreen' ) )
- ];
-
- const sim = new Sim( BalancingChemicalEquationsStrings[ 'balancing-chemical-equations' ].titleStringProperty, screens, {
+ ], {
credits: {
leadDesign: 'Kelly Lancaster (Java), Yuen-ying Carpenter (HTML5)',
softwareDevelopment: 'Chris Malley (PixelZoom, Inc.)',
Index: main/joist/js/HomeScreenButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/HomeScreenButton.ts b/main/joist/js/HomeScreenButton.ts
--- a/main/joist/js/HomeScreenButton.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/HomeScreenButton.ts (date 1682945078142)
@@ -15,6 +15,7 @@
import { Shape } from '../../kite/js/imports.js';
import merge from '../../phet-core/js/merge.js';
import optionize from '../../phet-core/js/optionize.js';
+import IntentionalAny from '../../phet-core/js/types/IntentionalAny.js';
import PhetColorScheme from '../../scenery-phet/js/PhetColorScheme.js';
import PhetFont from '../../scenery-phet/js/PhetFont.js';
import { FireListener, Node, PDOMPeer, Rectangle, Text, VBox, VBoxOptions, Voicing, VoicingOptions } from '../../scenery/js/imports.js';
@@ -36,9 +37,9 @@
export type HomeScreenButtonOptions = SelfOptions & ParentOptions & PickRequired<ParentOptions, 'tandem'>;
class HomeScreenButton extends Voicing( VBox ) {
- public readonly screen: Screen;
+ public readonly screen: Screen<IntentionalAny, IntentionalAny>;
- public constructor( screen: Screen, homeScreenModel: HomeScreenModel, providedOptions?: HomeScreenButtonOptions ) {
+ public constructor( screen: Screen<IntentionalAny, IntentionalAny>, homeScreenModel: HomeScreenModel, providedOptions?: HomeScreenButtonOptions ) {
const options = optionize<HomeScreenButtonOptions, SelfOptions, ParentOptions>()( {
cursor: 'pointer',
Index: main/joist/js/selectScreensTests.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/selectScreensTests.ts b/main/joist/js/selectScreensTests.ts
--- a/main/joist/js/selectScreensTests.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/selectScreensTests.ts (date 1682946535275)
@@ -9,13 +9,13 @@
*/
import selectScreens, { ScreenReturnType } from './selectScreens.js';
-import Screen from './Screen.js';
+import Screen, { AnyScreen } from './Screen.js';
import HomeScreen from './HomeScreen.js';
// test screen constants. Since these are tests, it is actually more valuable to typecast instead of making these actual screens.
-const a = 'a' as unknown as Screen;
-const b = 'b' as unknown as Screen;
-const c = 'c' as unknown as Screen;
+const a = 'a' as unknown as AnyScreen;
+const b = 'b' as unknown as AnyScreen;
+const c = 'c' as unknown as AnyScreen;
const hs = 'hs' as unknown as HomeScreen;
const getQueryParameterValues = ( queryString: string ) => {
@@ -56,7 +56,7 @@
/**
* Format the query string + all sim screens to uniquely identify the test.
*/
-const getDescription = ( queryString: string, allSimScreens: Screen[] ): string => `${queryString} ${JSON.stringify( allSimScreens )}`;
+const getDescription = ( queryString: string, allSimScreens: AnyScreen[] ): string => `${queryString} ${JSON.stringify( allSimScreens )}`;
QUnit.test( 'valid selectScreens', async assert => {
@@ -64,7 +64,7 @@
* Tests a valid combination of allSimScreens and screens-related query parameters, where the expectedResult should
* equal the result returned from ScreenSelector.select
*/
- const testValidScreenSelector = ( queryString: string, allSimScreens: Screen[], expectedResult: ScreenReturnType ) => {
+ const testValidScreenSelector = ( queryString: string, allSimScreens: AnyScreen[], expectedResult: ScreenReturnType ) => {
const queryParameterValues = getQueryParameterValues( queryString );
const result = selectScreens(
@@ -228,7 +228,7 @@
* Tests an invalid combination of allSimScreens and screens-related query parameters, where selectScreens should
* throw an error
*/
- const testInvalidScreenSelector = ( queryString: string, allSimScreens: Screen[] ) => {
+ const testInvalidScreenSelector = ( queryString: string, allSimScreens: AnyScreen[] ) => {
const queryParameterValues = getQueryParameterValues( queryString );
const description = getDescription( queryString, allSimScreens );
Index: main/joist/js/Screen.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/Screen.ts b/main/joist/js/Screen.ts
--- a/main/joist/js/Screen.ts (revision a88f873fd63c66c0e1300f2be0b96fbb614a9397)
+++ b/main/joist/js/Screen.ts (date 1682946754400)
@@ -72,9 +72,11 @@
// Accept any subtype of TModel (defaults to supertype), and any subtype of ScreenView (defaults to subtype).
type CreateView<M extends TModel, V> = ( model: M ) => V;
+export type AnyScreen = Screen<IntentionalAny, ScreenView>;
+
// 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 {
+class Screen<M extends TModel, V extends ScreenView> extends PhetioObject {
public backgroundColorProperty: Property<Color> | Property<string> | Property<Color | string>;
|
samreid
added a commit
to phetsims/number-compare
that referenced
this issue
May 1, 2023
samreid
added a commit
that referenced
this issue
May 1, 2023
samreid
added a commit
to phetsims/number-suite-common
that referenced
this issue
May 1, 2023
samreid
added a commit
to phetsims/beers-law-lab
that referenced
this issue
May 1, 2023
samreid
added a commit
to phetsims/number-play
that referenced
this issue
May 1, 2023
samreid
added a commit
to phetsims/balancing-chemical-equations
that referenced
this issue
May 1, 2023
|
I improved and committed the patch above. It appears to address the concerns raised by @pixelzoom in the topmost comment. @zepumph and I will meet next week to discuss. |
zepumph
added a commit
to phetsims/number-compare
that referenced
this issue
May 9, 2023
zepumph
added a commit
to phetsims/beers-law-lab
that referenced
this issue
May 9, 2023
zepumph
added a commit
to phetsims/number-play
that referenced
this issue
May 9, 2023
zepumph
added a commit
to phetsims/number-suite-common
that referenced
this issue
May 9, 2023
zepumph
added a commit
that referenced
this issue
May 9, 2023
|
@samreid and I dove into this again. The contravariance problem (where createView gets a parameter of Model for it) is inherent to how we have written Screen. This is not something that we can just TypeScript to get around. Instead, we punted, and @samreid's commits last week put the burden on joist instead of on all main files. Nice work! The worst line of code is Lines 427 to 429 in 0e249f9 because it thinks This is potentially a good-enough solution, but we will need to continue to be vigilant in Joist that we don't make a type error if we ever call functions on the model of an AnyScreen. We will continue with the "nuclear solution" over in #922 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In 1e5ca9d, @samreid made this change in Screen.ts:
I'm seeing 3 problems related to this:
(1) Developers are not using TModel for their top-level model classes. See for example phetsims/calculus-grapher#117. See also GravityAndOrbitsModel (@samreid), and many other examples.
(2) Developers are not parameterizing their Screen subclasses. See for example phetsims/calculus-grapher#117.
(3) Where developer are parameterizing their Screen subclass, the model M does not implement TModel, but tsc does not complain about this error. See for example greenhouse-effect.PhotonsScreen, where
PhotonsModelis not a modelclass PhotonsScreen extends Screen<PhotonsModel, PhotonsScreenView>@samreid why do you think this is an appropriate use of IntentionalAny?
The text was updated successfully, but these errors were encountered: