Skip to content
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

[Codegen 77] Extract the functions to compute partial properties in the Flow and TypeScript parsers. #36373

Closed

Conversation

MaeIg
Copy link
Contributor

@MaeIg MaeIg commented Mar 4, 2023

Summary

This PR aims to extract the switch(configType) block from the buildSchema function into a separate function in a shared file between typescript and flow. It is a task of #34872:

[Codegen 77 - assigned to @MaeIg] Extract the functions to compute partial properties from the index.js file (Flow and TypeScript)in the Flow and TypeScript parsers.

Changelog

[Internal] [Changed] - Extract the functions to compute partial properties in the Flow and TypeScript parsers.

Test Plan

yarn flow:
image

yarn lint:
image

yarn test
image

@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 Mar 4, 2023
@MaeIg MaeIg changed the title Refactor/extract function partial properties [Codegen 77] Extract the functions to compute partial properties in the Flow and TypeScript parsers. Mar 4, 2023
@MaeIg MaeIg force-pushed the refactor/extract-function-partial-properties branch from ba9e359 to 70abd30 Compare March 5, 2023 00:04
@rshest
Copy link
Contributor

rshest commented Mar 5, 2023

@MaeIg Could you please add tests for that as well?

@analysis-bot
Copy link

analysis-bot commented Mar 5, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,509,794 +922
android hermes armeabi-v7a 7,823,765 +917
android hermes x86 8,987,370 +923
android hermes x86_64 8,843,548 +928
android jsc arm64-v8a 9,136,772 +1,088
android jsc armeabi-v7a 8,326,312 +1,078
android jsc x86 9,189,716 +1,079
android jsc x86_64 9,448,710 +1,081

Base commit: 24dbb5d
Branch: main

@MaeIg MaeIg force-pushed the refactor/extract-function-partial-properties branch from 70abd30 to 9f5bf40 Compare March 6, 2023 22:00
@MaeIg
Copy link
Contributor Author

MaeIg commented Mar 6, 2023

@MaeIg Could you please add tests for that as well?

Hello @rshest,

I added a test for each parser. I'm not sure these tests are useful because there is only one branch and everything is already covered by higher level tests.

@facebook-github-bot
Copy link
Contributor

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

Comment on lines +189 to +195
* @paramater properties: properties from a Partial types.
* @parameter hasteModuleName: a string with the native module name.
* @paramater types: a map of type declarations.
* @paramater aliasMap: a map of type aliases.
* @paramater enumMap: a map of type enums.
* @paramater tryParse: a parser error capturer.
* @paramater cxxOnly: a boolean specifying if the module is Cxx only.
Copy link
Contributor

Choose a reason for hiding this comment

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

"parameter"?.. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups sorry for the typo!

I see that you fixed the typo and merged the PR, thank you! :)

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 7, 2023
@facebook-github-bot
Copy link
Contributor

@rshest merged this pull request in a448c6d.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…ypeScript parsers. (facebook#36373)

Summary:
This PR aims to extract  the switch(configType) block from the buildSchema function into a separate function in a shared file between typescript and flow. It is a task of facebook#34872:
> [Codegen 77 - assigned to MaeIg] Extract the functions to compute partial properties from the index.js file ([Flow](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/flow/modules/index.js#L180-L194) and [TypeScript](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L261-L278))in the Flow and TypeScript parsers.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Internal] [Changed] - Extract the functions to compute partial properties in the Flow and TypeScript parsers.

Pull Request resolved: facebook#36373

Test Plan:
yarn flow:
<img width="560" alt="image" src="https://user-images.githubusercontent.com/40902940/222934040-97e88691-c1d6-44a1-b7ee-e500b4698cd2.png">

yarn lint:
<img width="509" alt="image" src="https://user-images.githubusercontent.com/40902940/222934137-05b6f1fd-a755-4323-baac-5954d36fd613.png">

yarn test
<img width="388" alt="image" src="https://user-images.githubusercontent.com/40902940/222934145-76a2bc38-7469-4c30-9301-bc21cfadded4.png">

Reviewed By: cipolleschi

Differential Revision: D43867937

Pulled By: rshest

fbshipit-source-id: f78fe399ff7d7d2cf8f44d1165418566b0a25628
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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants