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

[Transforms] Merge master 06/06/2016 #8991

Merged
merged 224 commits into from
Jun 14, 2016
Merged

[Transforms] Merge master 06/06/2016 #8991

merged 224 commits into from
Jun 14, 2016

Conversation

yuit
Copy link
Contributor

@yuit yuit commented Jun 6, 2016

No description provided.

weswigham and others added 30 commits April 26, 2016 03:13
@@ -28,12 +28,17 @@ namespace ts {
const previousOnSubstituteNode = context.onSubstituteNode;
context.onSubstituteNode = onSubstituteNode;
context.enableSubstitution(SyntaxKind.Identifier);
context.enableSubstitution(SyntaxKind.BinaryExpression);
context.enableSubstitution(SyntaxKind.PrefixUnaryExpression);
context.enableSubstitution(SyntaxKind.PostfixUnaryExpression);
Copy link
Member

Choose a reason for hiding this comment

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

You need to enable emit notification for source files so that you know the current source file during substitution.

@yuit
Copy link
Contributor Author

yuit commented Jun 13, 2016

Current following tests are failing:

  • es5andes6module
  • es6modulekindWithES5Target1,2,6,11

const sourceFileName = sourceFile ? sourceFile.fileName : undefined;
if (bindingNameExportSpecifiers && bindingNameExportSpecifiers[sourceFileName] && hasProperty(bindingNameExportSpecifiers[sourceFileName], left.text)) {
if (bindingNameExportSpecifiersForFileMap &&
bindingNameExportSpecifiersForFileMap[currentSourceFileIdDuringSubstitution] &&
Copy link
Member

Choose a reason for hiding this comment

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

Can we store the result of this lookup in a local variable so we aren't performing the same lookup twice in a row?

@rbuckton
Copy link
Member

After you address this comment (and the related comments), 👍

@yuit yuit merged commit f235bf7 into transforms Jun 14, 2016
@yuit yuit deleted the transforms_mergemaster branch June 14, 2016 18:37
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Domain: Transforms Relates to the public transform API
Projects
None yet
Development

Successfully merging this pull request may close these issues.