Skip to content

Split import into var and type to avoid breaking backword compatability of tsserverlibrary#16409

Merged
mhegazy merged 1 commit into
masterfrom
Fix16393
Jun 10, 2017
Merged

Split import into var and type to avoid breaking backword compatability of tsserverlibrary#16409
mhegazy merged 1 commit into
masterfrom
Fix16393

Conversation

@mhegazy
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy commented Jun 9, 2017

We switched CommandNames from a namespace to a const enum. we have added an import alias declaration to cover that, but since it is a const enum, the import will be elided from the .js file. in #16381 I have removed the const label, this now uncovers the fact that CommandNames do not exisit in the generated JS.

this change switches to use a type/var aliases to ensure it gets emitted in the .js file.

Fix #16393

@mhegazy mhegazy requested a review from a user June 9, 2017 18:15
@ghost
Copy link
Copy Markdown

ghost commented Jun 9, 2017

This works as a quick fix but I think this should be filed as a bug?
Given:

namespace N {
    export const enum E { A, B }
}
namespace M {
    export import E = N.E;
}

I think we should emit something inside M if --preserveConstEnums is on.

We already do that in this case:
a.ts

export const enum E { A, B }

b.ts

export { E } from "./a";

@mhegazy
Copy link
Copy Markdown
Contributor Author

mhegazy commented Jun 10, 2017

we have treated --preserveConstEnums as a debugging flag, it does not change the emitted code, it jsut does not erase the enum declaration, so that at debug time you can look at the reverse map.

we have overloaded that use in the services and the compiler. i wounder if what we need is a flag to inline regular enum values instead.

@mhegazy mhegazy merged commit a404eda into master Jun 10, 2017
@mhegazy mhegazy deleted the Fix16393 branch June 10, 2017 18:12
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants