fix(1632): erase const enums after inlining#1639
Conversation
798d12b to
c5a62a9
Compare
441cca9 to
ded3fa0
Compare
jakebailey
left a comment
There was a problem hiding this comment.
So, this seems probably right but will defer to @weswigham 😄
|
@jakebailey ok, thanks |
weswigham
left a comment
There was a problem hiding this comment.
I was unsure if we wanted this behavior back and didn't just wanna make const enums always preserved, since that's what we were doing before we re-added inlining, but I suppose since vscode reported it, they'd like the runtime source elision behavior back, too. Tbh, preserveConstEnums should at least be the default nowadays, but that's a change for another PR.
We also need to elide namespace declarations containing exclusively const enums and/or types (preserveConstEnums dependent) for the same reasons you'd want the const enums elided. See the missing shouldEmitModuleDeclaration call from the original ts.ts source. ast.GetModuleInstanceState is already ported, so it shouldn't be too bad.
weswigham
left a comment
There was a problem hiding this comment.
Pretty good - some last comments on comment preservation, since we're working with NotEmittedStatements which explicitly exist to copy those.
Fixes #1632