Convert to typescript and resolve API type errors

[zepumph]

This means a couple of things:

• resolve the difference between the type that Voicing supports, and Description, and how that works for UtteranceQueue.
• Remove the support for looping alerts in Utterance.
• Convert files to typescript, and make sure that all typing exists harmoniously.
• Get rid of AlertableDef.
• make cancelUtterance and announce abstract after converting SpeechSynthesisUtterance to typescript

Note phetsims/tasks#1096

[zepumph]

The main thought @jessegreenberg and I had this afternoon was to have Utterances support functions. And update Utterance so that it is easier to get the "alert text" from an Utterance to pass to voicing .This will better align the two APIs. Mainly this work was brought about by phetsims/sun#742.

[zepumph]

• One thing to discuss with this is that I made null an acceptable member of IAlertable. I want to talk about how to handle that case. Perhaps if the Utterance has null alert, then we just ignore it. @jessegreenberg what do you think?

[jessegreenberg]

Was reviewing this issue and noticed that we lost history for Utterance, Ill see if I can restore it. Other TS files in utterance-queue have history still.

[jessegreenberg]

Here is how I ended up doing it. Im sure there is a cleaner way with git without having to manually revert, but some other things seemed too risky or had the same result of not saving history.

• Manually revert all commits (7) until the one before the TypeScript converson.
• git cherry-pick -n edaef5b00f3308912faf3f2f9abcf4e30ca22462 (the TS conversion SHA)
• git restore --staged . to unstage everything
• git add everything except Utterance.ts and Utterance.js
• Delete Utterance.ts and restore Utterance.js
• Commit this (with --no-verify)
• Rename Utterance.js -> Utterance.ts and commit (with --no-verify)
• Manually copy change from edaef5b into Utterance.ts.
• Cherry-pick the commits after the TypeScript conversion back onto master.

After all of this I verified that we still have history for all .ts files in utterance-queue.

[jessegreenberg]

One thing to discuss with this is that I made null an acceptable member of IAlertable.

I am not sure I understand but this seems fine to me. null was the default value for alert before this change so hopefully it should be supported. null is a valid value of ResolvedResponse so do we also need it in the type for _alert?

  private _alert: AlertableNoUtterance | null;

[zepumph]

Oh wow. Thank you so much! I really appreciate you picking up the git history slack. I should have know better.

I removed duplicate null types, and generally cleaned it up. Thank you!

I'm not yet confident that UtteranceQueue is totally set up to our new Utterance.alert type, I want to take another check on this. And then perhaps convert a bit more to typescript and check the rest of the boxes above.

[zepumph]

OK. I believe that I'm done with this issue. @jessegreenberg, everything in this batch of commits here was a straight forward refactor except one item. I found that it felt sketchy to have a getSynth() that could return null on a platform that doesn't support speech synthesis. We have some support for that case, but I think we rely on assertions a lot to guard against null pointer exceptions. In the wild, we wouldn't have assertions so we may want to be graceful. I added a couple of spots that checked if we had a synth before calling operations on it. Does this seem correct to you? I'd hate for the sim to break just because the platform doesn't have speech synthesis. For example, in the app, right? Should we make a separate issue for this?

Here is an example:

utterance-queue/js/SpeechSynthesisAnnouncer.ts

Lines 340 to 348 in 38f0f33

	private populateVoices(): void {
	const synth = this.getSynth();
	if ( synth ) {
	// the browser sometimes provides duplicate voices, prune those out of the list
	this.voices = _.uniqBy( synth.getVoices(), voice => voice.name );
	this.voicesChangedEmitter.emit();
	}
	}

Can you please spot check the conversion. Thanks!

[zepumph]

Oops sorry, I forgot to assign you.

[jessegreenberg]

I took a look at the TS conversion and things look good. I mostly reviewed the public API like options. I also verified that we still have history for everything.

I found that it felt sketchy to have a getSynth() that could return null on a platform that doesn't support speech synthesis.

I have thoughts, but lets create a new issue.

Back to you to close if it is time.