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

Ensure equivalent Flow and TypeScript turbo module definition generates the same output #34251

Closed
wants to merge 15 commits into from

Conversation

ZihanChen-MSFT
Copy link
Contributor

@ZihanChen-MSFT ZihanChen-MSFT commented Jul 22, 2022

Summary

Flow and TypeScript are two very similar programming languages. They are both recognizable as inputs for turbo module codegen. It is reasonable to require equivalent Flow and TypeScript turbo module definition generates the same output.

I add some test cases to ensure this. Glad that no functional inconsistency is found here.

Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Test Plan

yarn jest passed in packages/react-native-codegen

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 22, 2022
@kelset kelset added p: Microsoft Partner: Microsoft Partner Tech: Codegen Related to react-native-codegen labels Jul 25, 2022
@cipolleschi
Copy link
Contributor

Hi @ZihanChen-MSFT, thanks for the Pr. Could you update the Changelog section, please? If I'm not mistaken, it is taken automatically and it would be nicer to have an informative message for it. Thanks a lot!

@kelset
Copy link
Contributor

kelset commented Jul 25, 2022

@cipolleschi I took care of the changelog entry :)

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @ZihanChen-MSFT in 0ce4ea2.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 27, 2022
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
…es the same output (facebook#34251)

Summary:
Flow and TypeScript are two very similar programming languages. They are both recognizable as inputs for turbo module codegen. It is reasonable to require equivalent Flow and TypeScript turbo module definition generates the same output.

I add some test cases to ensure this. Glad that no functional inconsistency is found here.

## Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Pull Request resolved: facebook#34251

Test Plan: `yarn jest` passed in `packages/react-native-codegen`

Reviewed By: dmitryrykun

Differential Revision: D38116756

Pulled By: cipolleschi

fbshipit-source-id: 476adbd171a35813923f2020c826d21b1fc2eeed
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
…es the same output (facebook#34251)

Summary:
Flow and TypeScript are two very similar programming languages. They are both recognizable as inputs for turbo module codegen. It is reasonable to require equivalent Flow and TypeScript turbo module definition generates the same output.

I add some test cases to ensure this. Glad that no functional inconsistency is found here.

## Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Pull Request resolved: facebook#34251

Test Plan: `yarn jest` passed in `packages/react-native-codegen`

Reviewed By: dmitryrykun

Differential Revision: D38116756

Pulled By: cipolleschi

fbshipit-source-id: 476adbd171a35813923f2020c826d21b1fc2eeed
facebook-github-bot pushed a commit that referenced this pull request Sep 9, 2022
…es the same output (#34620)

Summary:
Pull request #34251 only partially worked because I didn't notice that there is ` 1` after the snapshot name. In this change I fixed the issue and find out there is one case that Flow and TypeScript parser generate different result.

It is expected since the test inputs are different. TypeScript removes one member because there is no `{...X, ...}` type in TypeScript. We could make the codegen support intersection type and rewrite it to `X & {...}`, but this will not be included here anyway.

## Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Pull Request resolved: #34620

Test Plan: `yarn jest` passed in `packages/react-native-codegen`

Reviewed By: NickGerleman

Differential Revision: D39321965

Pulled By: cipolleschi

fbshipit-source-id: aec60f5c9c18323cbd833bf5705af544d7db2e73
@ZihanChen-MSFT ZihanChen-MSFT deleted the ts-rncodegen4 branch September 9, 2022 21:44
dmytrorykun pushed a commit that referenced this pull request Sep 14, 2022
…es the same output (#34251)

Summary:
Flow and TypeScript are two very similar programming languages. They are both recognizable as inputs for turbo module codegen. It is reasonable to require equivalent Flow and TypeScript turbo module definition generates the same output.

I add some test cases to ensure this. Glad that no functional inconsistency is found here.

## Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Pull Request resolved: #34251

Test Plan: `yarn jest` passed in `packages/react-native-codegen`

Reviewed By: dmitryrykun

Differential Revision: D38116756

Pulled By: cipolleschi

fbshipit-source-id: 476adbd171a35813923f2020c826d21b1fc2eeed
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…es the same output (facebook#34620)

Summary:
Pull request facebook#34251 only partially worked because I didn't notice that there is ` 1` after the snapshot name. In this change I fixed the issue and find out there is one case that Flow and TypeScript parser generate different result.

It is expected since the test inputs are different. TypeScript removes one member because there is no `{...X, ...}` type in TypeScript. We could make the codegen support intersection type and rewrite it to `X & {...}`, but this will not be included here anyway.

## Changelog

[General] [Changed] - codegen: ensure equivalent Flow and TypeScript TM definition generates the same output

Pull Request resolved: facebook#34620

Test Plan: `yarn jest` passed in `packages/react-native-codegen`

Reviewed By: NickGerleman

Differential Revision: D39321965

Pulled By: cipolleschi

fbshipit-source-id: aec60f5c9c18323cbd833bf5705af544d7db2e73
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. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Tech: Codegen Related to react-native-codegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants