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

String valued enums differ from tsc #10785

Open
fiesh opened this issue Nov 30, 2019 · 7 comments
Open

String valued enums differ from tsc #10785

fiesh opened this issue Nov 30, 2019 · 7 comments

Comments

@fiesh
Copy link

@fiesh fiesh commented Nov 30, 2019

Bug Report

I think this should have been fixed by #7160.

Input Code

declare const enum E {
        e0 = "e0",
}

function f()
{
        console.log(E.e0);
}

Expected behavior/code
This is tsc's output:

function f() {
    console.log("e0" /* e0 */);
}

This is babel's output using @babel/preset-typescript:

function f() {
  console.log(E.e0);
}
  • Babel version(s): v7.7.4
  • Node/npm version: Node 12 / npm 6.12.0
  • OS: Gentoo Linux
  • Monorepo: Just test file
  • How you are using Babel: npm run babel -- --config-file /path/to/babelrc --out-file a.js a.ts with babelrc looking like this:
{
  "presets": [
    "@babel/preset-typescript"
  ]
}
@babel-bot

This comment has been minimized.

Copy link
Collaborator

@babel-bot babel-bot commented Nov 30, 2019

Hey @fiesh! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@futagoza

This comment has been minimized.

Copy link

@futagoza futagoza commented Nov 30, 2019

From the website/docs for this plugin:

This plugin does not support const enums because those require type information to compile

It also says, as a workaround, to use babel-plugin-const-enum; or remove const which makes the enum available at runtime (which I'm presuming you don't want?).

@JLHwung

This comment has been minimized.

Copy link
Contributor

@JLHwung JLHwung commented Nov 30, 2019

For a better Dev UX, we should throw an const enums are not supported error. Here is where the error is thrown, we should make sure it will also throw for declare const enums.

if (node.declare) {
path.remove();
return;
}
if (node.const) {
throw path.buildCodeFrameError("'const' enums are not supported.");
}

If you don't know how to clone Babel, follow these steps: (you need to have make and yarn available on your machine).

  1. Write a comment there to let other possible contributors know that you are working on this bug.
  2. Fork the repo
  3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
  4. Run yarn && make bootstrap
  5. Wait
  6. Run make watch (or make build whenever you change a file)
  7. Add a test (only input.js; options.json will be automatically generated) to packages/babel-plugin-transform-typescript/test/fixtures/enum/declare-const
  8. Update the code!
  9. yarn jest babel-plugin-transform-typescript to run the tests
    • If some test outputs don't match but the new results are correct, you can delete the bad options.json files and run the tests again
  10. If it is working, run make test to run all the tests
  11. Run git push and open a PR!
@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 30, 2019

Related: #6476

@fiesh

This comment has been minimized.

Copy link
Author

@fiesh fiesh commented Nov 30, 2019

It also says, as a workaround, to use babel-plugin-const-enum; or remove const which makes the enum available at runtime (which I'm presuming you don't want?).

I used babel-plugin-const-enum but didn't see any difference, unless I used it incorrectly.

Also removing the const from the enum doesn't change anything, so I don't understand what it meant by not supporting const enum. I guess a const enum is different from a const enum in a way?!

@fiesh

This comment has been minimized.

Copy link
Author

@fiesh fiesh commented Dec 1, 2019

OK, the problem is that declare removes the generated code when const is removed.

All our enums are generated automatically in our (C++) backend. We generate a declaration file that contains them, and since it's a declaration file, of course we can only write declarations. So I guess a workaround would be to generate an additional implementation file that contains the enum implementations as non-const. Hmm not ideal but doable.

@fiesh

This comment has been minimized.

Copy link
Author

@fiesh fiesh commented Dec 1, 2019

@JLHwung I also think this should only be a warning, not an error, at least in the declare case. When one uses string enums, one can also use the actual string value "e0" instead of E.e0, and TypeScript will still type check this. Using strings as such should be legal and would work with babel I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.