Skip to content

Fix commonjs export= merging (#27368)#27371

Merged
sandersn merged 1 commit intomasterfrom
fix-commonjs-exportequals-merging
Sep 26, 2018
Merged

Fix commonjs export= merging (#27368)#27371
sandersn merged 1 commit intomasterfrom
fix-commonjs-exportequals-merging

Conversation

@sandersn
Copy link
Copy Markdown
Member

I'm surprised we haven't seen more of this; I suspect it's because the
mixed module.exports= + export.foo= pattern isn't that common.
However, it'll happen any time that the exported symbol is unknown;
getCommonJsExportEquals blithely clones unknownSymbol and proceeds to
stick the exports.foo= properties onto it.

This causes problems later, because the compiler checks for
unknownSymbol with ===. The fix is to not stick properties onto a
clone of unknownSymbol. This makes the correct errors appear and removes
the crash.

Port of #27368 to master

I'm surprised we haven't seen more of this; I suspect it's because the
mixed `module.exports=` + `export.foo=` pattern isn't that common.
However, it'll happen any time that the exported symbol is unknown;
getCommonJsExportEquals blithely clones unknownSymbol and proceeds to
stick the `exports.foo=` properties onto it.

This causes problems later, because the compiler checks for
unknownSymbol with `===`. The fix is to not stick properties onto a
clone of unknownSymbol. This makes the correct errors appear and removes
the crash.
@sandersn sandersn merged commit 98ec1e8 into master Sep 26, 2018
@sandersn sandersn deleted the fix-commonjs-exportequals-merging branch September 26, 2018 19:40
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

1 participant