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

Generated JS code can end up with (reserved) keywords as identifiers #2806

Open
y21 opened this issue Feb 15, 2022 · 3 comments
Open

Generated JS code can end up with (reserved) keywords as identifiers #2806

y21 opened this issue Feb 15, 2022 · 3 comments
Labels

Comments

@y21
Copy link

y21 commented Feb 15, 2022

It seems wasm-bindgen does not check that identifiers that are valid in Rust code might not be valid in JavaScript.

Building a project containing a function like this:

use wasm_bindgen::prelude::wasm_bindgen;

#[wasm_bindgen]
pub fn test(class: u8) {}

... generates this piece of JS code:

/**
* @param {number} class
*/
export function test(class) {
    wasm.test(class);
}

which is invalid (class is a keyword and can't be used for identifiers in JS)

Steps to Reproduce

  1. wasm-pack new bug
  2. Paste the code above in src/lib.rs
  3. wasm-pack build
  4. Check pkg/bug_bg.js (filename depends on the project name)

Expected Behavior

Prefix identifier with _ so it can't clash with keywords (or mangle identifiers in some other way)

Actual Behavior

The identifier is not changed and clashes with the JavaScript "class" keyword

@y21 y21 added the bug label Feb 15, 2022
@petrstudynka
Copy link
Contributor

I am not sure whether generating idents with prefixes is the "right" approach. It breaks the Rust -> JS interface mapping contract.

I'd suggest issue compiler warnings once user uses idents that are keywords in js.

petrstudynka added a commit to petrstudynka/wasm-bindgen that referenced this issue Apr 6, 2022
petrstudynka added a commit to petrstudynka/wasm-bindgen that referenced this issue Apr 7, 2022
petrstudynka added a commit to petrstudynka/wasm-bindgen that referenced this issue Apr 16, 2022
petrstudynka added a commit to petrstudynka/wasm-bindgen that referenced this issue Jun 14, 2022
alexcrichton pushed a commit that referenced this issue Jun 17, 2022
* Mangle conflicting idents (#2806)

* Added tests (#2806)
@Nyrox
Copy link

Nyrox commented Aug 5, 2022

I guess I will tack on to this issue, but a slightly more insidious version of the same problem is that names in your rust code can also shadow the same names in the generated JS code.

I just ran into this with a function with the signature:
pub fn process_frame(mut wasm: WasmEngine, time: u64)
which then generated this piece of JS shim:

export function process_frame(wasm, time) {
    _assertClass(wasm, WasmEngine);
    if (wasm.ptr === 0) {
        throw new Error('Attempt to use a moved value');
    }
    var ptr0 = wasm.ptr;
    wasm.ptr = 0;
    uint64CvtShim[0] = time;
    const low1 = u32CvtShim[0];
    const high1 = u32CvtShim[1];
    wasm.process_frame(ptr0, low1, high1);
}

which in quite hilarious fashion breaks, because the parameter called 'wasm' shadows the fairly important import:
import * as wasm from './[REDACTED]_wasm_bg.wasm';
at the top of the file. :p

Ideally I guess all generated JS code should prefix their names with something, but some warnings for names like 'wasm' would be a good idea. 😂

@daxpedda

This comment was marked as off-topic.

@daxpedda daxpedda closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2023
@daxpedda daxpedda reopened this Jun 5, 2023
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

4 participants