Enable exported Rust structs to specify prototypal inheritance

[eggyal]

Rendered

Fixes rustwasm/wasm-bindgen#210 and rustwasm/wasm-bindgen#1721.

This is an early draft, but I'd be really grateful for feedback at this stage—not sure whether I'm detailing it correctly, nor where exactly to go with some of the blank sections?

[alexcrichton]

Thanks for writing this up! This is definitely something I think we need to support in one form or another, and it's great to see progress on this!

One of my main questions would be how we're going to implement this all in terms of generated code. It seems to me that when you specify a superclass you typically want to, in methods, be able to access the JsValue for the superclass (although probably typed instead of bland) as well as the custom struct data fields (what you defined in Rust). That doesn't map super well to how the bindings are generated today where an exported Rust struct ends up having a JS value on the other side but Rust can't really ever access it.

Would you be able to expand the detailed design with snippets and examples of what the generated code would look like? (both on the Rust and JS side of things)

[alexcrichton]

Heh yeah these are some of the things I was thinking of! To be clear I haven't thought a huge amount about this feature, so I don't really have all the answers. A good driving use case will help design this, and I figured web components was one which would need access to the actual JS object to do things like modify the DOM, but I could be wrong.

I'm not entirely sure how we're going to draw all these connections between parent/child classes. IDs and byte concatenation is one (although I don't know how to do that) but we could possibly just use strings as well. In any case this is where I think it'd be helpful to prototype something that's workable and then we can have that help influence the design of the RFC and such.

[alexcrichton]

I think it's probably fine to just stick with something that works for now , this is a relatively minor portion of the feature set in this change :)

[torch2424]

@eggyal This was brought to my attention at the Rust/Wasm WG meeting, and just wanted to give a few comments, hope I'm not driving the current discussion away:

1. This is super rad! 😄 Stoked that you are driving this, and I am sure it will be a huge help in a lot of rust/wasm apps 👍 🎉 Also, I think the API is very clean, I write a ton of JS, and a bit of rust/wasm, and I think it looks easy to use and makes sense.

2. The only thing I would suggest is naming? I think superclass makes sense in both ES6 and Rust land. But there is still a lot of popular JS libraries that don't use ES6 classes, and depend on prototypical inheritance. From your example:

// superclass imported from JS
#[wasm_bindgen]
extern "C" {
    pub type ImportedParent;
}
#[wasm_bindgen(superclass=ImportedParent)]
pub struct ChildOfImportedParent { ... }

If ImportedParent is not a class, but let's say, a popular library like lodash a user wants to extend / override with wasm based functions. Then, superclass doesn't make as much sense? Thus, I think renaming superclass to prototype, e.g #[wasm_bindgen(prototype=ImportedParent)], makes sense. Though, I don't feel super strongly about this, as you could easily say, prototype doesn't make sense for rust developers.

Hope that helps, thanks! 😄 👍

[Pauan]

Personally, I think the attribute should be called extends, since that matches the ES6 syntax, and it's also consistent with wasm-bindgen itself.

But that's all bikeshedding stuff which should be easy to change.

[eggyal]

@torch2424 @Pauan I agree that superclass may not be the best choice, but I deliberately chose not to use extends because I didn't want to overload its semantics. I'm definitely open to it, if the community is happy. For now, I've updated the RFC to use prototype.

I've also fleshed out the "Detailed Explanation" section quite a bit more to reflect the work I've undertaken in rustwasm/wasm-bindgen#1737 (together with further changes not yet pushed). Would very much welcome critique of this design!

[eggyal]

I've moved my earlier comments (to the extent they're still relevant) into the draft RFC where they now have more context and will hopefully make more sense; this also tidies up the conversation on this PR to (hopefully) facilitate wider discussion.

[Pauan]

@alexcrichton That doesn't map super well to how the bindings are generated today where an exported Rust struct ends up having a JS value on the other side but Rust can't really ever access it.

Are you referring to the JS class or the JS object? My understanding is that we can access the JS object.

[eggyal]

@Pauan Are you referring to the JS class or the JS object? My understanding is that we can access the JS object.

