[Codegen] Extract TypeAlias logic of translateTypeAnnotation from the flow and typescript folders in parsers-primitives#34918
Conversation
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
I think we should share this type between flow and typescript folders. Unfortunately, we can't share all the logic.
Should i move this type only in packages/react-native-codegen/src/parsers/utils.js? It is the closest common utils file between flow and typescript.
Or, should it be done in another task @cipolleschi?
There was a problem hiding this comment.
Yes, please! 😄
If you can move this type up to the parsers/utils.js and reuse it, it would be amazing!
Unfortunately, we can't share all the logic.
What do you mean with this? The type definition seems identical to me. Which logic you feel like we can't share? 🤔
There was a problem hiding this comment.
What do you mean with this? The type definition seems identical to me. Which logic you feel like we can't share? 🤔
I mean we can share TypeAliasResolutionStatus which is identical but not resolveTypeAnnotation which uses this type.
If it had been possible, I would have moved everything in react-native-codegen/src/parsers/utils.js 😄
There was a problem hiding this comment.
Ah yeah... resolveTypeAnnotation is another beast. One of the goals of the umbrella task is to make those two methods as close as possible with incremental changes so that, at some point, we may be able to put them together!
It is expected that it is not the time... yet! :D
There was a problem hiding this comment.
yes, let's refactor step by step 😁
I moved the type!
Base commit: e4dff28 |
cipolleschi
left a comment
cipolleschi
left a comment
There was a problem hiding this comment.
Love this! ❤️ Thank you so much for taking this!!! 😄
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Base commit: e4dff28 |
MaeIg
force-pushed
the
refactor/extract-typeAliasResolution-function
branch
from
580f557 to
2dd655e
Compare
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
/rebase |
It is to import other files with strict-local. We can also use "@flow strict" on all other files, but I don't know if this is what is wanted as it is not already activated.
github-actions
Bot
force-pushed
the
refactor/extract-typeAliasResolution-function
branch
from
2dd655e to
43765f3
Compare
|
@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 @MaeIg in 1fc27c4. When will my fix make it into a release? | Upcoming Releases |
5 participants
Summary
This PR aims to reduce code duplication by extracting
typeAliasResolutionlogic from the flow and typescript folders into a shared parsers-primitives file. It is a task of #34872:Changelog
[Internal] [Changed] - Extract TypeAlias logic of translateTypeAnnotation from the flow and typescript folders in parsers-primitives
Test Plan
All tests are passing, with
yarn jest react-native-codegen: