Skip to content

Compiler gives unhelpful error messages in the presence of multiple default exports #5084

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

Merged
merged 1 commit into from
Oct 9, 2015

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Oct 2, 2015

Proposed fix for #3886

The patch is not finished. I would just like to get some feedback from you guys if I'm on the right track or if you would choose another approach.

Thanks!

@msftclas
Copy link

msftclas commented Oct 2, 2015

Hi @MartyIX, 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;

@MartyIX
Copy link
Contributor Author

MartyIX commented Oct 4, 2015

Issue #95 seems very similar to this one. Maybe another IF statement would fixed it as well.

@MartyIX MartyIX force-pushed the patch-4 branch 2 times, most recently from 1110a26 to d0bfd00 Compare October 7, 2015 14:01

if (symbol.name === "default") {
// @todo(marty) Replace with Diagnostics.<something>
message = { code: 2300, category: ts.DiagnosticCategory.Error, key: "'default' keyword used multiple times." };
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 add a diagnostic with a unique code to diagnosticMessages.json. Then use the appropriate generated code.

Copy link
Member

Choose a reason for hiding this comment

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

Make the message "A module cannot have multiple default exports."

@DanielRosenwasser
Copy link
Member

Can you add the following 3 tests?

tests/cases/conformance/es6/modules/defaultExportWIthOverloads01.ts

export default function f();
export default function f(x: string);
export default function f(...args: any[]) {
}

tests/cases/conformance/es6/modules/multipleDefaultExports03.ts

export default function f() {
}
export default function f() {
}

tests/cases/conformance/es6/modules/multipleDefaultExports04.ts

export default class C {
}

export default class C {
}

@MartyIX MartyIX force-pushed the patch-4 branch 3 times, most recently from 2e168d2 to 32d5825 Compare October 9, 2015 12:18
export default class C {
}

// @filename: m2.ts
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't want two separate files, I wanted both declarations to occur in the same file.

Also, can you add multipleDefaultExports04?

export default function f() {
}
export default function f() {
}

@DanielRosenwasser
Copy link
Member

Hey @MartyIX - unfortunately I gave some bad feedback a little bit earlier regarding multiple default-exported functions. I said that "Duplicate function implementation" was the correct error message to give; however, I think that would be the right message to give only if the function declaration names are not the same.

Still, I think this work is great and we can take it in now. Thanks for the fix!

DanielRosenwasser added a commit that referenced this pull request Oct 9, 2015
Compiler gives unhelpful error messages in the presence of multiple default exports
@DanielRosenwasser DanielRosenwasser merged commit 12b436b into microsoft:master Oct 9, 2015
@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.

3 participants