[Codegen 84] Merge Parse-Module-Name `anon` fn of `Flow` & `TS` parsers' #36297

Pranav-yadav · 2023-02-26T10:36:53Z

Summary

Part of Umbrella #34872

[Codegen 84 - assigned to @Pranav-yadav] It depends on [Codegen 83] export the parseModuleName anonymous function (Flow, TypeScript) in a common parseModuleName function in the parsers-commons.js file.

merged Parse Module-Name anon fn of Flow & TS parsers; into a common parseModuleName fn in the parsers-commons.js
added tests for parseModuleName fn from parsers-commons.js
added callExpressionTypeParameters method to parsers
added tests for callExpressionTypeParameters method of parsers
used parser.callExpressionTypeParameters method in parseModuleName fn

PS: fixed merge conflicts several times :(

Overall :)

Changelog

[INTERNAL] [CHANGED] - Merge Parse-Module-Name anon fn of Flow & TS and add callExpressionTypeParameters method to parsers

Test Plan

yarn lint && yarn run flow && yarn test react-native-codegen ==> ✅

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js

analysis-bot · 2023-02-26T10:58:48Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,477,095	+0
android	hermes	armeabi-v7a	7,799,079	+0
android	hermes	x86	8,953,089	+0
android	hermes	x86_64	8,810,587	+0
android	jsc	arm64-v8a	9,108,176	+0
android	jsc	armeabi-v7a	8,304,844	+0
android	jsc	x86	9,159,032	+0
android	jsc	x86_64	9,418,319	+0

Base commit: a49446b
Branch: main

cipolleschi

@Pranav-yadav, thank you so much and great job!

I left a couple of comments that should be addressed before merging this PR.

Thank you again for the work you are putting into this!

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js

packages/react-native-codegen/src/parsers/parsers-commons.js

cipolleschi

Let's remove the commented test (see my comment for the why). I'll import and merge the PR.

Thank you for this work!

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js

Pranav-yadav · 2023-02-27T13:08:43Z

Let's remove the commented test (see my comment for the why). I'll import and merge the PR.
Thank you for this work!

aaah... Exactly... I've spent hours debugging it (the generated ASTs 😞) and going through the base test cases but they couldn't help as they use fake manually created ASTs and not the actual one created by parser. PS: was going to add counter for "hours wasted here :(", jk

Thanks. I'll rm that test.

cipolleschi

Thank you so much for taking care of this!

facebook-github-bot · 2023-03-01T12:24:00Z

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

Pranav-yadav · 2023-03-01T16:16:07Z

I see conflicts again... :(

Rebasing quickly to fix :)
Edit: rebased and fixed

@cipolleschi

- into a common `parseModuleName` fun in the `parsers-commons.js`

- in `parsers-commons-test.js` - related to: [Codegen 84]

@cipolleschi

…ror` in `parseModuleName` fn - reason: The problem here is that; ```flow export default TurboModuleRegistry.getEnforcing('SampleTurboModule'); ``` this result in an `UntypedModuleRegistryCallParserError`, because it is "not generic". The problem is that, to get a type for it, it "must" be a generic. I can't think of any other way for `getEnforcing` to be typed with a different type rather than generic. 🤔 I don't think there is a way to create a valid Flow code which is typed with something that is not a generic. I looked at the base test for that throw and they are manually creating an AST that satisfy that requirement, but that AST probably can't be actually generated. 🤔 -- @cipolleschi

facebook-github-bot · 2023-03-02T12:07:51Z

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

facebook-github-bot · 2023-03-02T16:34:39Z

@cipolleschi merged this pull request in 05454fa.

…#36297) Summary: Part of Umbrella facebook#34872 > [**Codegen 84** - assigned to Pranav-yadav] It depends on [Codegen 83] export the parseModuleName anonymous function (Flow, TypeScript) in a common parseModuleName function in the parsers-commons.js file. - merged Parse Module-Name _**anon**_ fn of `Flow` & `TS` parsers; into a common `parseModuleName` fn in the `parsers-commons.js` - added **tests** for `parseModuleName` fn from `parsers-commons.js` - added `callExpressionTypeParameters` method to **_parsers_** - added **tests** for `callExpressionTypeParameters` method of _parsers_ - used `parser.callExpressionTypeParameters` method in `parseModuleName` fn PS: fixed merge conflicts several times :( Overall :) ## Changelog [INTERNAL] [CHANGED] - Merge Parse-Module-Name anon fn of `Flow` & `TS` and add `callExpressionTypeParameters` method to **_parsers_** Pull Request resolved: facebook#36297 Test Plan: - `yarn lint && yarn run flow && yarn test react-native-codegen` ==> ✅ Reviewed By: rshest Differential Revision: D43694563 Pulled By: cipolleschi fbshipit-source-id: 99cf40ada0a567cd9ff91078f66fd4ac3684f7cc

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 Feb 26, 2023

Pranav-yadav commented Feb 26, 2023

View reviewed changes

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js Outdated Show resolved Hide resolved

cipolleschi requested changes Feb 26, 2023

View reviewed changes

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js Outdated Show resolved Hide resolved

packages/react-native-codegen/src/parsers/parsers-commons.js Outdated Show resolved Hide resolved

Pranav-yadav requested a review from cipolleschi February 26, 2023 18:23

cipolleschi requested changes Feb 27, 2023

View reviewed changes

packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js Outdated Show resolved Hide resolved

Pranav-yadav force-pushed the codegen-84-parseModuleName branch from 1db994a to aa19750 Compare February 27, 2023 13:59

Pranav-yadav requested a review from cipolleschi February 27, 2023 16:38

cipolleschi mentioned this pull request Feb 28, 2023

☂️ Improving Codegen #34872

Closed

cipolleschi approved these changes Mar 1, 2023

View reviewed changes

Pranav-yadav added 7 commits March 1, 2023 16:25

Merge Parse Module-Name anon fun of Flow & TS

9298b75

- into a common `parseModuleName` fun in the `parsers-commons.js`

add tests for parseModuleName fn from parsers-commons.js

aee154a

- in `parsers-commons-test.js` - related to: [Codegen 84]

add callExpressionTypeParameters method to parsers

dd90fe2

add tests for callExpressionTypeParameters method of parsers

53b07f5

use parser.callExpressionTypeParameters method in parseModuleName fn

8b65bd2

rebase on main, fix merge conflicts again :(

2b6a831

Pranav-yadav force-pushed the codegen-84-parseModuleName branch from aa19750 to 2b6a831 Compare March 1, 2023 16:40

facebook-github-bot closed this in 05454fa Mar 2, 2023

facebook-github-bot added the Merged This PR has been merged. label Mar 2, 2023

Pranav-yadav deleted the codegen-84-parseModuleName branch March 2, 2023 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Codegen 84] Merge Parse-Module-Name `anon` fn of `Flow` & `TS` parsers' #36297

[Codegen 84] Merge Parse-Module-Name `anon` fn of `Flow` & `TS` parsers' #36297

Uh oh!

Pranav-yadav commented Feb 26, 2023 •

edited

Loading

Uh oh!

Uh oh!

analysis-bot commented Feb 26, 2023 •

edited

Loading

Uh oh!

cipolleschi left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cipolleschi left a comment

Uh oh!

Uh oh!

Pranav-yadav commented Feb 27, 2023

Uh oh!

cipolleschi left a comment

Uh oh!

facebook-github-bot commented Mar 1, 2023

Uh oh!

Pranav-yadav commented Mar 1, 2023 •

edited

Loading

Uh oh!

facebook-github-bot commented Mar 2, 2023

Uh oh!

facebook-github-bot commented Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[Codegen 84] Merge Parse-Module-Name anon fn of Flow & TS parsers' #36297

[Codegen 84] Merge Parse-Module-Name anon fn of Flow & TS parsers' #36297

Uh oh!

Conversation

Pranav-yadav commented Feb 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Test Plan

Uh oh!

Uh oh!

analysis-bot commented Feb 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cipolleschi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Pranav-yadav commented Feb 27, 2023

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 1, 2023

Uh oh!

Pranav-yadav commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Mar 2, 2023

Uh oh!

facebook-github-bot commented Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[Codegen 84] Merge Parse-Module-Name `anon` fn of `Flow` & `TS` parsers' #36297

[Codegen 84] Merge Parse-Module-Name `anon` fn of `Flow` & `TS` parsers' #36297

Pranav-yadav commented Feb 26, 2023 •

edited

Loading

analysis-bot commented Feb 26, 2023 •

edited

Loading

cipolleschi left a comment •

edited

Loading

Pranav-yadav commented Mar 1, 2023 •

edited

Loading