Skip to content

Commit 0a8164d

Browse files
javachefacebook-github-bot
authored andcommitted
Fix off-by-one error in cxx codegen (facebook#36574)
Summary: Pull Request resolved: facebook#36574 We would previously generate the following codegen for optional args ``` return static_cast<NativeAudioModuleCxxSpecJSI *>(&turboModule)->playAudio( rt, args[0].asString(rt), args[1].isNull() || args[1].isUndefined() ? std::nullopt : std::make_optional(args[1].asString(rt)), args[2].isNull() || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asString(rt)), args[3].asNumber(), count < 4 || args[4].isNull() || args[4].isUndefined() ? std::nullopt : std::make_optional(args[4].asObject(rt)), count < 5 || args[5].isNull() || args[5].isUndefined() ? std::nullopt : std::make_optional(args[5].asObject(rt)), count < 6 || args[6].isNull() || args[6].isUndefined() ? std::nullopt : std::make_optional(args[6].asBool()) ); ``` However, the counts checked are off-by-one, causing us to incorrectly process args. Changelog: [General][Fixed] Issue with TurboModule C++ codegen with optional args Differential Revision: D44299193 fbshipit-source-id: f00b9f5e09c2f524f9393137346c256d8b6b2979
1 parent 4a15f90 commit 0a8164d

File tree

7 files changed

+981
-243
lines changed

7 files changed

+981
-243
lines changed

packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap

Lines changed: 800 additions & 200 deletions
Large diffs are not rendered by default.

packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ const HostFunctionTemplate = ({
4040
}>) => {
4141
const isNullable = returnTypeAnnotation.type === 'NullableTypeAnnotation';
4242
const isVoid = returnTypeAnnotation.type === 'VoidTypeAnnotation';
43-
const methodCallArgs = ['rt', ...args].join(', ');
44-
const methodCall = `static_cast<${hasteModuleName}CxxSpecJSI *>(&turboModule)->${methodName}(${methodCallArgs})`;
43+
const methodCallArgs = [' rt', ...args].join(',\n ');
44+
const methodCall = `static_cast<${hasteModuleName}CxxSpecJSI *>(&turboModule)->${methodName}(\n${methodCallArgs}\n )`;
4545

4646
return `static jsi::Value __hostFunction_${hasteModuleName}CxxSpecJSI_${methodName}(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {${
4747
isVoid
@@ -139,7 +139,7 @@ function serializeArg(
139139
} else {
140140
let condition = `${val}.isNull() || ${val}.isUndefined()`;
141141
if (optional) {
142-
condition = `count < ${index} || ${condition}`;
142+
condition = `count <= ${index} || ${condition}`;
143143
}
144144
return `${condition} ? std::nullopt : std::make_optional(${expression})`;
145145
}

packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleCpp-test.js.snap

Lines changed: 165 additions & 40 deletions
Large diffs are not rendered by default.

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,12 @@ AsyncPromise<std::string> NativeCxxModuleExample::getValueWithPromise(
117117
return promise;
118118
}
119119

120+
bool NativeCxxModuleExample::getWithWithOptionalArgs(
121+
jsi::Runtime &rt,
122+
std::optional<bool> optionalArg) {
123+
return optionalArg.value_or(false);
124+
}
125+
120126
void NativeCxxModuleExample::voidFunc(jsi::Runtime &rt) {
121127
// Nothing to do
122128
}

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ class NativeCxxModuleExample
120120

121121
AsyncPromise<std::string> getValueWithPromise(jsi::Runtime &rt, bool error);
122122

123+
bool getWithWithOptionalArgs(
124+
jsi::Runtime &rt,
125+
std::optional<bool> optionalArg);
126+
123127
void voidFunc(jsi::Runtime &rt);
124128

125129
void emitCustomDeviceEvent(jsi::Runtime &rt, jsi::String eventName);

packages/rn-tester/NativeCxxModuleExample/NativeCxxModuleExample.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export interface Spec extends TurboModule {
7070
+getValue: (x: number, y: string, z: ObjectStruct) => ValueStruct;
7171
+getValueWithCallback: (callback: (value: string) => void) => void;
7272
+getValueWithPromise: (error: boolean) => Promise<string>;
73+
+getWithWithOptionalArgs: (optionalArg?: boolean) => boolean;
7374
+voidFunc: () => void;
7475
+emitCustomDeviceEvent: (eventName: string) => void;
7576
}

packages/rn-tester/js/examples/TurboModule/NativeCxxModuleExampleExample.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type Examples =
5555
| 'promise'
5656
| 'rejectPromise'
5757
| 'voidFunc'
58+
| 'optionalArgs'
5859
| 'emitDeviceEvent';
5960

6061
class NativeCxxModuleExampleExample extends React.Component<{||}, State> {
@@ -99,6 +100,7 @@ class NativeCxxModuleExampleExample extends React.Component<{||}, State> {
99100
.then(() => {})
100101
.catch(e => this._setResult('rejectPromise', e.message)),
101102
voidFunc: () => NativeCxxModuleExample?.voidFunc(),
103+
optionalArgs: () => NativeCxxModuleExample?.getWithWithOptionalArgs(),
102104
emitDeviceEvent: () => {
103105
const CUSTOM_EVENT_TYPE = 'myCustomDeviceEvent';
104106
DeviceEventEmitter.removeAllListeners(CUSTOM_EVENT_TYPE);

0 commit comments

Comments
 (0)