Skip to content

Avoid the double-symbol trick for enums #39955

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
Aug 12, 2020

Conversation

elibarzilay
Copy link
Contributor

@elibarzilay elibarzilay commented Aug 7, 2020

Nameless jsdoc typedefs have their exportedness controlled by the
exportedness of the location they pull their name from.

Fixes #33575.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Aug 7, 2020
@elibarzilay elibarzilay force-pushed the 33575-w branch 2 times, most recently from 39a980b to 22cb2e6 Compare August 11, 2020 18:37
Nameless jsdoc typedefs have their exportedness controlled by the
exportedness of the location they pull their name from.

Fixes microsoft#33575.
@elibarzilay elibarzilay marked this pull request as ready for review August 11, 2020 19:48
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

But definitely get @sandersn to review, too.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

One question before I sign off.

// 1. It has an explicit name (since by default typedefs are always directly exported, either at the top level or in a container), or
if (!isJSDocEnumTag(node) && !!node.fullName) return true;
// 2. The thing a nameless typedef pulls its name from is implicitly a direct export (either by assignment or actual export flag).
const declName = getNameOfDeclaration(node);
Copy link
Member

Choose a reason for hiding this comment

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

is the code below here only for nameless typedefs, or can declName be a node inside a jsdoc comment?

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I convinced myself that the last half of the new function will do the right thing, but I think there may be jsdoc-only/named typedef cases that fall through to the final return false

@elibarzilay elibarzilay merged commit 620e260 into microsoft:master Aug 12, 2020
@elibarzilay elibarzilay deleted the 33575-w branch August 12, 2020 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jsdoc @enum with import/export
4 participants