Fix bug parsing anonymous closure types from Flow. #672
Conversation
If no parameter name exists in a function type, then set it to empty. Also,
fix the formatter so that `{param}: {ty}` is formatted as `{ty}` when
`{param}` is empty.
Fixes documentationjs#671
|
I tried to write a test in |
|
I feel like this just tries to work around the issue. We should understand what is really going on here first. |
Is there really anything deeper here other than "function parameters in a type signature may be unnamed"? I don't keep abreast of Flow changes, so maybe a dep bump is needed, but it seems reasonable to have some case analysis like there is in this PR for unnamed parameters. Or were you thinking of something else? |
There was a problem hiding this comment.
Sorry for being slow to review this.
I've now looked at the AST that Babel/Flow uses:
https://flowtype.org/try/#0PTAEAEDMBsHsHcBQiAuBPADgU1AFVALygB2ArgLYBGWAToQHygDOKNAlsQOYDcQA
and the name ends up as null so this change is good.
Can you add a file to test/fixtures that contains
// @flow
/** doc */
type T = number => string;and run the test with UPDATE=1 ... to generate the files.
| @@ -118,7 +118,10 @@ function formatType(getHref/*: Function*/, node/*: ?Object */) { | |||
| case Syntax.NameExpression: | |||
| return [link(node.name, getHref)]; | |||
| case Syntax.ParameterType: | |||
| return [t(node.name + ': ')].concat(formatType(getHref, node.expression)); | |||
| if (node.name) { | |||
| result.push(t(node.name + ': ')); | |||
There was a problem hiding this comment.
So this prints ': T' for the type number => string. Maybe it should just print T?
There was a problem hiding this comment.
Actually, it prints nothing in the case of an empty name... not sure that is what we want?
There was a problem hiding this comment.
If the name is empty, then it prints the value of result.concat(formatType(getHref, node.expression)).
|
Landing this with a follow up commit that has a fixture test. |
This allows `type T = number => string`. Previously we were only handling `type T = (x: number) => string` correctly. Followup to documentationjs#672 that: - Adds a test fixture - Fixes lint error
This allows `type T = number => string`. Previously we were only handling `type T = (x: number) => string` correctly. Followup to #672 that: - Adds a test fixture - Fixes lint errors
|
@arv Thank you so much. :-) |
|
@arv Sorry for the ping, but is there any chance of pushing out a new release with this fix? Thanks. :-) |
If no parameter name exists in a function type, then set it to empty. Also,
fix the formatter so that
{param}: {ty}is formatted as{ty}when{param}is empty.Fixes #671