-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat(jest-docblock): support multiple of the same @pragma #5502
Conversation
3fad3f7
to
e58ac7f
Compare
Codecov Report
@@ Coverage Diff @@
## master #5502 +/- ##
==========================================
+ Coverage 61.67% 61.71% +0.03%
==========================================
Files 213 213
Lines 7071 7075 +4
Branches 3 3
==========================================
+ Hits 4361 4366 +5
+ Misses 2709 2708 -1
Partials 1 1
Continue to review full report at Codecov.
|
Awesome, thanks for the fix. Internally at FB we discussed making this throw instead and I think for our internal use cases this makes sense, however I do not think that in open source Jest we can make such a decision. It's easier to support multiple of the same directives without having to ask users to change their behavior. |
Yeah, I can easily imagine this breaking TypeScript JSDoc, e.g: /**
* @typedef {Object} SpecialType - creates a new type named 'SpecialType'
* @property {string} prop1 - a string property of SpecialType
* @property {number} prop2 - a number property of SpecialType
* @property {number=} prop3 - an optional number property of SpecialType
* @prop {number} [prop4] - an optional number property of SpecialType
* @prop {number} [prop5=42] - an optional number property of SpecialType with default value
*/ |
Thanks for doing this! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Currently, when
jest-docblock
parses:we get
{ foo: 'y' }
.This PR changes it to
{ foo: ['x', 'y'] }
when there are multiple.The PR also adds support for that in
docblock.print()
.Test plan
Unit tests added and passing.
cc @vjeux