Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators #36030
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
Base commit: e168db4 |
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
vzaidman
added a commit
to vzaidman/react-native
that referenced
this pull request
Feb 1, 2023
…es generators (facebook#36030) Summary: Pull Request resolved: facebook#36030 Generate enum types in c++ turbo modules. This would export enums to TM H files such as: ``` enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 }; ``` And then use these enums in the TM schema such as: ``` public: virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0; ``` Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members. ``` public: virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0; ``` Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators Differential Revision: D42884147 fbshipit-source-id: 18890e2f12c337d32c7e31a27604249027c1ec37
vzaidman
force-pushed
the
export-D42884147
branch
from
a7604f8 to
e4eb7b7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
vzaidman
added a commit
to vzaidman/react-native
that referenced
this pull request
Feb 1, 2023
…es generators (facebook#36030) Summary: Pull Request resolved: facebook#36030 Generate enum types in c++ turbo modules. This would export enums to TM H files such as: ``` enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 }; ``` And then use these enums in the TM schema such as: ``` public: virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0; ``` Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members. ``` public: virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0; ``` Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators Differential Revision: D42884147 fbshipit-source-id: 3245e4995501fd0436013bad1cb1a2d570618861
vzaidman
force-pushed
the
export-D42884147
branch
from
e4eb7b7 to
87984b9
Compare
vzaidman
added a commit
to vzaidman/react-native
that referenced
this pull request
Feb 1, 2023
…es generators (facebook#36030) Summary: Pull Request resolved: facebook#36030 Generate enum types in c++ turbo modules. This would export enums to TM H files such as: ``` enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 }; ``` And then use these enums in the TM schema such as: ``` public: virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0; ``` Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members. ``` public: virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0; ``` Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators Differential Revision: D42884147 fbshipit-source-id: f51b943552a1af5b7ab4a2df94d62031e434da4b
vzaidman
force-pushed
the
export-D42884147
branch
from
87984b9 to
81b58e7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
vzaidman
added a commit
to vzaidman/react-native
that referenced
this pull request
Feb 1, 2023
…es generators (facebook#36030) Summary: Pull Request resolved: facebook#36030 Generate enum types in c++ turbo modules. This would export enums to TM H files such as: ``` enum class NativeEnumTurboModuleStatusNumEnum { Active = 2, Paused = 1, Off = 0 }; ``` And then use these enums in the TM schema such as: ``` public: virtual StatusStrEnum getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual StatusNumEnum getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, StatusStrEnum a, StatusNumEnum b) = 0; ``` Previously, we've only supporting generating these as jsi::String / double, ignoring the enum members. ``` public: virtual jsi::String getStatus(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual double getStatusState(jsi::Runtime &rt, jsi::Object statusProp) = 0; virtual jsi::Object getStateType(jsi::Runtime &rt, jsi::String a, double b) = 0; ``` Changelog: [General] [Added] Generate enum types instead of using string/number in c++ turbo modules generators Differential Revision: D42884147 fbshipit-source-id: 730203057a60e586da3083930e643d7301a794b9
vzaidman
force-pushed
the
export-D42884147
branch
from
81b58e7 to
4274bd0
Compare
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
…g/number in c++ turbo modules generators
Summary:
Generate enum types in c++ turbo modules.
For enums in the ts schema file such as:
```
export enum NumEnum {
ONE = 1,
TWO = 2,
}
```
This would export enums and the relevant Bridging to js and from js code to the spec H files such as:
```
#pragma mark - SampleTurboModuleCxxNumEnum
enum SampleTurboModuleCxxNumEnum { ONE, TWO };
template <>
struct Bridging<SampleTurboModuleCxxNumEnum> {
static SampleTurboModuleCxxNumEnum fromJs(jsi::Runtime &rt, int32_t value) {
if (value == 1) {
return SampleTurboModuleCxxNumEnum::ONE;
} else if (value == 2) {
return SampleTurboModuleCxxNumEnum::TWO;
} else {
throw jsi::JSError(rt, "No appropriate enum member found for value");
}
}
static jsi::Value toJs(jsi::Runtime &rt, SampleTurboModuleCxxNumEnum value) {
if (value == SampleTurboModuleCxxNumEnum::ONE) {
return bridging::toJs(rt, 1);
} else if (value == SampleTurboModuleCxxNumEnum::TWO) {
return bridging::toJs(rt, 2);
} else {
throw jsi::JSError(rt, "No appropriate enum member found for enum value");
}
}
};
```
That code would allow us to use these enums in the cxx files like this:
```
NativeCxxModuleExampleCxxEnumInt getNumEnum(
jsi::Runtime &rt,
NativeCxxModuleExampleCxxEnumInt arg);
```
Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators
Reviewed By: christophpurrer
Differential Revision: D42884147
fbshipit-source-id: 83639022f59c18028ae16d8e2c491686e8867ab0
vzaidman
force-pushed
the
export-D42884147
branch
from
4274bd0 to
80aaff0
Compare
|
This pull request was exported from Phabricator. Differential Revision: D42884147 |
|
This pull request has been merged in ceb1d0d. |
vzaidman
changed the title
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Feb 14, 2023
…degen Summary: [Changelog][Internal] The PR facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules. This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen. This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members. Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required. Differential Revision: D43292254 fbshipit-source-id: 50066dc89c495ba3cad23a26024ac485d03dc9d9
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Feb 14, 2023
…ceObserver Summary: [Changelog][Internal] After D42884147 (facebook@ceb1d0d) (facebook#36030) had landed, we now can use enums as data types when bridging between JS and C++ TurboModules. This diff takes advantage of this and converts the API to use the proper enums instead of int types that it was forced to use earlier. NOTE: This also relies on additional change to codegen, D43292254, which is required in order for enums to work also as part of data structures. Differential Revision: D43292282 fbshipit-source-id: fd271704ab60aec2b3ae5b260e76665ad50e206e
facebook-github-bot
pushed a commit
that referenced
this pull request
Feb 15, 2023
…degen (#36155) Summary: Pull Request resolved: #36155 [Changelog][Internal] The PR #36030 (diff D42884147 (ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules. This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen. This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members. Some part of the change is the same as D42008724 (963e45a), but there are also some changes related to the types, that were required. Reviewed By: christophpurrer Differential Revision: D43292254 fbshipit-source-id: b2d6cf4a2d4d233b8cc403ecd02b5be16d5d91a7
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
…g/number in c++ turbo modules generators (facebook#36030) Summary: Pull Request resolved: facebook#36030 Generate enum types in c++ turbo modules. For enums in the ts schema file such as: ``` export enum NumEnum { ONE = 1, TWO = 2, } ``` This would export enums and the relevant Bridging to js and from js code to the spec H files such as: ``` #pragma mark - SampleTurboModuleCxxNumEnum enum SampleTurboModuleCxxNumEnum { ONE, TWO }; template <> struct Bridging<SampleTurboModuleCxxNumEnum> { static SampleTurboModuleCxxNumEnum fromJs(jsi::Runtime &rt, int32_t value) { if (value == 1) { return SampleTurboModuleCxxNumEnum::ONE; } else if (value == 2) { return SampleTurboModuleCxxNumEnum::TWO; } else { throw jsi::JSError(rt, "No appropriate enum member found for value"); } } static jsi::Value toJs(jsi::Runtime &rt, SampleTurboModuleCxxNumEnum value) { if (value == SampleTurboModuleCxxNumEnum::ONE) { return bridging::toJs(rt, 1); } else if (value == SampleTurboModuleCxxNumEnum::TWO) { return bridging::toJs(rt, 2); } else { throw jsi::JSError(rt, "No appropriate enum member found for enum value"); } } }; ``` That code would allow us to use these enums in the cxx files like this: ``` NativeCxxModuleExampleCxxEnumInt getNumEnum( jsi::Runtime &rt, NativeCxxModuleExampleCxxEnumInt arg); ``` Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators Reviewed By: christophpurrer Differential Revision: D42884147 fbshipit-source-id: d34d1fc7ba268b570821dc108444196f69a431b2
OlimpiaZurek
pushed a commit
to OlimpiaZurek/react-native
that referenced
this pull request
May 22, 2023
…degen (facebook#36155) Summary: Pull Request resolved: facebook#36155 [Changelog][Internal] The PR facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules. This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen. This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members. Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required. Reviewed By: christophpurrer Differential Revision: D43292254 fbshipit-source-id: b2d6cf4a2d4d233b8cc403ecd02b5be16d5d91a7
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators
For enums in the ts schema file such as:
This would export enums and the relevant Bridging to js and from js code to the spec H files such as:
That code would allow us to use these enums in the cxx files like this:
Changelog: [General] [Added] Generate enum types that would be allowed to be used as well as string/number in c++ turbo modules generators
Differential Revision: D42884147