phetsims / tasks

General tasks for PhET that will be tracked on Github
MIT License
5 stars 2 forks source link

Convert common code to Typescript #1096

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

Devs- JO(20), JG(20), CM(20), JB(40), SR(40), CK(20), MK(8)

*Design Pattern work happens ahead of this work

Priority

Proactive conversions

scenery-phet items in typescript sims:

[ "TimeSpeed.js", "PhetFont.js", "ArrowNode.js", "MeasuringTapeNode.js", "PhetColorScheme.js", "ResetAllButton.js", "RestartButton.js", "TimeControlNode.js", "StopwatchNode.js", "NumberControl.js", "ArrowShape.js", "KeyboardHelpSection.js", "TextKeyNode.js", "RulerNode.js", "MagnifyingGlassZoomButtonGroup.js", "SceneryPhetConstants.js", "sceneryPhetStrings.js", "LetterKeyNode.js", "NumberKeyNode.js", "ArrowKeyNode.js", "NumberPicker.js", "LockNode.js", "NumberDisplay.js", "WireNode.js", "Alerter.js", "MathSymbols.js", "ThermometerNode.js", "ShadedSphereNode.js", "MultiLineText.js", "WavelengthSpectrumNode.js", "FaceNode.js", "StarNode.js", "PlayIconShape.js" ]

sun items in typescript sims:

[ "HSlider.js", "Checkbox.js", "HSeparator.js", "TextPushButton.js", "Panel.js", "AquaRadioButton.js", "RectangularRadioButtonGroup.js", "RectangularPushButton.js", "VSlider.js", "AccessibleSlider.js", "AccordionBox.js", "ComboBox.js", "ComboBoxItem.js", "ToggleNode.js", "AquaRadioButtonGroup.js", "VerticalAquaRadioButtonGroup.js", "VSeparator.js", "BooleanRoundToggleButton.js", "VerticalCheckboxGroup.js", "ABSwitch.js", "ArrowButton.js", "ClosestDragListener.js", "Carousel.js", "Dialog.js" ]

scenery items in any typescript file:

[ "Line", "Node", "PAINTABLE_DEFAULT_OPTIONS", "Rectangle", "Color", "CanvasNode", "Path", "LayoutBox", "Text", "VBox", "HBox", "RichText", "DragListener", "Utils", "LinearGradient", "NodeOptions", "NodeProperty", "SimpleDragHandler", "Image", "SceneryEvent", "Trail", "WebGLNode", "ShaderProgram", "HStrut", "Circle", "VStrut", "AlignGroup", "KeyboardUtils", "AlignBox", "FireListener", "Gradient", "ProfileColorProperty", "Font", "KeyboardDragListener", "ColorDef", "FocusHighlightFromNode", "SceneryConstants", "colorProfileProperty", "scenery", "PathOptions", "ColorProperty", "Pointer", "voicingManager", "FocusHighlightPath", "Voicing", "globalKeyStateTracker", "ParallelDOM", "PDOMPeer", "HeightSizable", "WidthSizable" ]
pixelzoom commented 2 years ago

My few experiences converting common-code to TS have been (mostly) smooth. The one thing I bump into all the time is how to handle options.

For example, AquaRadioButtonGroup is a subclass of LayoutBox, so conversion should look like:

type AquaRadioButtonGroupOptions = Omit< LayoutBoxOptions, 'children' >;

class AquaRadioButtonGroup<T> extends LayoutBox {
  constructor( property: Property<T>, items: AquaRadioButtonGroupItem<T>[], options?: AquaRadioButtonGroupOptions ) {

But LayoutBox has not been converted to TS, and LayoutBoxOptions does not exist. So I did this in the meantime:

//TODO https://github.com/phetsims/phet-core/issues/128 replace any with LayoutBoxOptions, when it exists
type AquaRadioButtonGroupOptions = Omit< any, 'children' >;

So it seems like we should give highest priority to converting more sim-facing scenery types.

zepumph commented 2 years ago

Yes understood, in the quarterly goals we decided that https://github.com/phetsims/tasks/issues/1097 was a precursor to this issue. I marked https://github.com/phetsims/phet-core/issues/128 as high priority to try to unblock these conversions as quickly as possible.

chrisklus commented 2 years ago

From 1/27/22 dev meeting: We checked in on this and @zepumph recommended we hold off until more Design Pattern work (like options patterns) is completed. We're thinking ~3 weeks will be a good timeline.

zepumph commented 2 years ago

As of dev meeting today, @jonathanolson has been converting a lot of scenery, simula-rasa is in typescript.

Joist seems important to do before converting much sim code.

How to limit technical debt as we get to some files but not others.

@jonathanolson will sprint on scenery conversions to help aid other common code for later in the quarter.

@samreid: We are not going to be done with this issue when our quarterly hours are up, and that's ok.

@samreid: what if we convert all files to .ts files, and add a no-check directive to the top.

samreid commented 2 years ago

Let’s create a TodoAny type to be used instead of any

Thinking this through a bit more, currently 99% of my (or maybe everyone’s) any are the “todo” type. Maybe it would be better to be explicit about when an any is supposed to be any. We could call it ANY_BY_DESIGN or INTENTIONAL_ANY . That seems a better match for where we are at. I’ll also comment this in the issue.

zepumph commented 2 years ago

IntentionalAny.ts was added based on a conversation in slack. Any presence of any in the project will be considered suspect.

zepumph commented 2 years ago

Using it in some mixins above.

samreid commented 2 years ago

IntentionalAny has been working great, thanks!

zepumph commented 2 years ago

From quarterly checkin today:

zepumph commented 2 years ago

From today's typescript conversion:

SR: It has been helpful for me to convert common code as I run into it while developing my sims.

We will create an issue to convert Dialog and Popupable.

Status of conversion in lines of code: scenery-phet JS: 13958 total, TS 12622 total sun: 878 JS, 13484 TS scenery: 37918 JS, 45377 TS axon: 2823 JS, 5032 TS joist: 7446 JS, 5736 TS

We are not complete with TS conversion, but this issue has run it's course. We are calling this sprint complete, and are ready to close.