Enum impl fn gives name conflict error

[cretz]

Based on the comment at #1496 (comment), this gives an error:

#[wasm_bindgen]
#[derive(Copy, Clone)]
pub enum ImageFormat {
    PNG,
    JPEG,
    GIF,
}

#[wasm_bindgen]
impl ImageFormat {
    #[wasm_bindgen]
    pub fn from_str(s: &str) -> Result<ImageFormat, JsValue> {
        match s {
            "PNG" => Ok(ImageFormat::PNG),
            "JPEG" => Ok(ImageFormat::JPEG),
            "GIF" => Ok(ImageFormat::GIF),
            _ => Err(JsValue::from(js_sys::Error::new(&format!("Unknown format: {}", s)))),
        }
    }
}

The error is:

error: cannot shadow already defined class `ImageFormat`

The wasm-bindgen version in my lock file is 0.2.48.

[GeorgeScrivener]

I'm running in to this as well using version 0.2.60 , is there a workaround for it?
I'm happy to help track it down but would need some pointers as I'm unfamiliar with the codebase. @alexcrichton where's the best place to start?

[alexcrichton]

The code to fix here is likely around crates/cli-support/src/js/* and this may just be a bug in generation or it may be something where we need to generate some bindings before others. I'm not entirely sure where the bug lies here myself, unfortunately.

[GeorgeScrivener]

The problem seems to be that the class gets generated twice here:

    pub fn generate(&mut self) -> Result<(), Error> {
        for (id, adapter) in crate::sorted_iter(&self.wit.adapters) {
            let instrs = match &adapter.kind {
                AdapterKind::Import { .. } => continue,
                AdapterKind::Local { instructions } => instructions,
            };
            self.generate_adapter(*id, adapter, instrs)?; // << First here as a Method
        }

        ...

        for e in self.aux.enums.iter() {
            self.generate_enum(e)?; // << Then again here
        }
        ...

Enum's are currently generated as JS objects like this:
export const EnumName = Object.freeze({ A:0,B:1,... });

What is the JS we expect to generate for methods on enums?

[alexcrichton]

I don't know what the expected JS for this is, it may be the case that JS doesn't support methods-on-enums.

[Pauan]

@alexcrichton Enums don't exist in JavaScript at all. They do exist in TypeScript, but they do not support methods at all, they are just simple C-style enums.

For example, this...

enum Foo {
    Bar,
    Qux,
    Corge,
}

...gets compiled by TypeScript into this:

(function (Foo) {
    Foo[Foo["Bar"] = 0] = "Bar";
    Foo[Foo["Qux"] = 1] = "Qux";
    Foo[Foo["Corge"] = 2] = "Corge";
})(Foo || (Foo = {}));

This is very similar to what wasm-bindgen currently generates.

TypeScript also supports "ADT-style" enums, which look like this:

type Foo
    = { type: "bar", value: number }
    | { type: "qux" }
    | { type: "corge", first: string, second: string };

These are translated into normal JS objects (e.g. { type: "corge", first: "yes", second: "no" }), and they also do not support methods at all, they are just raw data.

[alexcrichton]

I think this is perhaps an issue we can't fix then? We can likely provide a better error message but methods-on-enums may just not be supported.

[Pauan]

Yeah, I don't see any good ways to support methods on enums:

1. We could translate them into regular JS functions (not methods). That seems... surprising. And it would cause name conflicts.

2. We could translate enums into classes, but that would break conventions with TypeScript, which also seems bad. And that won't work with ADT-style enums (if we ever support those in the future).

We definitely should have a better error message though.

[Pauan]

As far as users are concerned, the solution for them is to export regular functions (not methods), like this:

#[wasm_bindgen]
#[derive(Copy, Clone)]
pub enum ImageFormat {
    PNG,
    JPEG,
    GIF,
}

#[wasm_bindgen]
pub fn from_str(s: &str) -> Result<ImageFormat, JsValue> {
    match s {
        "PNG" => Ok(ImageFormat::PNG),
        "JPEG" => Ok(ImageFormat::JPEG),
        "GIF" => Ok(ImageFormat::GIF),
        _ => Err(JsValue::from(js_sys::Error::new(&format!("Unknown format: {}", s)))),
    }
}

#[wasm_bindgen]
pub fn foo(x: ImageFormat) {
    // ...
}

Now everything works fine, both in the code generation and the TypeScript types.

[RunDevelopment]

I think we could support "static" methods using namespaces. To illustrate this, the Rust code from the first comment is equivalent to the following TS code:

export enum ImageFormat {
  PNG, JPEG, GIF
}

export namespace ImageFormat {
  export function fromStr(s: string): ImageFormat {
    if (s === "PNG") return ImageFormat.PNG;
    if (s === "JPEG") return ImageFormat.JPEG;
    if (s === "GIF") return ImageFormat.GIF;
    throw new Error("Unknown format: " + s);
  }
}

The TypeScript compiler generates the following JS and D.TS files for this:

export var ImageFormat;
(function (ImageFormat) {
    ImageFormat[ImageFormat["PNG"] = 0] = "PNG";
    ImageFormat[ImageFormat["JPEG"] = 1] = "JPEG";
    ImageFormat[ImageFormat["GIF"] = 2] = "GIF";
})(ImageFormat || (ImageFormat = {}));
(function (ImageFormat) {
    function fromStr(s) {
        if (s === "PNG") return ImageFormat.PNG;
        if (s === "JPEG") return ImageFormat.JPEG;
        if (s === "GIF") return ImageFormat.GIF;
        throw new Error("Unknown format: " + s);
    }
    ImageFormat.fromStr = fromStr;
})(ImageFormat || (ImageFormat = {}));

export declare enum ImageFormat {
    PNG = 0,
    JPEG = 1,
    GIF = 2
}
export declare namespace ImageFormat {
    function fromStr(s: string): ImageFormat;
}

So supporting enum methods that don't use self seems very possible with this structure.

---

For those curious, TS is quite smart in differentiating between the enum ImageFormat and the namespace ImageFormat. You can play around with it here and see for yourself.

[daxpedda]

But then it seems to me that neither JS or TS has proper support for something like this and we are just polyfilling it. This begs the question: why?
If this is only adding convenience and not a feature, I would prefer not to implement this.

However, the current error situation could definitely be improved, so that should be fixed.

Let me know if I misunderstood anything.

[RunDevelopment]

But then it seems to me that neither JS or TS has proper support for something like this and we are just polyfilling it.

I mean, JS doesn't have a concept of enums in the first place, but you are correct in that TS has no specific language feature/syntax for this. However, adding static members to enums using namespaces is explicitly given as an example in the documentation for how namespaces merge with other things of the same name. So this is very much an intended use case for namespaces and supported.

Note: TS namespaces predate ESM and were originally intended to scope/namespace classes/functions/etc. to compensate for the lack of modules in JS. Today, the docs even recommend that you don't use namespaces for modules in modern code. AFAIK, the primary use case namespaces today is to organize types in .d.ts and enabling things like nested classes and static properties/functions/etc. on things that don't traditionally support them (like enums and types).

[daxpedda]

I mean, JS doesn't have a concept of enums in the first place, but you are correct in that TS has no specific language feature/syntax for this.

Right, I was thinking that this concerns support for classes as well, but forgot that this is already supported because static methods are a thing in JS.

However, adding static members to enums using namespaces is explicitly given as an example in the documentation for how namespaces merge with other things of the same name. So this is very much an intended use case for namespaces and supported.

Thank you, this is quite convincing to me. I'm happy to review a PR.