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

Support WebIDL callback functions #257

Closed
fitzgen opened this issue Jun 8, 2018 · 9 comments
Closed

Support WebIDL callback functions #257

fitzgen opened this issue Jun 8, 2018 · 9 comments
Labels
closures Adding more support for closures on the boundary frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen help wanted We could use some help fixing this issue!

Comments

@fitzgen
Copy link
Member

fitzgen commented Jun 8, 2018

https://heycam.github.io/webidl/#idl-callback-functions

Example:

callback AsyncOperationCallback = void (DOMString status);

interface AsyncOperations {
  void performOperation(AsyncOperationCallback whenFinished);
};

Should probably translate these into FnMove(...) -> ... since we can't be sure about the way the interfaces might use the callbacks.

@fitzgen fitzgen added closures Adding more support for closures on the boundary frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen help wanted We could use some help fixing this issue! labels Jun 8, 2018
@Pauan
Copy link
Contributor

Pauan commented Jun 8, 2018

What's an FnMove? Did you mean FnMut?

@fitzgen
Copy link
Member Author

fitzgen commented Jun 11, 2018

Woops, yeah just meant that we have to give up ownership of anything the function closes over and move it into the function.

@Pauan
Copy link
Contributor

Pauan commented Jun 11, 2018

@fitzgen Right, stdweb handles that by requiring callbacks to be 'static, which seems to work well.

@alexcrichton
Copy link
Contributor

alexcrichton commented Jul 2, 2018

For anything that needs 'static then this translates in wasm-bindgen to &Closure<FnMut(...)>, but anything that doesn't need 'static this'll translate to a usual &mut FnMut(...)

@richard-uk1
Copy link
Contributor

richard-uk1 commented Aug 18, 2018

I'm trying to get my head around this.

Let's take a specific example (window.setTimeout, and window.cancelTimeout).

Here's the relevant webidl

callback Function = any(any... arguments);
[Throws]
long setTimeout(Function handler, optional long timeout = 0, any... arguments);
void clearTimeout(optional long handle = 0);

And we want to generate the equivalent of

#[wasm_bindgen]
extern {
    fn setTimeout(handler: &Closure<FnMut()>, timeout: u32) -> f64;
    fn clearTimeout(handle: f64);
}

Note this is a start, but not finished generation, because the FnMut should be able to return anything (not just void), and take any arguments, passable as a third parameter. However, I think these restrictions are not limiting, at least in the short term.

So it looks like we don't actually generate anything for Function (apart from maybe a type alias?), but use that information to generate signatures for functions (in this case in the WindowOrWorkerGlobalScope mixin).

I'm just seeing if others agree this is the correct approach before I do any work.

EDIT: Also we have to assume that the function will be called more than once, and we don't know when the function might be called, so we have to insist it is heap-allocated (Closure<..>).

I think this feature is blocked on #503 because the current impl for variadic (assume one arg) is incompatible with this construct, we the default should probably be no args.

@fitzgen
Copy link
Member Author

fitzgen commented Aug 20, 2018

For arguments to imported JS functions, I believe we want to use Closure<FnMut(...) -> ...>.

For returned values from imported JS functions/getters, I believe we want to use js_sys::Function.

@alexcrichton
Copy link
Contributor

@fitzgen and I had a good talk about this today, and the conclusions were:

  • We should probably bind all callbacks as js_sys::Function.
  • Next, we should provide the ability, from a Closure<T>, to get the underlying JsValue (which is actually a js_sys::Function but will need a cast)
  • Additionally we should provide the ability to get a JsValue corresponding to &mut FnMut(..) (or something equivalent)

The actual method of acquiring a JsValue from Closure<T> will require mega-hacks that's not worth going into here, but I'll look into the implementation tomorrow.

@fitzgen
Copy link
Member Author

fitzgen commented Sep 5, 2018

Next, we should provide the ability, from a Closure, to get the underlying JsValue (which is actually a js_sys::Function but will need a cast)

For context, the reason why it needs to be a JsValue instead of a js_sys::Function is because js_sys is a downstream crate that depends on wasm-bindgen, and wasm-bindgen doesn't have access to that type.

Alternatively, we could move js_sys::Function into wasm_bindgen and then re-export it again in js_sys.

Let's get something working first and then think about it. Likely it won't matter since everyone will be using web_sys for callbacks which can do the casting under the covers.

@alexcrichton
Copy link
Contributor

I've added support for this in #796

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closures Adding more support for closures on the boundary frontend:webidl Issues related to the WebIDL frontend to wasm-bindgen help wanted We could use some help fixing this issue!
Projects
None yet
Development

No branches or pull requests

4 participants