Skip to content

Add webidl optional support to web-sys #502

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

Closed
richard-uk1 opened this issue Jul 18, 2018 · 4 comments
Closed

Add webidl optional support to web-sys #502

richard-uk1 opened this issue Jul 18, 2018 · 4 comments

Comments

@richard-uk1
Copy link
Contributor

richard-uk1 commented Jul 18, 2018

Currently optional parameters are forced. I think they should be an Option<T>.

This depends on #14 to generate the correct glue code, provided the way options are represented in the end is compatible with these APIs.

Also, in the spec, optional parameters can have a default. This doesn't really affect the rust function, but it could be mentioned in some generated documentation, perhaps.

WebIDL snippet from the Console api (console.*):

[UseCounter]
void count(optional DOMString label = "default");

EDITS

  • 2018-07-18 Added more detail.
@alexcrichton
Copy link
Contributor

alexcrichton commented Jul 18, 2018

Unfortunately Option<T> is pretty hairy to support, so we may want to also brainstorm other ideas of how to not rely on #14. One option may be to export multiple bindings for optional methods, for example the above could become:

#[wasm_bindgen]
extern {
    fn count();
    #[wasm_bindgen(js_name = count)]
    fn count2(label: &str);
}

So perhaps new names could be manufactured for invocations with optional arguments specified?

@ohanar
Copy link
Member

ohanar commented Jul 18, 2018

This also ties into dealing with function overloading, which we need to deal with for proper support of WebIDL (in particular, web-sys needs it for constructors). I think what @alexcrichton suggests is basically what rust-bindgen does right now for C++ (see also rust-lang/rust-bindgen#334).

@fitzgen
Copy link
Member

fitzgen commented Jul 18, 2018

What rust-bindgen does is pretty sucky, and I hope we can come up with something better. Even just a better mangling algorithm would be an improvement, eg

#[wasm_bindgen]
extern {
    fn count();
    #[wasm_bindgen(js_name = count)]
    fn count_label(label: &str); // concatenate each optional argument's name to the method name
}

@richard-uk1
Copy link
Contributor Author

I can do this for now. I'm doing the console API, and maybe having a concrete implementation to see will aid bikeshedding.

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Jul 19, 2018
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

No branches or pull requests

4 participants