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

Jump to bottom

Inline const enum in import equals declaration #46664

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Inline const enum in import equals declaration #46664

wants to merge 6 commits into from

Conversation

@jablko
Copy link
Contributor

@jablko jablko commented Nov 3, 2021 

// @filename import-equals-const-enum.ts
const enum E { M }
const x1 = E.M;
import y1 = E.M;
const x2 = y1;

Currently we elide the enum declaration but not the import equals, which is an unavoidable reference error:

Base

// @filename import-equals-const-enum.js
var x1 = 0 /* M */;
var y1 = E.M;
var x2 = y1;

Head

// @filename import-equals-const-enum.js
var x1 = 0 /* M */;
var y1 = 0 /* M */;
var x2 = y1;

Fixes #16671 (comment)
jablko
jablko reviewed Nov 3, 2021
View changes
src/compiler/factory/utilities.ts
@@ -162,7 +162,7 @@ namespace ts {
const left = createExpressionFromEntityName(factory, node.left);
// TODO(rbuckton): Does this need to be parented?
const right = setParent(setTextRange(factory.cloneNode(node.right), node.right), node.right.parent);
return setTextRange(factory.createPropertyAccessExpression(left, right), node);
return setOriginalNode(setTextRange(factory.createPropertyAccessExpression(left, right), node), node);
Copy link
Contributor Author

@jablko jablko Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transformers/ts.ts replaces import equals entity names with synthetic variable declarations + property access:

TypeScript/src/compiler/transformers/ts.ts

Line 2999 in 766e83f

const moduleReference = createExpressionFromEntityName(factory, node.moduleReference as EntityName);

I experimented with resolving the synthetic property access, but settled on saving and resolving the original entity name instead?

@jablko jablko mentioned this pull request Nov 3, 2021
Open
@sandersn sandersn added this to Not started in PR Backlog Nov 17, 2021
@sandersn sandersn requested review from rbuckton and weswigham Dec 7, 2021
@sandersn sandersn moved this from Not started to Waiting on reviewers in PR Backlog Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbuckton rbuckton

@weswigham weswigham

Assignees

@rbuckton rbuckton

Labels
None yet
Projects
PR Backlog
  
Waiting on reviewers
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Imported const enum is not inlined in generated code
3 participants
@jablko @rbuckton @typescript-bot