I believe that @alexcrichton was referring to the class (apologies, I deleted the comment to which he was replying in trying to tidy this issue up a bit and move pertinent discussion into the RFC where it has better context). A solution to this is set out in the RFC and has been implemented in the prototype.

For the JS object, you are correct that the JsValue can already be accessed—but this would need to change a little under the current draft RFC (see section "Object instantiation").

[eggyal]

@Pauan, on reflection, it's actually a lot more difficult in an inheritance scenario.

The impl From<#name> for JsValue to which you referred wraps the Rust instance in its JS shim and obtains a JsValue representing the result: however, this "wrapping" currently uses Object.create() in order to avoid calling the shim constructor... this is acceptable for the status quo, where the shim object is a mere transparent proxy for the Rust instance—but in an inheritance scenario, proper instantiation of the shim may require super-constructor invocation (and indeed, it may no longer be acceptable for a single object to be represented by multiple shims over the course of its lifetime).

I therefore don't think that post-instantiation wrapping is straightforward for derived objects. Far better, I think, to require that shims for derived objects be instantiated upon construction and live for the duration of the object's lifetime. Of course, this does mean that the current implementation of that From trait is no longer appropriate.

I originally thought that a smart pointer holding the JsValue but that derefs in some way to the Rust object could solve this—but I'm beginning to see that it does not, since (without some sort of reverse-lookup registry) Rust instance methods would not have any means of accessing it: this is something that they will require in order to, for example, invoke methods on their prototype chain.

One idea could be to inject into exported derived objects a field that holds their prototype instance (which would be the typed JsValue of the shim for types that derive from imported types, or the actual Rust object for types that derive from exported types), which could then be returned by an implementation of Deref<Target=parent_type>::deref. While this would force construction to always fall within our control (where we can ensure the shim is properly instantiated), it would of course break pattern matching and perhaps other things too...

Evidently this is a much much much more involved idea than I had thought at the outset!

Still working on it...

[eggyal]

@alexcrichton @Pauan Following further learnings, I've completely rewritten the RFC but feel it's now extremely close to a workable solution. I have a little more to do to fully implement this, but very grateful for any & all thoughts at this stage.

[alexcrichton]

Sorry for taking awhile to get back to this @eggyal, but after reading this over I'm wondering if we can perhaps try to expose some of the lower level building blocks here? There's a lot of stylistic and library-style decisions that wasm-bindgen would have to make to implement this it looks like, but it might be cool if we could instead provide some building blocks to let iteration on various syntaxes happen in the ecosystem?

[eggyal]

That's definitely a good idea, @alexcrichton: this proposal turned out to be significantly more complex than I (naively) anticipated at first—and whilst I have got a working implementation, I made a number of choices along the way that with more considered thought would likely have been decided differently. Nevertheless, as you suggested early on, undertaking this implementation greatly improved my understanding of the issues...

I shall try to pull out some building blocks as you suggest.

[Nachasic]

Any progress on this?

[eggyal]

@Nachasic I've not looked at this for a while, but wasm-bindgen's (then?) current approach:

• instantiated shims via Object.create(), whereas parent classes that are native to JS may require that their constructors are invoked (such as is the case with custom HTML elements);

• could result in shims that are only semi-hobbled once "freed" because code that is native to JS might store state on the JS shim object itself; and

• instantiated a new shim object for the same underlying Rust struct every time it was shimmed—which may violate JS object identity assumptions.

This proposal addressed these problems by making some changes to the JS/WASM bridge; in particular, the JS shim object becomes instantiated (by calling JS constructors) upon the Rust struct's creation and is maintained until the Rust struct is dropped; furthermore the Rust struct is only accessible through an Rc in order to prevent it being dropped while still referenced from JS.

However this resulted in significant changes to the API, making it somewhat less ergonomic—in particular, you can no longer take ownership of exported types, and must instantiate them using a macro; furthermore, absent the WeakReferences TC39 proposal, shims must be manually freed or memory leaks could ensue.

Ultimately, given that this whole proposal is just a stop-gap until WASM interface-types lands and there wasn't a huge amount of interest, I wasn't convinced it was worth pursuing? I did try breaking it down in "building blocks" as @alexcrichton suggested, but I struggle to view this as much more than the one "big" change to shim construction (with everything else being housekeeping required to make that work).