Handle (T) and undefined properly in turbo module component codegen

[ZihanChen-MSFT]

Summary

In buildEventSchema and buildPropSchema, they check into property types to see if the given property could be converted into an event schema or a property schema. The original implementation only handles limited cases, I refactor them and make them easier to maintain.

In getPropertyType in events.js, it handles (T) at a wrong place, fixed.

In getPropertyType in props.js, it doesn't handle (T), fixed.

And I also fixed some other issues to make the codegen reports error better.

There are many duplicated test cases that cover every piece of the code, I changed some of them so that it tests both original cases and new cases.

Changelog

[General] [Changed] - Handle (T) and undefined properly in turbo module component codegen

Test Plan

yarn jest passed in packages/react-native-codegen

[cipolleschi]

Thank you for taking the time for this PR.
I left a few comments and some improvements that we can apply. Let me know what do you think.

I also have the feeling that this PR is doing a little bit too much: support parenthesized types, changing null for undefined, refactoring, error handling... It will be better to split it in multiple PR, what do you think?

[cipolleschi]

I know that this probably does not change anything, but I think we should keep also the other syntax, without parenthesis, to make sure that all the use cases are covered.

[ZihanChen-MSFT]

Sure, if you scroll down the file you will see a lot of similar thing, which remains unchanged. So cases without parenthesis are still covered.

[cipolleschi]

same as above.

[ZihanChen-MSFT]

BubblingEventHandler cases below still covered syntax without parenthesis.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,694,713	+36
android	hermes	armeabi-v7a	7,098,120	+38
android	hermes	x86	7,997,323	+51
android	hermes	x86_64	7,969,796	+54
android	jsc	arm64-v8a	9,565,094	+36
android	jsc	armeabi-v7a	8,332,078	+46
android	jsc	x86	9,505,525	+52
android	jsc	x86_64	10,097,156	+72

Base commit: ab5f26b
Branch: main

[ZihanChen-MSFT]

I also have the feeling that this PR is doing a little bit too much: support parenthesized types, changing null for undefined, refactoring, error handling... It will be better to split it in multiple PR, what do you think?

Hi, @cipolleschi , in general I do think smaller PR is better, but in this case, the "first event type argument could be void->undefined or null" one is just one line of code and it is embedded in the refactored code, and if I split into 2 PR it will get conflict no matter which one is merged first. So in this particular case I do think combining them in one is better.

[analysis-bot]

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

Base commit: ab5f26b
Branch: main

[facebook-github-bot]

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

[facebook-github-bot]

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

[react-native-bot]

This pull request was successfully merged by @ZihanChen-MSFT in 205cc9b.

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