Skip to content

Fix 10193: Compiler crash with decorator and two "export default"s#10239

Merged
yuit merged 14 commits into
masterfrom
fix10193
Oct 6, 2016
Merged

Fix 10193: Compiler crash with decorator and two "export default"s#10239
yuit merged 14 commits into
masterfrom
fix10193

Conversation

@yuit
Copy link
Copy Markdown
Contributor

@yuit yuit commented Aug 9, 2016

Fix #10193

Comment thread src/compiler/binder.ts Outdated
: Diagnostics.Duplicate_identifier_0;

// If the current node has NodeFlags.Default (e.g. if the node is class declaration or function declaration)
// and there is already another default export (i.e. symbol.declarations is not empty), we need to report such error
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.

It took me a while to tie this together. How about

// If the current node is a default export of some sort, then check if
// there are any other default exports that we need to error on.
// We'll know whether we have other default exports depending on if `symbol` already has a declaration list set.

Kanchalai Tanglertsampan added 4 commits August 11, 2016 10:04
@yuit
Copy link
Copy Markdown
Contributor Author

yuit commented Aug 15, 2016

🔔 @DanielRosenwasser

Comment thread src/compiler/binder.ts Outdated
}
else {
// This is to properly report an error in the case "export default { }" is after export default of class declaration or function declaration.
forEach(symbol.declarations, declaration => {
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.

use for..of and beak on the first matching entry.

Comment thread src/compiler/binder.ts

forEach(symbol.declarations, declaration => {
if (declaration.flags & NodeFlags.Default) {
if (symbol.declarations && symbol.declarations.length) {
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.

Do we need to do this loop? what if we just do something like:

if (symbol.declarations && symbol.declarations.length && 
   (isDefaultExport || (node.kind === SyntaxKind.ExportAssignment &&  !(<ExportAssignment>node).isExportEquals)) {
    message = Diagnostics.A_module_cannot_have_multiple_default_exports;
}

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Sep 14, 2016

@yuit what is the status of this PR?

@yuit
Copy link
Copy Markdown
Contributor Author

yuit commented Sep 30, 2016

@mhegazy the pr slip my attention 😅 with all 2.0 bugs. gonna update now
🔔 updated

@yuit yuit merged commit a67ad06 into master Oct 6, 2016
@yuit yuit deleted the fix10193 branch October 6, 2016 21:59
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants