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

Add #[wasm_bindgen(ignore)] to ignore a pub field and not generate getters/setters for it #1284

Closed
lrlna opened this issue Feb 21, 2019 · 9 comments · Fixed by #1410
Closed
Labels
assigned This issue has been assigned to a contributor frontend:macro Issues related to the proc-macro frontend to wasm-bindgen good first issue This is a good issue for people who have never contributed to wasm-bindgen before

Comments

@lrlna
Copy link

lrlna commented Feb 21, 2019

Hello!

I am attempting to use a HashMap inside a wrapper struct, but running into a conversion error.
Code to reproduce:

#[wasm_bindgen]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Parser {
  pub count: usize,
  pub fields: HashMap<String, Field>,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Field {
  pub name: String,
  pub count: usize,
}

#[wasm_bindgen]
impl Parser {
  #[wasm_bindgen(constructor)]
  #[wasm_bindgen(js_name="new")]
  pub fn wasm_new() -> Self {
    Parser {
      count: 0,
      fields: HashMap::new(),
    }
  }

  #[wasm_bindgen(js_name="toJson")]
  pub fn wasm_to_json(&mut self) -> JsValue {
    let field = Field {
      name: "field1".to_string(),
      count: 1
    };
    self.fields.insert("field1".to_string(), field);

    JsValue::from_serde(&self).unwrap()
  }
}

Running wasm-pack build causes this error:

  error[E0277]: the trait bound `std::collections::HashMap<std::string::String, std::string::String>: wasm_bindgen::convert::traits::IntoWasmAbi` is not satisfied
    --> src/lib.rs:11:1
     |
  11 | #[wasm_bindgen]
     | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::IntoWasmAbi` is not implemented for `std::collections::HashMap<std::string::String, std::string::String>`

  error[E0277]: the trait bound `std::collections::HashMap<std::string::String, std::string::String>: wasm_bindgen::convert::traits::FromWasmAbi` is not satisfied
    --> src/lib.rs:11:1
     |
  11 | #[wasm_bindgen]
     | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::FromWasmAbi` is not implemented for `std::collections::HashMap<std::string::String, std::string::String>`

  error: aborting due to 2 previous errors

I know the guide on Arbitrary Data Structures mentions to remove the #[wasm_bindgen] macro around the struct, but doing that also errors out.
Adjusted Parser Struct (and everything else from example stays the same):

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Parser {
  pub count: usize,
  pub fields: HashMap<String, Field>,
}

