-
-
Notifications
You must be signed in to change notification settings - Fork 303
Add support for function paramets without names #218
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
Conversation
README.md
Outdated
"raw":"string => void", | ||
"signature":{ | ||
"arguments":[ | ||
{ "name":"_", "type":{ "name":"string" } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about instead of using '_'
setting the field to null
? It seems clearer to me that a consumer of react-docgen checks if the name === null
instead of a "random" string name === '_'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @danez,
Got the inspiration from a flow-addon for atom (see screenshot). But I agree that null
would be a better and cleaner value for working with the data afterwards. Will update the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: getNameOrValue
returns a string value, and therefore null
would break flow. What do you think @danez?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yes seems it is used quite a bit. We could maybe return an empty string? Then !name
would work in the resulting documentation.
My intention is just that we should return something else than a non-empty string, because in the '_'
case you would not be able to differ if the variable name was _
or empty.
And consumers could still default to _
in the empty case, if the do not display shorthands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR setting the name to an empty string now.
758044d
to
dad3800
Compare
src/utils/getNameOrValue.js
Outdated
@@ -25,6 +25,8 @@ export default function getNameOrValue(path: NodePath, raw?: boolean): string { | |||
return node.name; | |||
case types.Literal.name: | |||
return raw ? node.raw : node.value; | |||
case types.FunctionTypeParam.name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this here seems weird to me. I wouldn't expect getNameOrValue
to be called with with a type annotation (but who knows...). It's also not quite right, since some FunctionTypeParam
do have a name.
Personally I feel it makes more sense to fix this in getFlowType
. If the name of the FunctionTypeParam
is null
, assign an empty string instead. Something like
name: param.node.name ? param.node.name.name || '';
@danez , why does this use getProperty
to extract the name at the moment? Shouldn't it directly by getNameOrValue(param.get('name'))
or just param.node.name.name
?
Am I looking at the wrong syntax tree or am I missing a case here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I have no idea :) But seems unnecessary as it can never be computed or be a literal.
Doing name: param.node.name ? param.node.name.name || '';
should be correct in getFlowType
unless the tests disagree.
Add support for flow functions with parameters that doesn't have a name. Eg. `value: string => void`. It gives the argument a name equal to an empty string: ''
dad3800
to
e85302d
Compare
Hi @fkling, |
Great, thank you all! |
Add support for flow functions with parameters that doesn't have a name.
Eg.
value: string => void
andvalue: (string, number) => void
This kind of functions ins't currently supported by
react-docgen
, and it's unable to parse the file.Source: flow