Extracted UnsupportedFunctionReturnTypeAnnotationParserError to throwIfUnsupportedFunctionReturnTypeAnnotationParserError#34965
Conversation
…IfUnsupportedFunctionReturnTypeAnnotationParserError
Base commit: 31f2199 |
Base commit: 31f2199 |
cipolleschi
left a comment
There was a problem hiding this comment.
Hi, thank you so much for taking this task! 👏
I left a very small comment on naming. A part from that, it looks good to me!
|
|
||
| function throwIfUnsupportedFunctionReturnTypeAnnotationParserError( | ||
| nativeModuleName: string, | ||
| flowReturnTypeAnnotation: $FlowFixMe, |
There was a problem hiding this comment.
nit: can we change the name of the parameter with just returnTypeAnnotation?
At this point, we are not discriminating between the language in the params.
Thank you so much!
There was a problem hiding this comment.
Then should I remove the returnTypeAnnotation on line 25? Can both of them be handled from a common variable?
There was a problem hiding this comment.
I'm not sure I am following your suggestion. My comment was related to the name of the parameter we are using.
Can you expand on how will you remove the returnTypeAnnotation? Would you like to pass directly the returnTypeAnnotation.type value?
There was a problem hiding this comment.
I have made some changes to the parameter name. I hope that works. Please let me know what you think.
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
This pull request was successfully merged by @harsh-siriah in 3247436. When will my fix make it into a release? | Upcoming Releases |
Summary
This PR is part of #34872
This PR extracts
UnsupportedFunctionReturnTypeAnnotationParserErrorexception to a separate function inside anerror-utils.jsfileChangelog
[Internal] [Changed] - Extract
UnsupportedFunctionReturnTypeAnnotationParserErrorto a seperate function insideerror-utils.jsTest Plan
Added unit case in
error-utils-test.jsfile