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

[tsdoc] remove "const" keyword before enum to use with typescript isolatedModules #306

Merged
merged 2 commits into from
Apr 6, 2022

Conversation

samisayegh
Copy link
Contributor

Summary

When a typescript project is using isolatedModules, referencing exported const enums is not allowed.

Details

Please refer to this PR.

@octogonz
Copy link
Collaborator

octogonz commented Apr 6, 2022

@samisayegh sorry this PR got overlooked.

I seem to recall that we have a way to enable isolatedModules without breaking consumers that DON'T use isolatedModules. Let me ask around.

@Gerrit0
Copy link
Contributor

Gerrit0 commented Apr 6, 2022

I think you are confusing isolatedModules with esModuleInterop (particularly, allowSyntheticDefaultImports, which is turned on by that option)

@octogonz
Copy link
Collaborator

octogonz commented Apr 6, 2022

Yes, I mistakenly thought this was a breaking change. But actually converting const enum to enum removes the inlining optimization entirely, which enables isolatedModules=true, without breaking isolatedModules=true source code, AND ALSO without breaking existing .js files transpiled with isolatedModules=true.

@dmichon-msft reminded me that there is a preserveConstEnums: true setting which avoids breaking .js consumers with isolatedModules=true, but the const enum still appears in the .d.ts files and thus will be a compiler error for TypeScript consumers.

I think the correct long term solution is a new TypeScript keyword as proposed in microsoft/TypeScript#37774 (comment). This seems like the only reasonable way for a .d.ts file to request for enums to be inlined while guaranteeing that it's safe not to inline them.

In any case, avoiding const enum is the simple short term solution for libraries that want to interoperate with isolatedModules=true, so we'll accept this PR.

@octogonz octogonz merged commit 318c3ff into microsoft:master Apr 6, 2022
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.

3 participants