Skip to content

Allow creating a T | null type #2251

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

Open
wyozi opened this issue Jul 24, 2020 · 6 comments
Open

Allow creating a T | null type #2251

wyozi opened this issue Jul 24, 2020 · 6 comments

Comments

@wyozi
Copy link

wyozi commented Jul 24, 2020

Motivation

This has been talked about in bunch of issues already (#1252, #2139, #1906 and more), but I couldn't find a current issue describing this specific issue: returning T | null instead of T | undefined in generated types for a Rust Option<T> type.

Proposed Solution

Either allow specifying the Typescript on a per-function basis (as described in comment to #1691) or introduce some kind of OptionNull<T> type. Even more interesting would be some kind of Union<T, JsValue::Null> type (reusable for other union type returning functions), but not sure how complicated that would get it.

Alternatives

There is the workaround of specifying a custom TS type with a custom section described at #1691, but that feels complicated for such a small thing that is sure to appear a lot in strictly typed code. I'm not sure the workaround even works when creating methods to Rust-defined JS-exported structs, in which case something even uglier (imo) is needed: #2155 (comment)

@9SMTM6
Copy link

9SMTM6 commented Jan 7, 2021

I mean, I don't at all have a say in this matter, but I don't see any value added by this. What are you gaining by this? Optional Parameters in Typescript are always defined as an T | undefined union. Why decide to handle returns differently?

@namse
Copy link
Contributor

namse commented Oct 24, 2022

Emscripten seems to check the value is undefined or null,

For example, CanvasKit has the parameter type Shader | null for setting shader https://github.com/google/skia/blob/main/modules/canvaskit/npm_build/types/index.d.ts#L2158 , and it makes error when I put undefined there.
And they did just bind that to cpp function using emscripten https://github.com/google/skia/blob/main/modules/canvaskit/canvaskit_bindings.cpp#L1668

So for me it's very useful if wasm-bindgen supports T | null type.

@9SMTM6
Copy link

9SMTM6 commented Oct 24, 2022

What kind of error? A runtime error or a typescript error? You were showing a type definition. You can get around that with just casting the return value without runtime overhead, or you could go the suggested alternate route.

If it actually needs null then yeah perhaps it has a small impact, but at the same time I'm not certain there isn't an easy way to do it yourself.

@namse
Copy link
Contributor

namse commented Oct 25, 2022

A runtime error, not typescript error. I can't cast without overhead because I use wasm-bindgen, not javascript. I call it from my rust code.

@9SMTM6
Copy link

9SMTM6 commented Oct 25, 2022

If it's a runtime error casting on the TS side would not help in any case.

I still don't have any say in that matter, but this seems like something I could much more so see as a value add.

Though perhaps it's a different issue. At the very least the alternative in OPs post won't work for you as it's essentially the equivalent of casting the type on the TS side.

@terwer
Copy link

terwer commented Nov 22, 2023

does this issue has any new progress?

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