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

Setter has wrong name in typescript header #1555

Closed
dtysky opened this issue May 25, 2019 · 5 comments
Closed

Setter has wrong name in typescript header #1555

dtysky opened this issue May 25, 2019 · 5 comments
Labels
bug help wanted We could use some help fixing this issue! typescript

Comments

@dtysky
Copy link

dtysky commented May 25, 2019

Describe the Bug

Setter has wrong name in typescript header

Steps to Reproduce

Very simple code:

impl Color {
  #[wasm_bindgen(getter)]
  pub fn r(&self) -> f64 {
    self.0
  }

  #[wasm_bindgen(setter)]
  pub fn set_r(&mut self, r: f64) {
    self.0 = r;
    self.4 = (if (self.0 > 1.) {255.} else if (self.0 < 0.) {0.} else {self.0 * 255.}) as u8;
  }
}

wasm_bindgen will generate right javascript code:

class Color
    /**
    * @returns {number}
    */
    get r() {
        if (this.ptr === 0) {
            throw new Error('Attempt to use a moved value');
        }
        return wasm.color_r(this.ptr);
    }
    /**
    * @param {number} r
    * @returns {void}
    */
    set r(r) {
        if (this.ptr === 0) {
            throw new Error('Attempt to use a moved value');
        }
        _assertNum(r);
        return wasm.color_set_r(this.ptr, r);
    }
}

But typescript header is wrong:

class Color {
  get r(): number;
  // wrong, shoud be `set r(r: number): void`
  set set_r(r: number): void;
}
@dtysky dtysky added the bug Something isn't working label May 25, 2019
@alexcrichton alexcrichton added bug help wanted We could use some help fixing this issue! typescript and removed bug Something isn't working labels May 28, 2019
@alexcrichton
Copy link
Contributor

cc @c410-f3r, would you perhaps be interested in taking a look at this?

@c410-f3r
Copy link
Contributor

Ooppsss... My bad.

@alexcrichton Sure! Thanks for letting me know about this issue.

@dtysky
Copy link
Author

dtysky commented Jun 7, 2019

Hi, @alexcrichton, @c410-f3r

There are a little difference between js file and ts header:

  1. If a member only has getter in js file, it shoud be readonly in ts header:
get elements() {
return []
}
readonly elements: Array;
  1. If a member has both getter and setter, it should be a normal member:
get elements() {
xxx
}

set elements(value) {
xxx
}
elements: Array;

@c410-f3r
Copy link
Contributor

c410-f3r commented Jun 7, 2019

The new implementation made by @alexcrichton recently (awesome work) has this functionality but unfortunately it is currently inverted, i.e., a setter has readonly and a getter does not. #1577 should fix (I hope) all problems related to TS accessors.

@alexcrichton
Copy link
Contributor

Thanks @c410-f3r!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted We could use some help fixing this issue! typescript
Projects
None yet
Development

No branches or pull requests

3 participants