Skip to content

[Codegen]: Extract getConfigType function to parsers/utils.js#35035

Closed
dhruvtailor7 wants to merge 3 commits into
facebook:mainfrom
dhruvtailor7:extract-get-config-type
Closed

[Codegen]: Extract getConfigType function to parsers/utils.js#35035
dhruvtailor7 wants to merge 3 commits into
facebook:mainfrom
dhruvtailor7:extract-get-config-type

Conversation

@dhruvtailor7
Copy link
Copy Markdown
Contributor

Summary

This PR is part of #34872.
This PR contains two changes, extracting the visitor object and then factoring out the getConfigType() function to the parsers/utils.js file. Then we can reuse the same function in both flow and typescript by passing the extracted visitor object.

Changelog

[Internal] [Changed] - Extract visitor object in the same file and factor out getConfigType() to parsers/utils.js.

Test Plan

Output of yarn jest react-native-codegen ensures all passed test cases

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 20, 2022
Copy link
Copy Markdown
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you a lot for doing this! 👏
I left a small nit/question, but it looks good to me!


visit(ast, {
CallExpression(node) {
function Visitor(infoMap: {isComponent: boolean, isModule: boolean}) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the V is capital on purpose because this is a type? Or can we have it lowercase like all the other functions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I kept capital on purpose. Should i change it lowercase?

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Oct 20, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,754,795 -2,079
android hermes armeabi-v7a 7,159,067 -1,901
android hermes x86 8,066,601 -1,877
android hermes x86_64 8,037,253 -1,952
android jsc arm64-v8a 9,612,937 -1,628
android jsc armeabi-v7a 8,379,072 -1,463
android jsc x86 9,559,947 -1,444
android jsc x86_64 10,152,843 -1,512

Base commit: ff398e4
Branch: main

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Oct 20, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ff398e4
Branch: main

@pull-bot
Copy link
Copy Markdown

PR build artifact for 336265b is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link
Copy Markdown

PR build artifact for 336265b is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

Copy link
Copy Markdown
Contributor

@cortinico cortinico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just left a minor comment

Comment thread packages/react-native-codegen/src/parsers/flow/index.js Outdated
@pull-bot
Copy link
Copy Markdown

PR build artifact for 2f76ad4 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link
Copy Markdown

PR build artifact for 2f76ad4 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. hacktoberfest-accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants