Skip to content

Commit e81c98c

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Fix Cpp codegen handling of optional arguments
Summary: Changelog: [General][Fixed] - Codegen for C++ TurboModules of optional method arguments was incorrect Reviewed By: christophpurrer Differential Revision: D40979066 fbshipit-source-id: 5bb48dbafc14dcea21b7e0b15e3f4bb527bc8476
1 parent f32a3a5 commit e81c98c

9 files changed

+70
-10
lines changed

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,10 @@ function serializeArg(
118118
index: number,
119119
resolveAlias: AliasResolver,
120120
): string {
121-
const {typeAnnotation: nullableTypeAnnotation} = arg;
121+
const {typeAnnotation: nullableTypeAnnotation, optional} = arg;
122122
const [typeAnnotation, nullable] =
123123
unwrapNullable<NativeModuleParamTypeAnnotation>(nullableTypeAnnotation);
124+
const isRequired = !optional && !nullable;
124125

125126
let realTypeAnnotation = typeAnnotation;
126127
if (realTypeAnnotation.type === 'TypeAliasTypeAnnotation') {
@@ -130,12 +131,15 @@ function serializeArg(
130131
function wrap(callback: (val: string) => string) {
131132
const val = `args[${index}]`;
132133
const expression = callback(val);
133-
134-
if (nullable) {
135-
return `${val}.isNull() || ${val}.isUndefined() ? std::nullopt : std::make_optional(${expression})`;
134+
if (isRequired) {
135+
return expression;
136+
} else {
137+
let condition = `${val}.isNull() || ${val}.isUndefined()`;
138+
if (optional) {
139+
condition = `count < ${index} || ${condition}`;
140+
}
141+
return `${condition} ? std::nullopt : std::make_optional(${expression})`;
136142
}
137-
138-
return expression;
139143
}
140144

141145
switch (realTypeAnnotation.type) {

‎packages/react-native-codegen/src/generators/modules/GenerateModuleH.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,22 @@ ${modules.join('\n\n')}
107107

108108
function translatePrimitiveJSTypeToCpp(
109109
nullableTypeAnnotation: Nullable<NativeModuleTypeAnnotation>,
110+
optional: boolean,
110111
createErrorMessage: (typeName: string) => string,
111112
resolveAlias: AliasResolver,
112113
) {
113114
const [typeAnnotation, nullable] = unwrapNullable<NativeModuleTypeAnnotation>(
114115
nullableTypeAnnotation,
115116
);
117+
const isRequired = !optional && !nullable;
116118

117119
let realTypeAnnotation = typeAnnotation;
118120
if (realTypeAnnotation.type === 'TypeAliasTypeAnnotation') {
119121
realTypeAnnotation = resolveAlias(realTypeAnnotation.name);
120122
}
121123

122124
function wrap(type: string) {
123-
return nullable ? `std::optional<${type}>` : type;
125+
return isRequired ? type : `std::optional<${type}>`;
124126
}
125127

126128
switch (realTypeAnnotation.type) {
@@ -199,6 +201,7 @@ function translatePropertyToCpp(
199201
const paramTypes = propTypeAnnotation.params.map(param => {
200202
const translatedParam = translatePrimitiveJSTypeToCpp(
201203
param.typeAnnotation,
204+
param.optional,
202205
typeName =>
203206
`Unsupported type for param "${param.name}" in ${prop.name}. Found: ${typeName}`,
204207
resolveAlias,
@@ -208,6 +211,7 @@ function translatePropertyToCpp(
208211

209212
const returnType = translatePrimitiveJSTypeToCpp(
210213
propTypeAnnotation.returnTypeAnnotation,
214+
false,
211215
typeName => `Unsupported return type for ${prop.name}. Found: ${typeName}`,
212216
resolveAlias,
213217
);

‎packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,25 @@ const SIMPLE_NATIVE_MODULES: SchemaType = {
275275
],
276276
},
277277
},
278+
{
279+
name: 'getValueWithOptionalArg',
280+
optional: false,
281+
typeAnnotation: {
282+
type: 'FunctionTypeAnnotation',
283+
returnTypeAnnotation: {
284+
type: 'PromiseTypeAnnotation',
285+
},
286+
params: [
287+
{
288+
optional: true,
289+
name: 'parameter',
290+
typeAnnotation: {
291+
type: 'GenericObjectTypeAnnotation',
292+
},
293+
},
294+
],
295+
},
296+
},
278297
{
279298
name: 'getEnums',
280299
optional: false,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_optionals(jsi
5454
return jsi::Value::undefined();
5555
}
5656
static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_optionalMethod(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
57-
static_cast<NativeSampleTurboModuleCxxSpecJSI *>(&turboModule)->optionalMethod(rt, args[0].asObject(rt), args[1].asObject(rt).asFunction(rt), args[2].asObject(rt).asArray(rt));
57+
static_cast<NativeSampleTurboModuleCxxSpecJSI *>(&turboModule)->optionalMethod(rt, args[0].asObject(rt), args[1].asObject(rt).asFunction(rt), count < 2 || args[2].isNull() || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt).asArray(rt)));
5858
return jsi::Value::undefined();
5959
}
6060
static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getArrays(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
@@ -337,6 +337,9 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithC
337337
static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithPromise(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
338338
return static_cast<NativeSampleTurboModuleCxxSpecJSI *>(&turboModule)->getValueWithPromise(rt, args[0].asBool());
339339
}
340+
static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithOptionalArg(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
341+
return static_cast<NativeSampleTurboModuleCxxSpecJSI *>(&turboModule)->getValueWithOptionalArg(rt, count < 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)));
342+
}
340343
static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getEnums(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {
341344
return static_cast<NativeSampleTurboModuleCxxSpecJSI *>(&turboModule)->getEnums(rt, args[0].asNumber(), args[1].asNumber(), args[2].asString(rt));
342345
}
@@ -354,6 +357,7 @@ NativeSampleTurboModuleCxxSpecJSI::NativeSampleTurboModuleCxxSpecJSI(std::shared
354357
methodMap_[\\"getValue\\"] = MethodMetadata {3, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValue};
355358
methodMap_[\\"getValueWithCallback\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithCallback};
356359
methodMap_[\\"getValueWithPromise\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithPromise};
360+
methodMap_[\\"getValueWithOptionalArg\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithOptionalArg};
357361
methodMap_[\\"getEnums\\"] = MethodMetadata {3, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getEnums};
358362
}
359363

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ protected:
8787
public:
8888
virtual jsi::Object difficult(jsi::Runtime &rt, jsi::Object A) = 0;
8989
virtual void optionals(jsi::Runtime &rt, jsi::Object A) = 0;
90-
virtual void optionalMethod(jsi::Runtime &rt, jsi::Object options, jsi::Function callback, jsi::Array extras) = 0;
90+
virtual void optionalMethod(jsi::Runtime &rt, jsi::Object options, jsi::Function callback, std::optional<jsi::Array> extras) = 0;
9191
virtual void getArrays(jsi::Runtime &rt, jsi::Object options) = 0;
9292
virtual std::optional<jsi::Object> getNullableObject(jsi::Runtime &rt) = 0;
9393
virtual std::optional<jsi::Object> getNullableGenericObject(jsi::Runtime &rt) = 0;
@@ -129,7 +129,7 @@ private:
129129
return bridging::callFromJs<void>(
130130
rt, &T::optionals, jsInvoker_, instance_, std::move(A));
131131
}
132-
void optionalMethod(jsi::Runtime &rt, jsi::Object options, jsi::Function callback, jsi::Array extras) override {
132+
void optionalMethod(jsi::Runtime &rt, jsi::Object options, jsi::Function callback, std::optional<jsi::Array> extras) override {
133133
static_assert(
134134
bridging::getParameterCount(&T::optionalMethod) == 4,
135135
\\"Expected optionalMethod(...) to have 4 parameters\\");
@@ -668,6 +668,7 @@ public:
668668
virtual jsi::Object getValue(jsi::Runtime &rt, double x, jsi::String y, jsi::Object z) = 0;
669669
virtual void getValueWithCallback(jsi::Runtime &rt, jsi::Function callback) = 0;
670670
virtual jsi::Value getValueWithPromise(jsi::Runtime &rt, bool error) = 0;
671+
virtual jsi::Value getValueWithOptionalArg(jsi::Runtime &rt, std::optional<jsi::Object> parameter) = 0;
671672
virtual jsi::String getEnums(jsi::Runtime &rt, double enumInt, double enumFloat, jsi::String enumString) = 0;
672673
673674
};
@@ -778,6 +779,14 @@ private:
778779
return bridging::callFromJs<jsi::Value>(
779780
rt, &T::getValueWithPromise, jsInvoker_, instance_, std::move(error));
780781
}
782+
jsi::Value getValueWithOptionalArg(jsi::Runtime &rt, std::optional<jsi::Object> parameter) override {
783+
static_assert(
784+
bridging::getParameterCount(&T::getValueWithOptionalArg) == 2,
785+
\\"Expected getValueWithOptionalArg(...) to have 2 parameters\\");
786+
787+
return bridging::callFromJs<jsi::Value>(
788+
rt, &T::getValueWithOptionalArg, jsInvoker_, instance_, std::move(parameter));
789+
}
781790
jsi::String getEnums(jsi::Runtime &rt, double enumInt, double enumFloat, jsi::String enumString) override {
782791
static_assert(
783792
bridging::getParameterCount(&T::getEnums) == 4,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,9 @@ namespace JS {
933933
- (void)getValueWithPromise:(BOOL)error
934934
resolve:(RCTPromiseResolveBlock)resolve
935935
reject:(RCTPromiseRejectBlock)reject;
936+
- (void)getValueWithOptionalArg:(NSDictionary *)parameter
937+
resolve:(RCTPromiseResolveBlock)resolve
938+
reject:(RCTPromiseRejectBlock)reject;
936939
- (NSString *)getEnums:(double)enumInt
937940
enumFloat:(double)enumFloat
938941
enumString:(NSString *)enumString;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,10 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo
377377
@DoNotStrip
378378
public abstract void getValueWithPromise(boolean error, Promise promise);
379379
380+
@ReactMethod
381+
@DoNotStrip
382+
public abstract void getValueWithOptionalArg(ReadableMap parameter, Promise promise);
383+
380384
@ReactMethod(isBlockingSynchronousMethod = true)
381385
@DoNotStrip
382386
public abstract String getEnums(double enumInt, double enumFloat, String enumString);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getVal
384384
return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, PromiseKind, \\"getValueWithPromise\\", \\"(ZLcom/facebook/react/bridge/Promise;)V\\", args, count, cachedMethodId);
385385
}
386386
387+
static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithOptionalArg(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
388+
static jmethodID cachedMethodId = nullptr;
389+
return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, PromiseKind, \\"getValueWithOptionalArg\\", \\"(Lcom/facebook/react/bridge/ReadableMap;Lcom/facebook/react/bridge/Promise;)V\\", args, count, cachedMethodId);
390+
}
391+
387392
static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getEnums(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
388393
static jmethodID cachedMethodId = nullptr;
389394
return static_cast<JavaTurboModule &>(turboModule).invokeJavaMethod(rt, StringKind, \\"getEnums\\", \\"(DDLjava/lang/String;)Ljava/lang/String;\\", args, count, cachedMethodId);
@@ -402,6 +407,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const JavaTurboMo
402407
methodMap_[\\"getValue\\"] = MethodMetadata {3, __hostFunction_NativeSampleTurboModuleSpecJSI_getValue};
403408
methodMap_[\\"getValueWithCallback\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithCallback};
404409
methodMap_[\\"getValueWithPromise\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithPromise};
410+
methodMap_[\\"getValueWithOptionalArg\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithOptionalArg};
405411
methodMap_[\\"getEnums\\"] = MethodMetadata {3, __hostFunction_NativeSampleTurboModuleSpecJSI_getEnums};
406412
}
407413

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,10 @@ namespace facebook {
453453
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, \\"getValueWithPromise\\", @selector(getValueWithPromise:resolve:reject:), args, count);
454454
}
455455
456+
static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithOptionalArg(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
457+
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, PromiseKind, \\"getValueWithOptionalArg\\", @selector(getValueWithOptionalArg:resolve:reject:), args, count);
458+
}
459+
456460
static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getEnums(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
457461
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, StringKind, \\"getEnums\\", @selector(getEnums:enumFloat:enumString:), args, count);
458462
}
@@ -494,6 +498,9 @@ namespace facebook {
494498
methodMap_[\\"getValueWithPromise\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithPromise};
495499
496500
501+
methodMap_[\\"getValueWithOptionalArg\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithOptionalArg};
502+
503+
497504
methodMap_[\\"getEnums\\"] = MethodMetadata {3, __hostFunction_NativeSampleTurboModuleSpecJSI_getEnums};
498505
499506

0 commit comments

Comments
 (0)