Many issues with options types

[pixelzoom]

There are many issues with options types in this repo, including:

• missing type definitions for subclasses
• malformed uses of optionize
• places where combineOptions should be used instead of optionize

It's probably easist if I address them, then assign to @jessegreenberg and @zepumph to review. (I hope that's OK.)

[pixelzoom]

@jessegreenberg and/or @zepumph please review. Let me know if you have any questions, or if you see any problems.

[zepumph]

Thanks for this. There was definitely some slop there that I was glad to sort out. I have two thoughts here.

1. Announcer.announce is kinda a tricky case. We want to be able to pass in options from Utterance.announcerOptions, but when creating an announcer, it isn't parameterized to the Announcer it will announce to (by design). So we don't really have strong typing there. Thus announce methods are a bit strange to begin with, but I think your and my commits help with this.
2. We still seem to be coming to a consensus as a project about when and how to use combineOptions. The cases you had though don't seem correct to me. In each case, I prefer optionize because there are actual defaults that we want to make sure are provided, because they are used later in the function. I think that vast majority of options usages in functions will use Optionize instead of combineOptions, because, though they most likely lack parent options, they will still need the type safety as SelfOptions and ProvidedOptions come together (ensuring defaults are provided).

Please review. I'm happy to discuss further.

[pixelzoom]

I can't say that I agree with the combineOptions usages that were changed to optionize. But I'm certainly willing to be convinced. So let's discuss via Zoom.

I should also note that in AriaLiveAnnouncer.ts, I found the names AriaLiveAnnounceSelfOptions and AriaLiveAnnounceOptions to be incredibilty confusing. At first I thought there names were typos, using "Announce" instead of "Announcer". Quite awhile later, I realized that they are related to the announce method. As a general pattern, I think that an option type that is specific to a method should be named {{CLASS_NAME}}{{METHOD_NAME}}Options. There is no "AriaLive" class, it's "AriaLiveAnnouncer". So (while verbose) these types would be clearer if they were renamed AriaLiveAnnouncerAnnounceSelfOptions and AriaLiveAnnouncerAnnounceOptions respectively.

[pixelzoom]

Also note that you may have problems if you ever add options to AnnouncerAnnounceOptions in Announcer.ts. SpeechSynthesisAnnouncer overrides announce like this:

override announce( utterance: Utterance ): void

... so it's blowing off AnnouncerAnnounceOptions. That seems brittle, not a good API.

[pixelzoom]

I met with @zepumph on Slack.

He convinced me that the combineOptions usages that he changed to optionize are correct. These are cases where type-checking is needed, and the options need to satisfy the complete contract of the options type.

For the types related to AriaLiveAnnouncer announce, @zepumph agreed that AriaLiveAnnouncerAnnounceSelfOptions and AriaLiveAnnouncerAnnounceOptions are better names, so I changed them and added doc in the above commits.

For the potential problem that I noted about SpeechSynthesisAnnouncer... @zepumph is aware of this, and there are apparently bigger design problems with the "announce" API. So we'll ignore that for this issue, and maybe address it in another time/place.

Finally... We identified a potentail improvement to optionize3, which @zepumph is investigating in https://github.com/phetsims/phet-core/issues/118.

Closing.