Skip to content

[Transforms] fix8038 and 8047#8071

Merged
yuit merged 2 commits into
transformsfrom
transforms_fix8038
Apr 14, 2016
Merged

[Transforms] fix8038 and 8047#8071
yuit merged 2 commits into
transformsfrom
transforms_fix8038

Conversation

@yuit
Copy link
Copy Markdown
Contributor

@yuit yuit commented Apr 13, 2016

fix #8047 and fix #8038

@yuit yuit added the Domain: API: Transforms Relates to the public transform API label Apr 13, 2016
getSynthesizedClone(name),
/*location*/ node
);
if (name.originalKeywordKind === SyntaxKind.DefaultKeyword && languageVersion <= ScriptTarget.ES3) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

default is not necessarily the only thing that needs an element access in ES3. ES3 permits _IdentifierName_s but not ES3 _ReservedWord_s to be on the right of a property access.

http://www.ecma-international.org/publications/files/ECMA-ST-ARCH/ECMA-262,%203rd%20edition,%20December%201999.pdf

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am aware of that. However, my understanding is this case, export/import, only "default" will be affected because other reservedWords can't be used in exportClause and therefore shouldn't be able to appear in importSpecifier

Originally I do check for all other ES3 and I won't mind to do that approach. I didn't keep it because I am not sure it will be more confusing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the brhaviour in master today as well. In the future we should add an es3 transform that will write keywords correctelly as well as trailing commas removal etc.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Apr 14, 2016

👍

@yuit yuit merged commit c21ff64 into transforms Apr 14, 2016
@yuit yuit deleted the transforms_fix8038 branch April 14, 2016 16:30
@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: API: Transforms Relates to the public transform API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants