Skip to content

Option<JsValue> and interoperate with js in a more js natural way #1906

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
trivigy opened this issue Dec 9, 2019 · 11 comments
Closed

Option<JsValue> and interoperate with js in a more js natural way #1906

trivigy opened this issue Dec 9, 2019 · 11 comments

Comments

@trivigy
Copy link

trivigy commented Dec 9, 2019

Motivation

I have been trying to expose a rust method to javascript which accepts an optional arbitrary javascript object. Unfortunately so far most of my attempts to do it properly, and I put an emphasis on properly, are unsuccessful. The main issue I am running into can be described as the following error message.

  --> src/components/button.rs:67:1
   |
67 | #[wasm_bindgen]
   | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::OptionFromWasmAbi` is not implemented for `wasm_bindgen::JsValue`
   |
   = note: required because of the requirements on the impl of `wasm_bindgen::convert::traits::FromWasmAbi` for `std::option::Option<wasm_bindgen::JsValue>`

I will address that I think "properly" is in the context of this entire issue.

From reading the other conversations across open and closed issues and documentations I understand that the case is that this feature is simply not implemented. I could not however understand whether this is because it is too complicated or not something that is sought after. I am including the following links for the record and minor explanations of what I had learned and where.

https://rustwasm.github.io/docs/wasm-bindgen/reference/types/jsvalue.html
Here one can see the table above that shows the state of what and where this type works and Option<T> says NO.

#14 (comment)
#14 (comment)
#14 (comment)
Here are additional mentions of this. This conversation was very interesting and seems to indicate that ultimately we want to have something like that in the library but at a much more complex capacity. Allowing for Option<CustomType>.

Now this seems to be as a great goal to have but I find it hard to reconcile with how typical javascript/typescript workflow is done. For example, following these instructions it is possible to expose a custom rust type to js HERE, although very limited in functionality. In particular the only public types that are currently supported are numbers. This means that something along the lines of the following is not possible.

#[wasm_bindgen]
pub struct ExportedNamedStruct {
    pub inner: String,
}

There are workaround that I have seen being suggested that recommend utilizing setters/getters, however I see the workaround approach as very antipattern to a typical js/ts code. Let me explain better in a form of an example.

Example

Lets suppose I expose a structure like this and a method that takes such structure:

#[wasm_bindgen]
pub struct Baz {
    field: String,
}

#[wasm_bindgen]
impl Baz {
    #[wasm_bindgen(getter)]
    pub fn field(&self) -> String {
        self.field
    }

    #[wasm_bindgen(setter)]
    pub fn set_field(&mut self, field: String) {
        self.field = field;
    }
}

#[wasm_bindgen]
pub fn mount(obj: &Baz) {
    // code
}

This type of pattern will effectively force the work on the js side to be as follows:

const obj = new Baz();
obj.field = "some string";
module.mount(obj);

However more often than not what I see to be a more natural javascript code looks as following:

module.mount({field: "some string"});

Ultimately what I am trying to reconcile is how could it be possible to actually allow for the more natural typeless work on the js side (whether or not it is better to have a type system is not at the core of my debate) while allowing for a more natural typed work on the rust side.

Additional Context

On a separate note, however closely related to my discussion of how things should work, there is a conversation related to how typescript files are generated. I found this very important because when typescript type files are generated, they ultimately act as typechecking system for the exported rust types. Currently when exporting something like Option<js_sys::Object> the generated type ends up as someFunc(name?: any) which is good and sort of achieves the Option<JsValue> typescript generation on the js/ts side. But obviously this is not Option<JsValue> and since the js_sys objects are generated automatically, I am not entirely sure how well it supports memory copying between js/rust models.

Furthermore, I have read some interesting proposals and debates geared towards allowing wasm_bindgen to better control the generated ts code. In particular #1691 is really touching directly to the point I want to make, even though the semantics of this proposed solution is debate-able.

Typescript can be very useful at leveraging typechecking on the js side. Much closer to how developing with js/ts feels. For example I could potentially want to generate the following typescript to control the js function signature rather than creating and exposing custom objects.

static mount(obj: {field: string}): void;

while the code on the rust side could remain as the following without exposing too much implementation details.

#[wasm_bindgen]
pub fn mount(obj: Option<JsValue>) {
    // code
}

Proposed Solution

So my proposed solution is simple. Lets make Option working and lets allow for tighter control over the generated typescript file and call it a day.

Current workaround

Currently my work around is, as explained above. Is to use Option<js_sys::Object> in rust functions to force generation of the someFunc(obj?: any) typescript signature. Then when such function gets called from the js side, I convert the actual passed object as follows:

pub fn mount(selector: &str, props: Option<js_sys::Object>) {
        let props = JsValue::from(props);
        if props.is_object() {
            info!("props.is_object() == true");
        } else {
            info!("props.is_object() == false");
        }
}

The reason I convert to JsValue and do the is_object() check is because I noticed that this doesn't actually ensures that what is passed into the object from the js side is actually an object. Therefore I end up checking it to make sure.

@trivigy
Copy link
Author

trivigy commented Dec 9, 2019

I would greatly appreciate any insights and implementation details to further my understanding. I would love to give this issue a stab and try to implement some parts of it. I am fairly new to rust so please be gentle.

@alexcrichton
Copy link
Contributor

We don't implement this for Option<JsValue> because it's not clear what should map to None but you should be able to use a bare JsValue and you can manually test for undefined if that's the desired semantics you'd like to have.

Would that work for you?

@trivigy
Copy link
Author

trivigy commented Dec 11, 2019

That is absolutely fine when it comes to the rust side. I don't even mind particularly about my workaround. The issue happens on the typescript generation side. Because without Option, the generated signature is non-optional function someFunc(val: any) while with Option the generated signature is optional function someFunc(val?: any). Notice the ?. This in itself is very significant because it forces the typescript library user to have to put a value. someFunc({}) vs someFunc(). That is why this issue was mentioning two connected but separate issues.

As for the mapping to of undefined or null to None question. In my opinion is should absolutely be both. null is always an explicit indication by the user that this value is omitted. e.g. someFunc(null). However undefined is an implicit indication that the value is omitted by the user. Either due to lack of knowledge of the library or due to desire to keep the calling syntax clean. e.g. someFunc() where the actual someFunc could have received a value. This explicit vs implicit choice does not exist in typed languages and therefore does not translate well. But they should be treated identical when it comes to parameter variables. When dealing with return values, should definitely be left to the developer to properly return and document the return types and values.

@alexcrichton
Copy link
Contributor

Ah I see, thanks for explaining!

I think this would probably be best solved with some sort of mechanism to customize the typescript output of wasm-bindgen. I agree with the usage patterns you've mentioned, but unfortunately I think we still won't add support for Option<JsValue>.

I think perhaps #1197 is related to this?

@alexcrichton
Copy link
Contributor

Also similar to #1691

@trivigy
Copy link
Author

trivigy commented Dec 11, 2019

I think for my specific use case as long as the typescript customization is better implemented this doesn't even matter. I'll do some digging to try and figure out how complicated it is to add the kind of typescript customization people have been mentioning. Fairly new to rust but I catch up faster than most. 😅

Any pointers and hints are much appreciated.

@Pauan
Copy link
Contributor

Pauan commented Dec 12, 2019

@trivigy The handling of undefined/null is extremely non-trivial. Consider this perfectly valid ES6 function:

function foo(a = 5) {
    return a;
}

foo()           // returns 5
foo(undefined)  // returns 5
foo(null)       // returns null

In this case there is a distinction between undefined and null for optional arguments. The same is true for many host functions which distinguish between undefined and null.

So if we were trying to match ES6 semantics, then we should accept undefined but not null. This also matches what TypeScript does, since optional parameters in TypeScript accept undefined but not null.

On the other hand, there's plenty of situations where undefined and null are not distinguished and are treated as the same. It's extremely chaotic and there isn't any consistency. So it's not an easy thing to decide.

(I agree that we should have better TypeScript customization/interop, I'm only speaking specifically about the handling of Option)

@trivigy
Copy link
Author

trivigy commented Dec 12, 2019

@Pauan Appreciate the comment. Though I completely disagree with the premise of the argument. Unless I am missing something, Rust does not have default values for method parameters. Therefore this example doesn't hold at all. To add a counter to the example. In the specific case of the discussion here, only function parameters typing is at question. Thus the discussion is purely about situation when it is rust code that was exposed to js. In those cases, the developer that is exposing the call is also responsible for handling what happens when the interface that he/she exposed is invoked. In such case I would argue the largest compatibility is best in which case why would ES6 syntax even be in consideration?!

However, backward compatibility might have issues. If someone already implemented an Option and relies on the fact Ok(T) was filled with either undefined or null appropriately, they will have to make the appropriate adjustments. But backward compatibility is an issue within itself in almost any case of library changes.

In either case, to @alexcrichton point, I think this issue is best resolved with significantly better typescript typings and NO change on the rust side. It just sounds like a simpler solution that delivers same if not more value.

@Pauan
Copy link
Contributor

Pauan commented Dec 12, 2019

@trivigy The question is what should we do with Option in function arguments?

In other words, if you were to create a Rust function like this...

#[wasm_bindgen]
pub fn foo(x: Option<u32>) {
    ...
}

...then wasm-bindgen would generate a corresponding TypeScript function like this:

export function foo(x?: number) {
    ...
}

In that case, if somebody called foo() or foo(undefined) then Rust would receive None, but if somebody called foo(null) then it would panic, because null is not a u32.

If we instead accepted null, then that would mean wasm-bindgen would have to generate a TypeScript function like this...

export function foo(x?: number | null) {
    ...
}

...but that differs from how optional arguments work in JS and TypeScript, which can cause confusion for users.

So to avoid confusing users, it's beneficial to match the behavior of JS/TypeScript, which means rejecting null. But rejecting null isn't great, we'd really like to be as permissive as possible (for the reasons you mentioned).

So, since it's tricky to decide on which behavior is correct, we have chosen to punt on the issue and just not make a decision (which is why you can't use Option in function arguments).

@alexcrichton
Copy link
Contributor

I'm gonna go ahead and close this in favor of #1691 since I think it's the route we'll likely want to pursue for this.

@terwer
Copy link

terwer commented Nov 22, 2023

+1, Option is very usefull for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants