Skip to content

Give a better message for multiple default exported functions of differing names #5198

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

Closed

Conversation

DanielRosenwasser
Copy link
Member

This is a follow up to @MartyIX's work on #5084.

With this change, instead of reporting Duplicate function implementation. for the following:

export default function foo() { }
export default function bar() { }

We will report A module cannot have multiple default exports.

However, for

export default function foo() { }
export default function foo() { }

we will report the original error message.

We also now correctly report on default-exported function declarations with no names.

if (isConstructor) {
multipleConstructorImplementation = true;
}
else {
duplicateFunctionDeclaration = true;
duplicateDefaultExports = !!(node.flags & NodeFlags.Default);
Copy link
Member

Choose a reason for hiding this comment

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

At this point we're not sure that it's a duplicate default export, right? Maybe the name should be different.
(I could be wrong, I've only skimmed areAllFunctionDeclarations... so far)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, these are all declarations of the same symbol, so if one is a default export, they should all be default exports.

Still, that's a great assertion to check here. I'll add it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I realized that we don't need the extra variable, we can just check the implementation.

*
* It is probably more accurate to say that there are duplicate function implementations.
*
* @param declarations
Copy link
Member

Choose a reason for hiding this comment

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

I don't know about our JSDoc style, but in the past I have skipped parameters that had no associated documentation. Is that allowed in JSDoc? Alternatively, instead of deleting this line, it would be nice to have some text here.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I'll remove it

@DanielRosenwasser DanielRosenwasser deleted the multipleDefaultFunctionsOfDifferingNames branch February 26, 2016 08:20
@DanielRosenwasser DanielRosenwasser restored the multipleDefaultFunctionsOfDifferingNames branch February 26, 2016 08:20
@mhegazy mhegazy deleted the multipleDefaultFunctionsOfDifferingNames branch November 2, 2017 21:05
@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