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

Convert common code to Typescript #1096

Closed
5 of 10 tasks
zepumph opened this issue Jan 6, 2022 · 10 comments
Closed
5 of 10 tasks

Convert common code to Typescript #1096

zepumph opened this issue Jan 6, 2022 · 10 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jan 6, 2022

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

  1. Review Sims that are in TS and convert CC based upon these as priorities
  2. Create a ranked list

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
Copy link
Contributor

pixelzoom commented Jan 7, 2022

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
Copy link
Member Author

zepumph commented Jan 7, 2022

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

@chrisklus
Copy link

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
Copy link
Member Author

zepumph commented Feb 10, 2022

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.

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

@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
Copy link
Member

samreid commented Feb 10, 2022

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 added a commit to phetsims/phet-core that referenced this issue Feb 10, 2022
@zepumph
Copy link
Member Author

zepumph commented Feb 10, 2022

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

zepumph added a commit to phetsims/scenery that referenced this issue Feb 10, 2022
@zepumph
Copy link
Member Author

zepumph commented Feb 10, 2022

Using it in some mixins above.

@samreid
Copy link
Member

samreid commented Feb 18, 2022

IntentionalAny has been working great, thanks!

@zepumph
Copy link
Member Author

zepumph commented Mar 31, 2022

From quarterly checkin today:

  • JB: Tambo is pretty much done. Sun 57 files TS, 30 files JS
  • CM: Sun demo converted, lots of scenery phet converted, ~30-40 files. "Let's not convert deprecated files" (with a resounding group "YAY").
  • MK: Lots in utterance-queue and accessibility mixins
  • JG: Scenery types, and a few hours working on ts lint. I worked about 20, but need about 20 more for scenery accessibility feature internals
  • JO: Converted: most of the public facing stuff in scenery, most of Kite except shape, many highly used stuff in dot, and buoyancy. Worked on poolable mixin pattern.
  • SR: converted common code for 14 hours (16 allocated), most of this was when running into it from center-and-variablility.

@zepumph
Copy link
Member Author

zepumph commented May 12, 2022

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.

@zepumph zepumph closed this as completed May 12, 2022
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

4 participants