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

Disallow async constructors #3976

Open
benthecarman opened this issue Jun 5, 2024 · 4 comments
Open

Disallow async constructors #3976

benthecarman opened this issue Jun 5, 2024 · 4 comments
Labels

Comments

@benthecarman
Copy link

benthecarman commented Jun 5, 2024

Describe the Bug

wasm_bindgen allows you to create an async constructor even though this is not supported by typescript.

Steps to Reproduce

#[wasm_bindgen]
pub struct MyStruct {
	data: u32
}

#[wasm_bindgen]
impl MyStruct {
  #[wasm_bindgen(constructor)]
  pub async fn new(data: u32) -> Self {
     // do something async
     Self { data }
  }
}

This compiles and actually works, however seems like undefined behavior and should just fail to compile

Expected Behavior

Unknown, probably fail to compile

Actual Behavior

Works

@Liamolucko
Copy link
Collaborator

We considered removing support for this in #3562, since it was originally a bug, but discovered that people had started relying on it and so left it to avoid breakage.

You can see lots of instances of people doing this with https://github.com/search?q=%2F%23%5C%5Bwasm_bindgen%5C%28constructor%5C%29%5C%5D%5B+%5Cn%5D*pub+async+fn%2F&type=code.

@benthecarman
Copy link
Author

Do you know what actually ends up happening under the hood, is it safe?

@Liamolucko
Copy link
Collaborator

Do you know what actually ends up happening under the hood, is it safe?

Yes; wasm-bindgen treats constructors nearly the same way as regular associated functions, and so it generates code based on the return type of the constructor (which for async functions is a JsValue after desugaring) rather than the type it's attached to.

This is what the generated JS code ends up looking like:

export class MyStruct {
    // ...
    constructor(data) {
        const ret = wasm.mystruct_new(data);
        return takeObject(ret);
    }
}

JS actually supports returning from constructors, and if the returned value is something other than this then new MyStruct() just returns that value instead of actually making a MyStruct.

@daxpedda
Copy link
Collaborator

Maybe we should investigate if we can emit a warning in this case (with the help of deprecated).

@daxpedda daxpedda reopened this Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants