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

add reverse mappings from value to name on enums exported from rust/wasm #2240

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

Treeki
Copy link
Contributor

@Treeki Treeki commented Jul 15, 2020

This PR adds reverse mappings to enums exported from Rust, so that the string corresponding to a particular integer value can be obtained easily. This matches what TypeScript does for enums defined directly by TypeScript code.

I’ve updated the corresponding tests to take this into account.

This goes some way towards resolving #2045, albeit the approach is still a little different - I’ve kept the Object.freeze call that wasm-bindgen performs on its enums (whereas TypeScript does not seem to return a frozen object at all). Here’s an example of how they compare:

// TypeScript output
var Shape;
(function (Shape) {
    Shape[Shape["Circle"] = 1] = "Circle";
    Shape[Shape["Triangle"] = 3] = "Triangle";
    Shape[Shape["Rectangle"] = 4] = "Rectangle";
})(Shape || (Shape = {}));

// wasm-bindgen output
module.exports.Shape = Object.freeze({ Circle:1,"1":"Circle",Triangle:3,"3":"Triangle",Rectangle:4,"4":"Rectangle", });

Nevertheless, having reverse mappings still makes it more compatible with TypeScript code.

@alexcrichton
Copy link
Contributor

Sounds reasonable to me! CI thinks this may need a rebase though?

@Treeki
Copy link
Contributor Author

Treeki commented Jul 16, 2020

I am very confused - it's just one commit based off the current tip of the master branch, GitHub claims it's mergeable, and my other PR didn't have this issue (albeit it did fail on the rustfmt phase, which I've just fixed).

This page claims it can be fixed by closing and reopening the PR, so I suppose I'll try that... https://help.appveyor.com/discussions/problems/4830-stuck-on-appveyor-was-unable-to-build-non-mergeable

@Treeki Treeki closed this Jul 16, 2020
@Treeki Treeki reopened this Jul 16, 2020
@Treeki
Copy link
Contributor Author

Treeki commented Jul 16, 2020

This has gotten further, but one of the jobs has failed with a SSL error involving fetching the crates.io index, which feels like it’s unrelated to the PR itself.

I’ve not used AppVeyor before; is there a way to fix this that doesn’t involve pushing an unnecessary new commit or doing another ugly close/re-open cycle?

@alexcrichton
Copy link
Contributor

Ah those are just spurious errors, no need to worry about them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants