Skip to content

Fix #8738: Handles Re-assignment of Exported Clause Member#8739

Merged
vladima merged 9 commits into
microsoft:masterfrom
evansb:fix-8738
Jun 7, 2016
Merged

Fix #8738: Handles Re-assignment of Exported Clause Member#8739
vladima merged 9 commits into
microsoft:masterfrom
evansb:fix-8738

Conversation

@evansb
Copy link
Copy Markdown
Contributor

@evansb evansb commented May 22, 2016

This Pull Request Fixes #8738.

When exporting a clause directly, TypeScript emits the member assignment to exports after the variable declaration of the member. This ignores all assignment statement afterwards.

Patch Summary:
When there is assignment operator expression =, +=, etc, check if the target is a member of export clause. If it is, make sure that the export clause is updated.

Behaviour before patch:

var foo = 2;
foo = 3;
export { foo }

compiles to

var foo = 2;
exports.foo = 2;
foo = 3;

Compile result after patch:

var foo = 2;
exports.foo = 2;
exports.foo = foo = 3;

Export alias are updated as well, I use the same code for variable declaration.

@msftclas
Copy link
Copy Markdown

Hi @evansb, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@evansb evansb changed the title Fix #8738: Handles assignment to exported member Fix #8738: Handles Re-assignment of Exported Clause Member May 22, 2016
Comment thread src/compiler/emitter.ts Outdated
if (internalExportClauseMemberChanged) {
for (const specifier of exportSpecifiers[(<Identifier>node.left).text]) {
emitStart(specifier.name);
emitContainingModuleName(specifier);
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 part is duplicated. we also need to handle default for ES3 to be emitted as ["default"] instead of .default.

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 don't quite get this part, why does ES3 exports["default"] has anything to do since we're changing exports members?

@evansb
Copy link
Copy Markdown
Contributor Author

evansb commented May 23, 2016

The latest commit handles postfix and prefix ++ and -- operator.
For postfix it will be compiled to foo += 1 before prepended with alias exports.foo = foo += 1

Comment thread src/compiler/emitter.ts Outdated
for (const specifier of exportSpecifiers[name.text]) {
emitStart(specifier.name);
emitContainingModuleName(specifier);
write(".");
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.

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.

Oh right, of course the identifier itself can be "default", didn't think of that. Thanks

Comment thread src/compiler/emitter.ts Outdated
return isSourceFileLevelDeclarationInSystemJsModule(targetDeclaration, /*isExported*/ true);
}

function isNameOfExportedSourceLevelDeclarationInClauseModule(node: Node): boolean {
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.

can you rename it to isNameOfExportedDeclarationInNonES6Module?

@vladima vladima merged commit 8b7fb8e into microsoft:master Jun 7, 2016
@vladima
Copy link
Copy Markdown
Contributor

vladima commented Jun 7, 2016

thanks @evansb!

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.

4 participants