Error:

  error[E0277]: the trait bound `Parser: wasm_bindgen::convert::traits::IntoWasmAbi` is not satisfied
    --> src/lib.rs:23:1
     |
  23 | #[wasm_bindgen]
     | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::traits::IntoWasmAbi` is not implemented for `Parser`
     |
     = note: required because of the requirements on the impl of `wasm_bindgen::convert::traits::ReturnWasmAbi` for `Parser`

Thank you for your time!

@fitzgen
Copy link
Member

fitzgen commented Feb 21, 2019

Hey @lrlna -- thanks for filing an issue.

Right now, whenever there is a pub field on a #[wasm_bindgen] struct, we generate a corresponding getter and setter for the ES6 class. In your example, this results in type errors because HashMaps can't be sent between wasm and js (unless you serialize/deserialize like you are doing elsewhere in the example).

We could add a #[wasm_bindgen(ignore)] attribute or something to ignore a pub field and skip generating these getters and setters for it. Something like:

#[wasm_bindgen]
pub struct Parser {
    pub count: usize,
    #[wasm_bindgen(ignore)]
    pub fields: HashMap<String, Field>,
}

In the meantime, you can get your example code working by either

  • Making fields not pub and adding internal getter and setter methods for your crate to use.
  • Making it pub(crate) so that your crate can still use the field directly, but wasm-bindgen won't attempt to generate a getter/setter pair.

@fitzgen fitzgen added the frontend:macro Issues related to the proc-macro frontend to wasm-bindgen label Feb 21, 2019
@fitzgen fitzgen changed the title Into/FromWasmAbi not implemented for HashMap Add #[wasm_bindgen(ignore)] to ignore a pub field and not generate getters/setters for it Feb 21, 2019
@lrlna
Copy link
Author

lrlna commented Feb 25, 2019

Ohhhhh that's really interesting, but totally makes sense. I guess I was trying to avoid running into trouble with HashMaps by just serializing at the very end when I output, but should be thinking about the struct as well.

Having it not be a pub field is really not that much of a hassle in fact and that totally compiles. But, I do really like the idea of having a #[wasm_bindgen(ignore)], especially for packages that are meant to work as a wasm target and just a rust crate. E.g. if there are certain things only accessible to those using the rust version, but not the js version \o/

Thanks so much for your help, @fitzgen !

@alexcrichton alexcrichton added the good first issue This is a good issue for people who have never contributed to wasm-bindgen before label Feb 25, 2019
@tyleranton
Copy link
Contributor

So is adding #[wasm_bindgen(ignore)] to ignore a pub field and not generate getters/setters for it the proposed solution?

I can take on this issue with some guidance!

@fitzgen
Copy link
Member

fitzgen commented Feb 28, 2019

So is adding #[wasm_bindgen(ignore)] to ignore a pub field and not generate getters/setters for it the proposed solution?

Correct!

I can take on this issue with some guidance!

  • Here is where you would add the ignore attribute:

    pub struct BindgenAttrs {
    /// List of parsed attributes
    pub attrs: Vec<(Cell<bool>, BindgenAttr)>,
    }
    macro_rules! attrgen {
    ($mac:ident) => {
    $mac! {
    (catch, Catch(Span)),
    (constructor, Constructor(Span)),
    (method, Method(Span)),
    (static_method_of, StaticMethodOf(Span, Ident)),
    (js_namespace, JsNamespace(Span, Ident)),
    (module, Module(Span, String, Span)),
    (getter, Getter(Span, Option<Ident>)),
    (setter, Setter(Span, Option<Ident>)),
    (indexing_getter, IndexingGetter(Span)),
    (indexing_setter, IndexingSetter(Span)),
    (indexing_deleter, IndexingDeleter(Span)),
    (structural, Structural(Span)),
    (final_("final"), Final(Span)),
    (readonly, Readonly(Span)),
    (js_name, JsName(Span, String, Span)),
    (js_class, JsClass(Span, String, Span)),
    (extends, Extends(Span, syn::Path)),
    (vendor_prefix, VendorPrefix(Span, Ident)),
    (variadic, Variadic(Span)),
    (typescript_custom_section, TypescriptCustomSection(Span)),
    (start, Start(Span)),
    }
    };
    }

  • You would skip creating a StructField if the field has the ignore attribute here:

I think that should be enough to get you started, but don't hesitate to ask any more questiosn!

@fitzgen fitzgen added the assigned This issue has been assigned to a contributor label Feb 28, 2019
@tyleranton
Copy link
Contributor

@fitzgen

It's taking some time to figure out how everything works. I have a couple of questions.

  1. Do I just add something like (ignore, Ignore(Span)) to the macro rules for attrgen?

  2. I'm guessing I would add something like match field.attrs {} after (or before) the match field.vis {}? If so, would I check for BindgenAttrs::Ignore? Or how is that checked?

I'm having a little trouble piecing together how the bindgen attribute stuff is working, so I apologize if these questions don't make sense.

Thanks!

@alexcrichton
Copy link
Contributor

@tyleranton yeah you'll want to add (ignore, Ignore(span)), it's a bit of a wonky macro syntax but that'll add a method called ignore to BindgenAttrs where you can learn whether the ignore attribute was specified in #[wasm_bindgen]

You'll later read that just below here. You're interested in the attributes of the field though, so just before you read it out you'll want to execute:

let attrs = BindgenAttrs::find(&mut field.attrs)?;
let ignored = attrs.ignore();
attrs.check_used()?;
if ignored {
    continue;
}

@LegNeato
Copy link
Contributor

LegNeato commented Mar 7, 2019

Should this be named skip to align naming with serde? https://serde.rs/field-attrs.html#serdeskip

@ababo
Copy link

ababo commented Apr 16, 2021

Don't we have a regression here?

use wasm_bindgen::prelude::*;

use base::model;

#[wasm_bindgen]
pub struct Viewer {}

#[wasm_bindgen]
impl Viewer {
    pub fn from_model_stream(
        _stream: &web_sys::ReadableStream,
    ) -> Result<Viewer, JsValue> {
        Ok(Viewer {})
    }

    #[wasm_bindgen(skip)]
    pub fn from_model(_model: &model::Model) -> Result<Viewer, JsValue> {
        Ok(Viewer {})
    }

    pub fn start(
        &mut self,
        _canvas: &web_sys::HtmlCanvasElement,
    ) -> Result<(), JsValue> {
        Ok(())
    }
}

gives

error[E0277]: the trait bound `Model: RefFromWasmAbi` is not satisfied
 --> viewer/src/viewer.rs:8:1
  |
8 | #[wasm_bindgen]
  | ^^^^^^^^^^^^^^^ the trait `RefFromWasmAbi` is not implemented for `Model`

@TobiasJacob
Copy link

I ran into the same issue, but the answer in StackOverflow helped

https://stackoverflow.com/questions/51388721/is-it-possible-to-have-wasm-bindgen-ignore-certain-public-functions-in-an-impl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned This issue has been assigned to a contributor frontend:macro Issues related to the proc-macro frontend to wasm-bindgen good first issue This is a good issue for people who have never contributed to wasm-bindgen before
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants