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

Automatically camelify names #1818

Open
Pauan opened this issue Oct 16, 2019 · 32 comments · May be fixed by #4215
Open

Automatically camelify names #1818

Pauan opened this issue Oct 16, 2019 · 32 comments · May be fixed by #4215
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever)

Comments

@Pauan
Copy link
Contributor

Pauan commented Oct 16, 2019

Motivation

Right now you can use js_name to convert a Rust foo_bar into JS fooBar (or vice versa). But 99% of the time you want it to be camel case, so this just ends up being repetitive boilerplate:

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_name = fooBar)]
    fn foo_bar(...);

    #[wasm_bindgen(js_name = quxCorge)]
    fn qux_corge(...);

    #[wasm_bindgen(js_name = yesNo)]
    fn yes_no(...);
}

Proposed Solution

wasm-bindgen should automatically camelify JS names, so foo_bar will be translated into fooBar on import/export.

It will still be possible to use js_name to manually override that if the user does not want camel-case.

This is a breaking change, so we should batch it together with other breaking changes.

@Pauan Pauan added the enhancement New feature or request label Oct 16, 2019
@limira
Copy link
Contributor

limira commented Oct 16, 2019

Using wasm-bindgen, if I need to write some glue code in JS, I name all of them in snake case. So, this is a breaking change for me. I am very appreciate if this new feature doesn't break existing code.

@Pauan
Copy link
Contributor Author

Pauan commented Oct 16, 2019

@limira Of course your code won't break, because wasm-bindgen follows semver, so this would be done in a major version bump, at the same time as the other breaking changes.

@limira
Copy link
Contributor

limira commented Oct 16, 2019

It will break, eventually - because sooner or later, we have to move up to the latest version, right? That said, if you are unable to find a way to avoid the breaking, just break it - I understand the reason of this change.

@limira
Copy link
Contributor

limira commented Oct 16, 2019

Thinking about this. I wonder what others do? I started using wasm-bindgen in Jul 2018 and found it useful for real app. It's nearly one+half years ago. I chose to name functions in JS glue code in snake case to avoid using the js_name attribute. How many people do the same as me? (So, they never think about automatically camelify names, and therefore no issue was raised until this?)

That said, and as I said in the previous comment, I understand the reason of this change. I just want to deal with the breakage less painful. For example, importing JS code named in snake case:

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen]
    fn foo_bar(...);

    #[wasm_bindgen]
    fn qux_corge(...);

    #[wasm_bindgen]
    fn yes_no(...);
}

Must be modified to:

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_name = foo_bar)]
    fn foo_bar(...);

    #[wasm_bindgen(js_name = qux_corge)]
    fn qux_corge(...);

    #[wasm_bindgen(js_name = yes_no)]
    fn yes_no(...);
}

Doing the modification manually, it requires a lot of copy and paste. Can we do it automatically with minimal effort?

If this change will eventually happen, I guess the attribute js_name should help doing this less painful. For example, js_name = * means the JS's name is the same as in Rust. So it just requires pasting the string js_name = * to all broken imports and ... it's done.

@alexcrichton alexcrichton added breaking-change Tracking breaking changes for the next major version bump (if ever) and removed enhancement New feature or request labels Oct 17, 2019
@alexcrichton
Copy link
Contributor

I'd personally be in favor of this, it sounds like a great idea!

@limira we could always add support for something like this for that:

#[wasm_bindgen(disable_auto_camel_case)]
extern "C" {
    fn foo_bar(...);
    fn qux_corge(...);
    fn yes_no(...);
}

@alexcrichton
Copy link
Contributor

Along the same lines actually, it'd be great to get this implemented today rather than waiting for a breaking change, that way the breaking change is simply switching defaults!

It'd be great if we could add #[wasm_bindgen(auto_camel_case)] or something like that as a top-level attribute which would trigger the logic for automatically case switching.

@ibaryshnikov
Copy link
Member

can we consider a cli flag as an alternative?
also, are there more users of wasm-bindgen except browsers/node? Can standalone runtimes like wasmer use it?

@Pauan
Copy link
Contributor Author

Pauan commented Oct 19, 2019

@ibaryshnikov A CLI flag would break dependencies.

Since wasm-bindgen relies upon a JS engine, it can't really run in standalone runtimes like wasmer.

@ibaryshnikov
Copy link
Member

I'd like to add that I like the original idea. But there are only motivation and proposed solution fields in it. I'd like to see alternatives and drawbacks as well (and may be more on this).

A CLI flag would break dependencies.

@Pauan can you elaborate on this?

Since wasm-bindgen relies upon a JS engine

and here, too

About the latter, there are interface types and host bindings proposals, so, maybe there's more use for wasm-bindgen in the future?

@Pauan
Copy link
Contributor Author

Pauan commented Oct 20, 2019

@ibaryshnikov can you elaborate on this?

If you use a library (which uses wasm-bindgen), then that library will be relying on a certain case. But a CLI flag would change the casing of everything, your code and the library's code, and thus cause the library to break.

As for the JS engine part, even with interface types wasm-bindgen still needs to generate JS glue, and that will continue to be true for a long time. Plus there are things like inline_js and module.

@ibaryshnikov
Copy link
Member

@Pauan thanks for the explanations!

If you use a library (which uses wasm-bindgen), then that library will be relying on a certain case.

that's true, that makes cli option not usable (

even with interface types wasm-bindgen still needs to generate JS glue

In the Vision section of #1524 there's

We leverage WebIDL bindings and anyref to completely remove the wasm-bindgen-specific metadata and 99% of the JS bindings glue

Is there a section of goals and non goals for wasm-bindgen? I think it can make things a lot clearer.

Automatically changing the case looks useful, but not natural. If I get it right, using js_name changes not only the name of a function in JS but also the name of a function inside wasm. If only we could update this behavior to keep names inside wasm in snake_case and names in JS in camelCase, it might be more future-proof in case of new targets.

Finally, I'd like to propose a solution similar to serde: https://serde.rs/container-attrs.html#rename_all

#[wasm_bindgen(rename_all = "camelCase")]
extern "C" {
    fn foo_bar(...);

    fn qux_corge(...);

    fn yes_no(...);
}

@Pauan
Copy link
Contributor Author

Pauan commented Oct 22, 2019

If I get it right, using js_name changes not only the name of a function in JS but also the name of a function inside wasm.

That doesn't matter, since you're not importing the .wasm file, you're importing the .js file which then re-exports the wasm function (with appropriate glue code if needed).

The .wasm file contains many imports/exports which are internal implementation details, it doesn't work if you import it directly.

If only we could update this behavior to keep names inside wasm in snake_case and names in JS in camelCase, it might be more future-proof in case of new targets.

Why do you think that snake_case would be more future-proof?

In any case, wasm imports and exports are not identifiers, they are just raw UTF-8 strings, so they can be literally anything, so there is no compatibility issue (either now or in the future).

@Pauan
Copy link
Contributor Author

Pauan commented Oct 22, 2019

(Since it wasn't linked, here is a link to the PR for this: #1823 )

@ibaryshnikov
Copy link
Member

That doesn't matter, since you're not importing the .wasm file, you're importing the .js file which then re-exports the wasm function (with appropriate glue code if needed).

In es modules integration proposal we can import wasm, it already works in node.js behind a flag.

Why do you think that snake_case would be more future-proof?

Only snake_case inside wasm, js glue can be anything, as it'll be removed one day (as of #1524). Becase users will not have to rename their functions like this

#[wasm_bindgen(js_name = my_function)]
pub fn my_function() {}

Can changes from #1823 also be discussed before implementing?

@Pauan
Copy link
Contributor Author

Pauan commented Oct 22, 2019

In es modules integration proposal we can import wasm, it already works in node.js behind a flag.

That's not what I meant, I meant that the wasm generated by wasm-bindgen isn't designed to be imported directly, and it won't work right if you try and do that. You have to import the .js file.

Only snake_case inside wasm

As I said, wasm does not use identifiers, it uses UTF-8 strings, so the imports/exports can be literally anything, they are not restricted to snake_case at all, they can contain any Unicode character.

js glue can be anything, as it'll be removed one day

As I explained, the JS glue will probably never be able to be fully removed, only reduced.

Becase users will not have to rename their functions like this

That doesn't have anything to do with the name in wasm though, that's for the JS name. The wasm name can be anything (it could even be a random auto-generated string, which is what stdweb does).

Can changes from #1823 also be discussed before implementing?

I don't see what needs to be discussed, it's a simple straightforward change that has significant advantages and no drawbacks.

But if you have any issues with it, please do mention them.

@ibaryshnikov
Copy link
Member

ibaryshnikov commented Oct 22, 2019

As of snake_case I have nothing to add, I'd like to hear more people here.

But if you have any issues with it, please do mention them.

We can have

#[wasm_bindgen(rename_all = camelCase)]
#[wasm_bindgen(rename_all = snake_case)]

instead of

#[wasm_bindgen(js_case)]
#[wasm_bindgen(rust_case)] // I just made it up, but what should we use in a camel-by-default world?

or instead of

#[wasm_bindgen(auto_camel_case)]
#[wasm_bindgen(disable_auto_camel_case)]

@Pauan
Copy link
Contributor Author

Pauan commented Oct 22, 2019

@ibaryshnikov In principle I do like that idea, but I can't think of any situation where you would want anything other than camelCase, and the extra verbosity has to be considered.

There is also an inconsistency with rename_all and js_name (which should probably be changed to rename).

On the other hand, if we go with automatic casing, then I agree that rename_all = snake_case is very nice. So I'm torn.

@alexcrichton
Copy link
Contributor

I'm not personally too keen on adding all the casing options supported by Serde, and I'm not really sure if there's a use case other than "camel case everything" or "turn off the auto camel case". I like the idea of just having one attribute name but we can do that multiple ways, for example #[wasm_bindgen(auto_camel_case = false)] or something like that.

If there's a compelling use case for something other than on/off of all case switching, then I think I could be persuaded for a featureful option like serde, but if there's not a use case other than "we should fill out the dots in the matrix" then I don't think we should try to support something so powerful.

@ibaryshnikov
Copy link
Member

@alexcrichton thanks for clarifications. Would you mind sharing some thoughts about the goals / non goals of wasm bindgen? Does it target only JS runtimes, or maybe any plans to support standalone webassembly runtimes?

@alexcrichton
Copy link
Contributor

Heh those sorts of goals/non-goals are constantly evolving over time. At this time wasm-bindgen isn't really built to support standalone runtimes in any way other than wasm interface types, but even then the support isn't great. I don't think there's an answer of "what does wasm-bindgen want to target?" since it's all so hypothetical and fluid today. We'll basically continue to slot wasm-bindgen in best we see and otherwise tweak design if necessary.

(I'm not sure how this is related to an attribute to automatically name names though, these seem like orthogonal concerns if any)

@ibaryshnikov
Copy link
Member

I'm not sure how this is related to an attribute to automatically name names though, these seem like orthogonal concerns if any

@alexcrichton here's an example
https://github.com/wasmerio/wasmer/blob/7e640c0dfb76fa70d6216e7fe7908953bdb71b8e/examples/plugin.rs#L154
If the names are automatically changed to camelCase, then

instance.func::<(i32), i32>("plugin_entrypoint")

will become

instance.func::<(i32), i32>("pluginEntrypoint")

It's not much of a problem, just may look confusing at a first glance. I'm still not sure if there will be any benefits from using wasm_bindgen with such runtime instead of a plain rustc.

@alexcrichton
Copy link
Contributor

@ibaryshnikov oh I think you're talking about perhaps when we change the defaults, right? That's still not a set-in-stone question I think, and adding an opt-in rename shouldn't break anyone?

@ibaryshnikov
Copy link
Member

@alexcrichton yes, I meant defaults. Opt-in is good to go

@clord
Copy link

clord commented Sep 2, 2020

Also struct members would have to be renamed I suppose.

@tirithen
Copy link

tirithen commented Feb 9, 2022

I just experimented around a bit with wasm-bindgen, it is amazing. Being mainly a JavaScript developer, one of the first things I wanted to change was to get camel case for all the fields and methods in my project.

It would be great with some kind of auto conversion. Ideally where I would only need to declare this in a single place. Could it not be as a feature in Cargo.toml?

As long as the change is only needed in one single place per project I see no problem with having camelCase as an opt-in.

@ZhaoXiangXML
Copy link

Any news on this? I'm really interested in this feature

@jquesada2016
Copy link

Instead of a global feature, why not just annotate it at the top level of the #[wasm_bindgen] attribute? Much like serde's behavior of #[serde(rename_all = camelCase)]?

Something like

#[wasm_bindgen(rename_all = camelCase)]
pub struct Foo { /* ... */ }

@jquesada2016
Copy link

This would then not be a breaking change.

@Kixunil
Copy link

Kixunil commented Mar 10, 2023

IMO it'd be great to use the same syntax as serde even if not all renamings are implemented because it makes it easier to recognize and remember.

@ngryman
Copy link
Contributor

ngryman commented Aug 23, 2023

I also vote for the Serde-like attribute as it'd be non-breaking and intuitive.

Most Rust developers know Serde's macros and would feel at home with a similar attribute for wasm-bindgen. "Fun" fact, I actually tried to use rename_all before going to the docs, assuming it would work 😅

@terwer
Copy link

terwer commented Feb 4, 2024

any progess? I prefer serde style like this #[serde(rename_all = "camelCase")]

may be

#[wasm_bindgen(rename_all = "camelCase")]

@daxpedda
Copy link
Collaborator

daxpedda commented Feb 5, 2024

Happy to review a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever)
Projects
None yet