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

Remove class wrap for constructors in Rust exports #3562

Merged
merged 6 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
bindgen placeholder.
[#3233](https://github.com/rustwasm/wasm-bindgen/pull/3233)

* Changed constructor implementation in generated JS bindings, it is now
possible to override methods from generated JS classes using inheritance.
When exported constructors return `Self`.
[#3562](https://github.com/rustwasm/wasm-bindgen/pull/3562)

### Fixed

* Fixed bindings and comments for `Atomics.wait`.
Expand All @@ -66,6 +71,9 @@
* Use fully qualified paths in the `wasm_bindgen_test` macro.
[#3549](https://github.com/rustwasm/wasm-bindgen/pull/3549)

* Fixed bug allowing JS primitives to be returned from exported constructors.
[#3562](https://github.com/rustwasm/wasm-bindgen/pull/3562)

## [0.2.87](https://github.com/rustwasm/wasm-bindgen/compare/0.2.86...0.2.87)

Released 2023-06-12.
Expand Down
31 changes: 25 additions & 6 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,12 @@ impl<'a, 'b> Builder<'a, 'b> {
// more JIT-friendly. The generated code should be equivalent to the
// wasm interface types stack machine, however.
for instr in instructions {
instruction(&mut js, &instr.instr, &mut self.log_error)?;
instruction(
&mut js,
&instr.instr,
&mut self.log_error,
&self.constructor,
)?;
}

assert_eq!(js.stack.len(), adapter.results.len());
Expand Down Expand Up @@ -537,7 +542,12 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
}
}

fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) -> Result<(), Error> {
fn instruction(
js: &mut JsBuilder,
instr: &Instruction,
log_error: &mut bool,
constructor: &Option<String>,
) -> Result<(), Error> {
match instr {
Instruction::ArgGet(n) => {
let arg = js.arg(*n).to_string();
Expand Down Expand Up @@ -987,18 +997,27 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
}

Instruction::RustFromI32 { class } => {
js.cx.require_class_wrap(class);
let val = js.pop();
js.push(format!("{}.__wrap({})", class, val));
match constructor {
Some(name) if name == class => {
js.prelude(&format!("this.__wbg_ptr = {} >>> 0;", val));
js.push(String::from("this"));
}
Some(_) | None => {
js.cx.require_class_wrap(class);
js.push(format!("{}.__wrap({})", class, val));
}
}
}

Instruction::OptionRustFromI32 { class } => {
js.cx.require_class_wrap(class);
assert!(constructor.is_none());
let val = js.pop();
js.cx.require_class_wrap(class);
js.push(format!(
"{0} === 0 ? undefined : {1}.__wrap({0})",
val, class,
))
));
}

Instruction::CachedStringLoad {
Expand Down
35 changes: 34 additions & 1 deletion crates/cli-support/src/wit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,10 @@ impl<'a> Context<'a> {
Some(class) => {
let class = class.to_string();
match export.method_kind {
decode::MethodKind::Constructor => AuxExportKind::Constructor(class),
decode::MethodKind::Constructor => {
verify_constructor_return(&class, &descriptor.ret)?;
AuxExportKind::Constructor(class)
}
decode::MethodKind::Operation(op) => {
if !op.is_static {
// Make the first argument be the index of the receiver.
Expand Down Expand Up @@ -1424,6 +1427,36 @@ impl<'a> Context<'a> {
}
}

/// Verifies exported constructor return value is not a JS primitive type
fn verify_constructor_return(class: &str, ret: &Descriptor) -> Result<(), Error> {
match ret {
Descriptor::I8
| Descriptor::U8
| Descriptor::ClampedU8
| Descriptor::I16
| Descriptor::U16
| Descriptor::I32
| Descriptor::U32
| Descriptor::F32
| Descriptor::F64
| Descriptor::I64
| Descriptor::U64
| Descriptor::Boolean
| Descriptor::Char
| Descriptor::CachedString
| Descriptor::String
| Descriptor::Option(_)
| Descriptor::Enum { .. }
| Descriptor::Unit => {
bail!("The constructor for class `{}` tries to return a JS primitive type, which would cause the return value to be ignored. Use a builder instead (remove the `constructor` attribute).", class);
}
Descriptor::Result(ref d) | Descriptor::Ref(ref d) | Descriptor::RefMut(ref d) => {
verify_constructor_return(class, d)
}
_ => Ok(()),
}
}

/// Extract all of the `Program`s encoded in our custom section.
///
/// `program_storage` is used to squirrel away the raw bytes of the custom
Expand Down
11 changes: 11 additions & 0 deletions crates/cli/tests/reference/builder.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* tslint:disable */
/* eslint-disable */
/**
*/
export class ClassBuilder {
free(): void;
/**
* @returns {ClassBuilder}
*/
static builder(): ClassBuilder;
}
61 changes: 61 additions & 0 deletions crates/cli/tests/reference/builder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
let wasm;
export function __wbg_set_wasm(val) {
wasm = val;
}


const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder;

let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true });

cachedTextDecoder.decode();

let cachedUint8Memory0 = null;

function getUint8Memory0() {
if (cachedUint8Memory0 === null || cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
}
return cachedUint8Memory0;
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}
/**
*/
export class ClassBuilder {

static __wrap(ptr) {
ptr = ptr >>> 0;
const obj = Object.create(ClassBuilder.prototype);
obj.__wbg_ptr = ptr;

return obj;
}

__destroy_into_raw() {
const ptr = this.__wbg_ptr;
this.__wbg_ptr = 0;

return ptr;
}

free() {
const ptr = this.__destroy_into_raw();
wasm.__wbg_classbuilder_free(ptr);
}
/**
* @returns {ClassBuilder}
*/
static builder() {
const ret = wasm.classbuilder_builder();
return ClassBuilder.__wrap(ret);
}
}

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
};

11 changes: 11 additions & 0 deletions crates/cli/tests/reference/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct ClassBuilder(());

#[wasm_bindgen]
impl ClassBuilder {
pub fn builder() -> ClassBuilder {
ClassBuilder(())
}
}
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/builder.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(module
(type (;0;) (func (result i32)))
(type (;1;) (func (param i32)))
(func $__wbg_classbuilder_free (;0;) (type 1) (param i32))
(func $classbuilder_builder (;1;) (type 0) (result i32))
(memory (;0;) 17)
(export "memory" (memory 0))
(export "__wbg_classbuilder_free" (func $__wbg_classbuilder_free))
(export "classbuilder_builder" (func $classbuilder_builder))
)
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/constructor.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/* tslint:disable */
/* eslint-disable */
/**
*/
export class ClassConstructor {
free(): void;
/**
*/
constructor();
}
53 changes: 53 additions & 0 deletions crates/cli/tests/reference/constructor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
let wasm;
export function __wbg_set_wasm(val) {
wasm = val;
}


const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder;

let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true });

cachedTextDecoder.decode();

let cachedUint8Memory0 = null;

function getUint8Memory0() {
if (cachedUint8Memory0 === null || cachedUint8Memory0.byteLength === 0) {
cachedUint8Memory0 = new Uint8Array(wasm.memory.buffer);
}
return cachedUint8Memory0;
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}
/**
*/
export class ClassConstructor {

__destroy_into_raw() {
const ptr = this.__wbg_ptr;
this.__wbg_ptr = 0;

return ptr;
}

free() {
const ptr = this.__destroy_into_raw();
wasm.__wbg_classconstructor_free(ptr);
}
/**
*/
constructor() {
const ret = wasm.classconstructor_new();
this.__wbg_ptr = ret >>> 0;
return this;
}
}

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
};

13 changes: 13 additions & 0 deletions crates/cli/tests/reference/constructor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct ClassConstructor(());

#[wasm_bindgen]
impl ClassConstructor {

#[wasm_bindgen(constructor)]
pub fn new() -> ClassConstructor {
ClassConstructor(())
}
}
10 changes: 10 additions & 0 deletions crates/cli/tests/reference/constructor.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(module
(type (;0;) (func (result i32)))
(type (;1;) (func (param i32)))
(func $__wbg_classconstructor_free (;0;) (type 1) (param i32))
(func $classconstructor_new (;1;) (type 0) (result i32))
(memory (;0;) 17)
(export "memory" (memory 0))
(export "__wbg_classconstructor_free" (func $__wbg_classconstructor_free))
(export "classconstructor_new" (func $classconstructor_new))
)
24 changes: 24 additions & 0 deletions crates/cli/tests/wasm-bindgen/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,27 @@ fn function_table_preserved() {
.wasm_bindgen("");
cmd.assert().success();
}

#[test]
fn constructor_cannot_return_option_struct() {
let (mut cmd, _out_dir) = Project::new("constructor_cannot_return_option_struct")
.file(
"src/lib.rs",
r#"
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct Foo(());

#[wasm_bindgen]
impl Foo {
#[wasm_bindgen(constructor)]
pub fn new() -> Option<Foo> {
Some(Foo(()))
}
}
"#,
)
.wasm_bindgen("--target web");
cmd.assert().failure();
}
38 changes: 38 additions & 0 deletions guide/src/reference/attributes/on-rust-exports/constructor.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,41 @@ import { Foo } from './my_module';
const f = new Foo();
console.log(f.get_contents());
```

## Caveats

Starting from v0.2.48 there is a bug in `wasm-bindgen` which breaks inheritance of exported Rust structs from JavaScript side (see [#3213](https://github.com/rustwasm/wasm-bindgen/issues/3213)). If you want to inherit from a Rust struct such as:

```rust
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub struct Parent {
msg: String,
}

#[wasm_bindgen]
impl Parent {
#[wasm_bindgen(constructor)]
fn new() -> Self {
Parent {
msg: String::from("Hello from Parent!"),
}
}
}
```

You will need to reset the prototype of `this` back to the `Child` class prototype after calling the `Parent`'s constructor via `super`.

```js
import { Parent } from './my_module';

class Child extends Parent {
constructor() {
super();
Object.setPrototypeOf(this, Child.prototype);
}
}
```

This is no longer required as of v0.2.88.