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

Add separateDataFiles to @rnw/cli and @rnw/codegen #11990

Merged
merged 13 commits into from
Aug 14, 2023

Conversation

ZihanChen-MSFT
Copy link
Contributor

@ZihanChen-MSFT ZihanChen-MSFT commented Aug 8, 2023

Description

Type of Change

  • New feature (non-breaking change which adds functionality)

Why

In devmain there are already requirements that, types involved in a turbo module needs to be part of the API.
Such project structure would be:

  • The API implemented in C++
  • Native code calls the API
  • The turbo module calls the API
  • JS code calls the turbo module

When a feature needs to be available to both C++ and JS, it is possible that the API and the turbo module will share some data structures. If by semantic they are always exactly the same, why not just publish generated types?

What

In order to complete the story, the turbo module C++ codegen needs to generate a clean header files for custom types if required. Setting codegenConfig.windows.separateDataTypes does this.

Testing

Tested in sample-apps

Changelog

Should this change be included in the release notes: yes

npx react-native codegen-windows will generate two files if there is any custom type to generate. The additional file NativeXXXDataTypes.g.h contains all generated custom types.

Microsoft Reviewers: codeflow:open?pullrequest=https://github.com/microsoft/react-native-windows/pull/11990&drop=dogfoodAlpha

@acoates-ms
Copy link
Contributor

acoates-ms commented Aug 8, 2023

Can we split the two files into separate generators, which would align with core where there are separate generators for the cpp vs h files?
Then instead of having to have another option for the config, we can just specify the generators we require.

@ZihanChen-MSFT
Copy link
Contributor Author

Can we split the two files into separate generators, which would align with core where there are separate generators for the cpp vs h files? Then instead of having to have another option for the config, we can just specify the generators we require.

Both file needs the input (in the intermediate format) to be compiled again to identify anonymous custom types before generating it, they shared many complex logic. I think it is good to just keep them in one generator.

@ZihanChen-MSFT ZihanChen-MSFT force-pushed the rm-typefile branch 2 times, most recently from f44cead to 69f918a Compare August 8, 2023 22:51
@ZihanChen-MSFT ZihanChen-MSFT requested a review from a team as a code owner August 9, 2023 22:26
@ZihanChen-MSFT ZihanChen-MSFT changed the title Add allInOne to @rnw/cli and @rnw/codegen Generate types and spec in two files for turbo module Aug 9, 2023
@ZihanChen-MSFT ZihanChen-MSFT force-pushed the rm-typefile branch 2 times, most recently from 787e00b to 322793d Compare August 11, 2023 19:36
@ZihanChen-MSFT ZihanChen-MSFT enabled auto-merge (squash) August 11, 2023 19:52
@ZihanChen-MSFT ZihanChen-MSFT force-pushed the rm-typefile branch 3 times, most recently from 641525b to 7bed9f6 Compare August 14, 2023 18:15
@ZihanChen-MSFT ZihanChen-MSFT changed the title Generate types and spec in two files for turbo module Add separateDataFiles to @rnw/cli and @rnw/codegen Aug 14, 2023
Copy link
Member

@vmoroz vmoroz left a comment

Choose a reason for hiding this comment

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

:shipit:

@ZihanChen-MSFT ZihanChen-MSFT merged commit 690d70c into microsoft:main Aug 14, 2023
42 checks passed
ZihanChen-MSFT added a commit to ZihanChen-MSFT/react-native-windows that referenced this pull request Aug 15, 2023
* Add `allInOne` to `createNM2Generator`

* Add `allInOne` to @rnw/cli and @rnw/codegen

* ...

* ...

* ...

* Update sample-apps

* Change files

* Fix code review comment

* Remove `allInOne` option and make it always happen

* Update generated files

* Update #include

* Fix build break

* Suppress clang-format on some include order
ZihanChen-MSFT added a commit to ZihanChen-MSFT/react-native-windows that referenced this pull request Aug 15, 2023
* Add `allInOne` to `createNM2Generator`

* Add `allInOne` to @rnw/cli and @rnw/codegen

* ...

* ...

* ...

* Update sample-apps

* Change files

* Fix code review comment

* Remove `allInOne` option and make it always happen

* Update generated files

* Update #include

* Fix build break

* Suppress clang-format on some include order
ZihanChen-MSFT added a commit that referenced this pull request Aug 15, 2023
* @rnw/codegen generates GetStructInfo instead of REACT_STRUCT (#11982)

* #include <NativeModules.h>

* Separate custom types and reflection in template

* Generate GetStructInfo function instead of REACT_STRUCT

* ...

* Update generated files

* Change files

* ...

* Generate types and spec in two files for turbo module (#11990)

* Add `allInOne` to `createNM2Generator`

* Add `allInOne` to @rnw/cli and @rnw/codegen

* ...

* ...

* ...

* Update sample-apps

* Change files

* Fix code review comment

* Remove `allInOne` option and make it always happen

* Update generated files

* Update #include

* Fix build break

* Suppress clang-format on some include order

* ...

* Change files

* Update packages.lock.json
ZihanChen-MSFT added a commit that referenced this pull request Aug 15, 2023
* @rnw/codegen generates GetStructInfo instead of REACT_STRUCT (#11982)

* #include <NativeModules.h>

* Separate custom types and reflection in template

* Generate GetStructInfo function instead of REACT_STRUCT

* ...

* Update generated files

* Change files

* ...

* Generate types and spec in two files for turbo module (#11990)

* Add `allInOne` to `createNM2Generator`

* Add `allInOne` to @rnw/cli and @rnw/codegen

* ...

* ...

* ...

* Update sample-apps

* Change files

* Fix code review comment

* Remove `allInOne` option and make it always happen

* Update generated files

* Update #include

* Fix build break

* Suppress clang-format on some include order

* ...

* Change files

* Update packages.lock.json
@ZihanChen-MSFT ZihanChen-MSFT deleted the rm-typefile branch August 16, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants