Wasm-bindgen: Parameter `maybe_memory` should be optional

[d3lm]

Describe the Bug

I have noticed that when the compilation target is web that the maybe_parameter of the initi(input, maybe_memory) is required. The name implies that it might be missing and especially for the first call to init it's most likely missing if input is a string and the module is fetched. Only subsequent calls might have memory. Maybe we change the type of this to also accept null or undefined or mark it as optional.

I think it would make sense to have a sanity check in load that, if module is not a function nor a Response object, meaning it's a WebAssembly module, then we add a check to ensure that maybe_memory is not undefined.

Alternatively, we can pass null! for memory_memory but to me this feels hacky.

Steps to Reproduce

1. Compile Rust to wasm32-unknown-unknown with target being web

---

If there is a solution to this and a clear path, I am volunteering to work on a PR to get this resolved.

[alexcrichton]

Thanks for the report, but I believe that this parameter is already optional and not required if memory isn't imported. It should only be required if the wasm module imports memory, in which case there's nothing we can do if it's not supplied.

[Pauan]

@alexcrichton Hmmm, but this is the generated code when multithreading is enabled:

async function load(module, imports, maybe_memory) {
    if (typeof Response === 'function' && module instanceof Response) {
        memory = imports.wbg.memory = new WebAssembly.Memory({initial:17,maximum:16384,shared:true});
        if (typeof WebAssembly.instantiateStreaming === 'function') {
            try {
                return await WebAssembly.instantiateStreaming(module, imports);

            } catch (e) {
                if (module.headers.get('Content-Type') != 'application/wasm') {
                    console.warn("`WebAssembly.instantiateStreaming` failed because your server does not serve wasm with `application/wasm` MIME type. Falling back to `WebAssembly.instantiate` which is slower. Original error:\n", e);

                } else {
                    throw e;
                }
            }
        }

        const bytes = await module.arrayBuffer();
        return await WebAssembly.instantiate(bytes, imports);

    } else {
        memory = imports.wbg.memory = maybe_memory;
        const instance = await WebAssembly.instantiate(module, imports);

        if (instance instanceof WebAssembly.Instance) {
            return { instance, module };

        } else {
            return instance;
        }
    }
}

So if you pass in a Module or ArrayBuffer then it won't work properly. So it should instead be something like this:

async function load(module, imports, maybe_memory) {
    if (maybe_memory == null) {
        maybe_memory = new WebAssembly.Memory({initial:17,maximum:16384,shared:true});
    }

    memory = imports.wbg.memory = maybe_memory;

    if (typeof Response === 'function' && module instanceof Response) {
        if (typeof WebAssembly.instantiateStreaming === 'function') {
            try {
                return await WebAssembly.instantiateStreaming(module, imports);

            } catch (e) {
                if (module.headers.get('Content-Type') != 'application/wasm') {
                    console.warn("`WebAssembly.instantiateStreaming` failed because your server does not serve wasm with `application/wasm` MIME type. Falling back to `WebAssembly.instantiate` which is slower. Original error:\n", e);

                } else {
                    throw e;
                }
            }
        }

        const bytes = await module.arrayBuffer();
        return await WebAssembly.instantiate(bytes, imports);

    } else {
        const instance = await WebAssembly.instantiate(module, imports);

        if (instance instanceof WebAssembly.Instance) {
            return { instance, module };

        } else {
            return instance;
        }
    }
}

Also the .d.ts file should make maybe_memory optional, like this:

export default function init (module_or_path?: InitInput | Promise<InitInput>, maybe_memory?: WebAssembly.Memory): Promise<InitOutput>;

[alexcrichton]

Ah sorry, yes, that seems like a reasonable update to me!

[RReverser]

Can confirm, we discussed this on the original PR and I've also ran into this recently and left a comment there but haven't gotten around to submitting issue. https://github.com/rustwasm/wasm-bindgen/pull/1412#discussion_r416964251

Thanks @d3lm!

[d3lm]

Yep, I agree with @Pauan. The code he provided is what I was hoping for, to make the wasm memory optional especially for the generated type definition.