Skip to content

Fix error when exporting const enums (#33060) #34721

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

Merged
merged 3 commits into from
Oct 25, 2019

Conversation

EvanCahill
Copy link
Member

Const enums should be allowed to be exported using an export declaration. The fix is to remove an incorrect check that restricts exporting const enums in export declarations to work only when --preserveConstEnums is true.

Fixes #33060

@weswigham
Copy link
Member

weswigham commented Oct 24, 2019

Actually, you do need preserveConstEnums on. If you don't, the enum declaration will be erased, but the export declaration will still emit something along the lines of

exports.MyConstEnum = MyConstEnum;

which is going to throw at runtime, as MyConstEnum won't exist. (?)

@EvanCahill
Copy link
Member Author

I don't see that as the behavior currently. It is correctly eliding the const enum export.

@weswigham
Copy link
Member

Oh, hum. I guess if we're already eliding the export then it's not so bad.

@EvanCahill
Copy link
Member Author

I'm happy to add another test case to explicitly test that const enums are not emitted on re-export without preserveConstEnums.

Also I'd really love this to go into 3.7 if everything looks good since this behavior is a regression from 3.5.

@weswigham
Copy link
Member

A new test'd be great, yeah.

@EvanCahill
Copy link
Member Author

Done.

@weswigham weswigham merged commit 88f3593 into microsoft:master Oct 25, 2019
@weswigham
Copy link
Member

cc @RyanCavanaugh do we wanna do anything with this for 3.7?

@DanielRosenwasser
Copy link
Member

@typescript-bot cherry-pick this to release-3.7 and LKG

@typescript-bot
Copy link
Collaborator

Heya @DanielRosenwasser, I couldn't find the branch 'release-3.7' on Microsoft/TypeScript. You may need to make it and try again.

@DanielRosenwasser
Copy link
Member

Ugh, come on, ignore the backticks you jerk robot.

@typescript-bot cherry-pick this to release-3.7 and LKG

@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, I've opened #34811 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Oct 29, 2019
Component commits:
c8bdddf Allow export declaration to reference const enums

857e7c4 Update baselines

6e0da2b Add test to verify reexported const enums are elided
DanielRosenwasser pushed a commit that referenced this pull request Oct 30, 2019
* Cherry-pick PR #34721 into release-3.7

Component commits:
c8bdddf Allow export declaration to reference const enums

857e7c4 Update baselines

6e0da2b Add test to verify reexported const enums are elided

* Update LKG
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.

Cannot export const enum in Export Declaration
4 participants