Skip to content
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

RtcPeerConnection::set_local_description generated documentation for required features is wrong #1569

Closed
kyren opened this issue Jun 1, 2019 · 2 comments · Fixed by #1572
Labels
bug docs Issues related to documentation frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate

Comments

@kyren
Copy link

kyren commented Jun 1, 2019

Describe the Bug

RtcPeerConnection::set_local_description docs say that the features required to use it are RtcPeerConnection and RtcSessionDescriptionInit, but it will not work without also enabling "RtcSdpType".

Also, the "RtcSessionDescriptionInit" type is not generated at all without enabling "RtcSdpType". I don't know why this is, but it might have something to do with RtcSessionDescriptionInit::new taking an RtcSdpType?

I had to do a lot of trial and error to figure out which missing feature was required (copying the entire set of Rtc* features and bisecting.

Steps to Reproduce

  1. Create a new project that uses web-sys 0.3.22 and imports web_sys::RtcSessionDescriptionInit.
  2. enable web-sys feature RtcSessionDescriptionInit
  3. get a compilation error because RtcSessionDescriptionInit it not found in web_sys

Expected Behavior

I would expect RtcSessionDescriptionInit to only require the feature which is the name of its type, and I would also expect the docs for RtcPeerConnection::set_local_description to mention all required features.

Actual Behavior

They do not, and you have to bisect the entire list of available features to find which one is missing, or know what might have gone wrong. Is there a better way to discover what features are actually required for a type / method other than the generated documentation in case it goes wrong?

@kyren kyren added the bug Something isn't working label Jun 1, 2019
@kyren
Copy link
Author

kyren commented Jun 1, 2019

Now that I know a bit more about the API, this issue makes a bit more sense. You can't create an RtcIceCandidateInit without an RtcSdpType to begin with (but you could get one from javascript I suppose). RtcPeerConnection::set_local_description is also supposed to accept an RtcIceCandidateas well I think, but that's a separate issue.

In any case, this is not a big deal now that I know about it and it has an easy workaround that you'd probably do anyway, but it's still sort of a usability issue.

@fitzgen fitzgen added frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate bug docs Issues related to documentation and removed bug Something isn't working labels Jun 3, 2019
@fitzgen
Copy link
Member

fitzgen commented Jun 3, 2019

Thanks for filing a bug!

RtcPeerConnection::set_local_description docs say that the features required to use it are RtcPeerConnection and RtcSessionDescriptionInit, but it will not work without also enabling "RtcSdpType".

We should definitely fix this docs bug. It sounds like our webidl frontend is missing features that are required because of constructors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug docs Issues related to documentation frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen web-sys Issues related to the `web-sys` crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants