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

Inline const enum in import equals declaration #46664

Draft
wants to merge 6 commits into
base: main
Choose a base branch
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)

@@ -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

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:

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?

@sandersn sandersn added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 18, 2022
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

This needs some dedicated tests (I don't see one testing the issue described in the OP) and a merge to resolve the conflicts.

@sandersn
Copy link
Member

Do you want to keep working on this? Otherwise I'd like to close it to reduce the number of open PRs.

@jablko jablko marked this pull request as draft April 1, 2022 14:10
@@ -38,7 +38,7 @@ var o = {
1: true
};
var a = 1 /* A */;
var a1 = 1 /* "A" */;
var a1 = 1 /* A */;
Copy link

Choose a reason for hiding this comment

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

why changed?

I think it caused by following to:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imported const enum is not inlined in generated code
6 participants