Skip to content

Add WebRTC Stats spec #919

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

Merged
merged 1 commit into from
Dec 12, 2020
Merged

Add WebRTC Stats spec #919

merged 1 commit into from
Dec 12, 2020

Conversation

trptcolin
Copy link
Contributor

This proposes bringing in the WebRTC Stats spec. The driver here is updating the RTCStatsType enum values, as described in microsoft/TypeScript#36650, but there are other changes that the README-recommended fetch-idl process brings in as well.

My initial thinking was that getting all the spec updates in is preferable, but I'm also not totally sure where browser adoption is for this spec, so I'm not sure what level belongs in TypeScript. Could isolate to just RTCStatsType to mitigate risk.

Side note: importing this spec via the README instructions did require a bit of editing to remove the text "WebIDL" in front of each WebIDL block, due to a .idlHeader element with that text appearing inside the .idl code block. It looks like a number of other specs have a similar thing going on:

  • Gamepad
  • Push
  • Screen Orientation
  • Selection
  • Web Share
  • and this one, WebRTC Stats

I'm happy to open a separate issue to suggest some kind of filtering to avoid similar manual IDL-editing effort in the future if that makes sense - just let me know.

Along other updates, this updates the values in RTCStatsType, which
addresses microsoft/TypeScript#36650
@trptcolin trptcolin requested a review from sandersn as a code owner September 23, 2020 02:06
@ghost
Copy link

ghost commented Sep 23, 2020

CLA assistant check
All CLA requirements met.

@saschanaz
Copy link
Contributor

I guess it's relatively safer to add experimental dictionary fields, as it won't cause "undefined is not a function" error.

For .idlHeader issue this should work: 97c3235

@orta
Copy link
Contributor

orta commented Dec 11, 2020

We're struggling with getting the adoption info for this also, querying caniuse for some of the identifiers in here either just in Firefox or just in Safari - IMO, if you constrain it to just RTCStatsType we'll merge but until then we'll probably need to see some larger evidence of use 👍🏻

@saschanaz
Copy link
Contributor

Using Searchfox and webkit-search can be helpful for that.

I can see at least some fields are supported in Firefox, but not all.

@orta
Copy link
Contributor

orta commented Dec 12, 2020

Thanks @saschanaz - those two sites are really useful (I've added them to the README so we can use it more often) - I think on the whole most of these new fields are in FF/Safari which means it's reasonable to merge 👍🏻

@orta orta merged commit b441061 into microsoft:master Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants