Skip to content

Fix the order of JSDocTag kinds#19589

Merged
sandersn merged 1 commit into
masterfrom
reorder-jsdoctag-kinds
Oct 30, 2017
Merged

Fix the order of JSDocTag kinds#19589
sandersn merged 1 commit into
masterfrom
reorder-jsdoctag-kinds

Conversation

@sandersn
Copy link
Copy Markdown
Member

JSDocTypeLiteral should not be grouped with the JSDocTag kinds, so move it just before they start, instead of just after. This updates the LastJSDocNode and LastJSDocTagNode.

Found in palantir/tslint#3414

JSDocTypeLiteral should not be grouped with the JSDocTag kinds, so move
it just before they start, instead of just after. This updates the
LastJSDocNode and LastJSDocTagNode.
@sandersn sandersn requested a review from a user October 30, 2017 22:04
@sandersn
Copy link
Copy Markdown
Member Author

@weswigham @mhegazy I can't remember if shuffles in the SyntaxKind enum are OK to have as breaking changes, since this is a const enum. I think technically they are no worse than any other breaking change, but practically they might be more confusing.

@ghost
Copy link
Copy Markdown

ghost commented Oct 30, 2017

It would be nice if we had a way to declare that something is an enum value but not declare what number it is. Users shouldn't be depending on JSDocPropertyTag being exactly 284.

@sandersn
Copy link
Copy Markdown
Member Author

@Andy-MS we do, it's a normal enum instead of a const enum. But those are slower, so we don't use them. I remember @weswigham describing some shenanigans to ensure that we at least make a regular enum available in our API, but I don't remember how resilient those shenanigans are.

@sandersn sandersn merged commit f26ac9f into master Oct 30, 2017
@sandersn sandersn deleted the reorder-jsdoctag-kinds branch October 30, 2017 23:07
@weswigham
Copy link
Copy Markdown
Member

I remember @weswigham describing some shenanigans to ensure that we at least make a regular enum available in our API, but I don't remember how resilient those shenanigans are.

We grep for const enum (.*) { in our .d.ts and replace with enum $1 {.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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