Allow `null` candidate in `RTCPeerConnection.addIceCandidate`

[silverlyra]

Add an override for addIceCandidate’s candidate parameter to allow it to be null, which is a special value indicating that ICE gathering is complete.

If no candidate object is specified, or its value is null, an end-of-candidates signal is sent to the remote peer [emphasis added]

Fixes #1738.

[github-actions[bot]]

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

[silverlyra]

@microsoft-github-policy-service agree

[saschanaz]

Passing null is same as passing undefined in this case per https://webidl.spec.whatwg.org/#js-dictionary. I'm not sure we really need a special case here?

[saschanaz]

We could instead allow null in general this case, hmm.

[silverlyra]

We could instead allow null in general this case, hmm.

I looked at the WebIDL §3.2.17 you linked to – what would you say the general fix would be? Any optional T parameter becomes nullable if T is a dictionary? Or any dictionary parameter becomes nullable if none of its members are required?

I'm not sure we really need a special case here?

It is noted as a special case not captured by WebIDL in the WebRTC specification:

Due to WebIDL processing, addIceCandidate(null) is interpreted as a call with the default dictionary present, which, in the above algorithm, indicates end-of-candidates for all media descriptions and ICE candidate generation. This is by design for legacy reasons.

I do think it’s appropriate to declare these parameters as nullable. The types right now are meaningfully incorrect – implementing WebRTC signaling is (in part) basically a process of piping icecandidate events on one peer into the addIceCandidate method of the remote peer. But TypeScript rejects this, because the event candidate property is nullable, and the method candidate parameter isn’t.

The simplest RTCPeerConnection sample just creates two peer connections in the same browser tab. If I reproduce the relevant parts of its code in TypeScript:

let pc1 = new RTCPeerConnection();
pc1.addEventListener('icecandidate', e => onIceCandidate(pc1, e));

let pc2 = new RTCPeerConnection();
pc2.addEventListener('icecandidate', e => onIceCandidate(pc2, e));

async function onIceCandidate(pc: RTCPeerConnection, event: RTCPeerConnectionIceEvent) {
  let target = pc === pc2 ? pc1 : pc2;
  await target.addIceCandidate(event.candidate);
}

it doesn’t build:

Argument of type RTCIceCandidate | null is not assignable to parameter of type RTCIceCandidateInit | undefined. Type null is not assignable to type RTCIceCandidateInit | undefined.