Skip to content

typescriptServices.js in the official npm package should be generated with preserveConstEnums on #1104

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

Closed
Arnavion opened this issue Nov 9, 2014 · 5 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@Arnavion
Copy link
Contributor

Arnavion commented Nov 9, 2014

The tsc.js should not suppress const enums, otherwise enums like ModuleKind and SyntaxKind are not present in the generated JavaScript. As a result, compiler wrappers, etc. not written in TypeScript cannot use those enums and must resort to hard-coding their values in their source.

Code that worked before:

this._options = {
    module: ts.ModuleKind.None,
    out: "output.js",
...

must now be written like this:

this._options = {
    module: 0 /* ts.ModuleKind.None */,
    out: "output.js",
...

Only wrappers themselves written in TypeScript will not have a problem, since the compiler will do this transformation for them.

I understand that you may not want to disable this optimization for tsc.js itself. If the intent is to have a separate file to export the API (#372) instead of exporting it from tsc.js, then that file should be compiled with preserveConstEnums on.

@DanielRosenwasser DanielRosenwasser added the Suggestion An idea for TypeScript label Nov 9, 2014
@DanielRosenwasser
Copy link
Member

I disagree. We're not shipping anything called typescript-dev in npm, we're shipping typescript. If your intention is to consume portions of the compiler, I think you should clone it from our repo and do whatever you want with it. I'd rather the majority of our users get a faster compiler through npm.

@Arnavion
Copy link
Contributor Author

Arnavion commented Nov 9, 2014

I'd rather the majority of our users get a faster compiler through npm.

That's why I said in the last paragraph that you may want to do this not for tsc.js itself, but for whatever file you'll expose a compiler API (#372) from in the future.

Uptil now it was possible to append module.exports = ts; to the official package's tsc.js and get a compiler API, but let's disregard that as it's not officially supported by you but a hack. So there's nothing for you to do just yet. I just want to make sure that you're aware of the issue. If you ever do decide to provide a compiler API, then it'd be good to have the API not suppress const enums as that makes the API useless to anyone who doesn't use TS to interface with it.

(Note that by "compiler API" I mean not just TypeScriptCompiler but the services code as well.)

@DanielRosenwasser
Copy link
Member

My apologies, I missed that part. Yes, when we are ready to support an API, this sounds like a reasonable consideration.

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Dec 12, 2014
@DanielRosenwasser DanielRosenwasser self-assigned this Dec 12, 2014
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 1.4 milestone Dec 12, 2014
@DanielRosenwasser DanielRosenwasser changed the title tsc.js in the official npm package should be generated with preserveConstEnums on typescriptServices.js in the official npm package should be generated with preserveConstEnums on Dec 12, 2014
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Dec 16, 2014
@mhegazy mhegazy closed this as completed Dec 16, 2014
@Arnavion
Copy link
Contributor Author

@DanielRosenwasser @mhegazy Is it intentional this fix is only in release-1.4 and not in master too?

@DanielRosenwasser
Copy link
Member

We plan to do a cherry-pick of changes from release-1.4 into master soon.

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants