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 optional fields in struct reflect in typescript #1988

Closed
clearloop opened this issue Feb 3, 2020 · 3 comments
Closed

Support optional fields in struct reflect in typescript #1988

clearloop opened this issue Feb 3, 2020 · 3 comments

Comments

@clearloop
Copy link
Contributor

clearloop commented Feb 3, 2020

Motivation

Support optional fields in struct reflect in typescript, for example:

#[wasn_bindgen]
pub struct Ping {
    pong: Option<bool>,
}

in .d.ts

export class Ping {
    pong?: bool;
}

Proposed Solution

Struct generation in wasm_bindgen separate fields into getters and setters, and the ret_ty for getter or args for setter which decide the final field type out are embedded in JsBuilder, we can't checkout if some fields' types are optional, but...optional fields generate some special docs like @returns{{{}}} and @params{{{}}} we can trace.

If it is appropriate to render fields' types with checking if its doc contains .. | undefined ?

For example, we can change this line

*ty = ret_ty.to_string();

if docs.contains("undefined") {
    *ty = "?".to_string() + ret_ty
} else {
    *ty = ret_ty.to_string()
}

and this line

ts_dst.push_str(ty);

if &ty[0..1] == "?" {
    ts_dst.push_str("?: ");
    ts_dst.push_str(&ty[1..]);
} else {
    ts_dst.push_str(": ");
    ts_dst.push_str(&ty);
}

Alternatives

I think the solution above is quite a mess, but I can't figure out a better one.

@Pauan
Copy link
Contributor

Pauan commented Feb 3, 2020

I like the idea of generating optional fields... however, I would suggest that whatever tool you are using should work with | undefined, because from JS's perspective there isn't much difference, and many TypeScript programs will use | undefined.

@clearloop
Copy link
Contributor Author

clearloop commented Feb 3, 2020

The | undefined checking just applies to setter and getter in .d.ts, if this solution can make a pr, I'd like to do it, or any other way can add optional fields definition to Typescript...really need this.

btw, @Pauan, would you mind to have a look at #1986, I just wrote a pr about #[wasm_bindgen(plain_object)] yesterday.

The words in your avatar are really awesome! “花鳥" means flowers and birds in Chinese and "旅鳥" means a bird keep travelling!

@alexcrichton
Copy link
Contributor

Seems like a reasonable feature to me to support as well! A PR would be most welcome to implement this

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

3 participants