Review TypeScript conversion

[jbphet]

In today's developer/typescript meeting we decided to have set of reviews on new and converted TypeScript code in order to get feedback, cross pollinate techniques and ideas, and generally improve the quality of the code. I requested that tambo be reviewed, and @jonathanolson was assigned to do it.

I've set up a meeting for tomorrow (4/15/2022) with @jonathanolson to review some specific questions that I have and set some general direction for the review.

[jbphet]

For reference, the issue under which the TS port was done is #160.

[jonathanolson]

The typescript overall looks great! I feel like I only have nit-picky things where I've done things differently. Notably:

• [x] There are cases of @public, @private, @constructor, etc. that are unneeded (e.g. AmplitudeModulator, MultiSelectionSoundPlayerFactory)
• [x] There are cases of duplicated documentation and type info at field declaration and when they are set in constructors (e.g. AmplitudeModulator, ContinuousPropertySoundGenerator.remainingFadeTime, MultiClip.audioContextStateChangeListener). Presumably they should only be documented/typed/visibilitied at the field declaration site.
• [x] There are cases of missing return types in many places (.e.g base64SoundToByteArray, BinMapper.mapToBin)
• [x] There are cases of duplicated documentation and type info in the type declarations AND in the optionize call. soundManager.addSoundGenerator is showing types and documentation in its optionize - that should be moved (or only) in the type, so we're not duplicating? NoiseGenerator also.
• [x] Some things like soundManager.masterOutputLevel/reverbLevel, MultiClip.playAssociatedSound don't document parameter types
• [x] BinMapperOptions has a required option but is itself optional in the constructor of BinMapper? Expect to either be required in constructor, or optional in the type.
• [x] Would expect providedOptions/optionize in BinMapper constructor
• [x] Should SoundGeneratorInitializationOptions's categories be a string union (cheap enumeration) instead of allowing any strings?
• [x] Lots of fields are being made public as NumberProperty/BooleanProperty. This prevents swapping them with DerivedProperty/etc. in the future. Do we want to type them more generally, e.g. Property?
• [x] Some required providedOptions with an Options type with nothing required (MultiClip). Presumably use providedOptions?:
• [ ] Cases like PitchedPopGenerator's local variables (oscillator/gainNode) where the type is documented as a comment could be changed to explicit type declarations, e.g. const oscillator: OscillatorNode =

EDIT: I (@jbphet) changed the list to checkboxes.

[jbphet]

I've addressed all of the suggestions - thanks for the review @jonathanolson! Closing.