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 for named and optional parameters in .d.ts #1129

Closed
dtysky opened this issue Dec 24, 2018 · 5 comments · Fixed by #1483
Closed

Support for named and optional parameters in .d.ts #1129

dtysky opened this issue Dec 24, 2018 · 5 comments · Fixed by #1483

Comments

@dtysky
Copy link

dtysky commented Dec 24, 2018

This issue is related to #982 which is fixed, but that only makes the return type will be reflected, if we use optional parameters, it still does not work. Another issue is that definitions of method will not keep their original name and use arg0... to instead, it may lose the semantic of parameters.

#[wasm_bindgen]
pub struct Color(f64, f64, f64, f64);

#[wasm_bindgen]
impl Color {
  #[wasm_bindgen(constructor)]
  pub fn new(r: Option<f64>, g: Option<f64>, b: Option<f64>, a: Option<f64>) -> Color {
    let mut color = Color(0.0, 0.0, 0.0, 0.0);

    match r {
      Some(value) => color.0 = value,
      None => ()
    }

    match g {
      Some(value) => color.1 = value,
      None => ()
    }

    match b {
      Some(value) => color.2 = value,
      None => ()
    }

    match a {
      Some(value) => color.3 = value,
      None => ()
    }

    color
  }
}

will generate dts:

export class Color {
  free(): void;
  constructor(arg0: number, arg1: number, arg2: number, arg3: number);
}

which I expect:

export class Color {
  free(): void;
  // use optional and keep semantic
  constructor(r?: number, g?: number, b?: number, a?: number);
}

Could we fix those issues for better programming experience?
Thanks.

@alexcrichton
Copy link
Contributor

Sounds like a good idea to me! I know the typescript generation with Option has been buggy in the past, and doing our best to preserve the argument names sounds like a great idea to me as well

@c410-f3r
Copy link
Contributor

c410-f3r commented Mar 4, 2019

If you guys don't mind, I will work on this. Just need a little help to figure out the best approach as everything related to optional parameters seems to be scattered across the repository.

@alexcrichton
Copy link
Contributor

Sure yeah!

Optional arguments are all handled roughly around here for TypeScript generation. Currently mostly generate | undefined rather than foo?, but doing either seems reasonable to me (not sure if there's a difference on the TypeScript side though).

For threading through the actual names of the arguments is a bit trickier. That would start around here to modify the AST that the CLI and macro both use, and then propagating the change outwards from there to fix issues, eventually requiring you to parse the Rust code and learn about the argument names as it goes along.

@c410-f3r
Copy link
Contributor

Optional arguments are supported since #1201. Can this issue be closed?

@alexcrichton
Copy link
Contributor

I believe so, thanks @c410-f3r!

There may be some follow-up items here and there, but it may be best at this point to have follow-up issues for those.

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

Successfully merging a pull request may close this issue.

3 participants