From 67b9628af588e8fc778d732fc387dbd48acf705e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 29 Feb 2024 13:12:49 -0800 Subject: [PATCH] Modify native module codegen to throw JS errors when passing null values for non-nullable arguments Summary: Changelog: [General][Breaking] Native modules using the codegen now throw an error when called with `null` for optional but not nullable arguments. ## Context Right now, if you have a native module using the codegen with a method like this: ``` someMethod(value?: number): void; ``` And you call it like this: ``` NativeModule.someMethod(null); ``` The app doesn't throw an error, but it should because this method shouldn't accept `null` according to its type definition. ## Changes This modifies the codegen to only check for `undefined` in those cases, otherwise trying to cast the value to the expected type and failing if it's `null`. NOTE: this is technically a breaking change, but if people are using Flow or TypeScript in their projects they're very unlikely to hit this case, because they would've complained if you tried to pass `null` in these cases. Reviewed By: cipolleschi Differential Revision: D54206289 fbshipit-source-id: 58f2f2f3009d203b96189d3c66d1ae98a9e4fb36 --- .../GenerateModuleCpp-test.js.snap | 56 +++++++++---------- .../generators/modules/GenerateModuleCpp.js | 6 ++ .../GenerateModuleCpp-test.js.snap | 6 +- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap b/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap index 1fbcaf5a6e5..3130ea41984 100644 --- a/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap +++ b/packages/react-native-codegen/e2e/__tests__/modules/__snapshots__/GenerateModuleCpp-test.js.snap @@ -643,7 +643,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSp static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSpecJSI_getObjectShape(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObjectShape( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSpecJSI_getAlias(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { @@ -714,70 +714,70 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_voidF static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getBool(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getBool( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getNumber(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getNumber( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getString(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getString( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asString(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asString(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getArray(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getArray( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asArray(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asArray(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getObject(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObject( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getObjectShape(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObjectShape( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getAlias(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getAlias( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getRootTag(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getRootTag( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValue(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getValue( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()), - count <= 1 || args[1].isNull() || args[1].isUndefined() ? std::nullopt : std::make_optional(args[1].asString(rt)), - count <= 2 || args[2].isNull() || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()), + count <= 1 || args[1].isUndefined() ? std::nullopt : std::make_optional(args[1].asString(rt)), + count <= 2 || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValueWithCallback(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { static_cast(&turboModule)->getValueWithCallback( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asFunction(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asFunction(rt)) ); return jsi::Value::undefined(); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValueWithPromise(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getValueWithPromise( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) ); } @@ -1464,7 +1464,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSp static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSpecJSI_getObjectShape(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObjectShape( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleNullableAndOptionalCxxSpecJSI_getAlias(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { @@ -1535,70 +1535,70 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_voidF static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getBool(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getBool( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getNumber(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getNumber( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getString(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getString( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asString(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asString(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getArray(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getArray( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asArray(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asArray(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getObject(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObject( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getObjectShape(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getObjectShape( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getAlias(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getAlias( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getRootTag(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getRootTag( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValue(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getValue( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()), - count <= 1 || args[1].isNull() || args[1].isUndefined() ? std::nullopt : std::make_optional(args[1].asString(rt)), - count <= 2 || args[2].isNull() || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asNumber()), + count <= 1 || args[1].isUndefined() ? std::nullopt : std::make_optional(args[1].asString(rt)), + count <= 2 || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValueWithCallback(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { static_cast(&turboModule)->getValueWithCallback( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asFunction(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt).asFunction(rt)) ); return jsi::Value::undefined(); } static jsi::Value __hostFunction_NativeSampleTurboModuleOptionalCxxSpecJSI_getValueWithPromise(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getValueWithPromise( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) ); } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js index 2fa915d1943..8928c0e8566 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js @@ -132,6 +132,12 @@ function serializeArg( const val = `args[${index}]`; const expression = callback(val); + // param?: T + if (optional && !nullable) { + // throw new Error('are we hitting this case? ' + moduleName); + return `count <= ${index} || ${val}.isUndefined() ? std::nullopt : std::make_optional(${expression})`; + } + // param: ?T // param?: ?T if (nullable || optional) { diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleCpp-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleCpp-test.js.snap index 7be9050fc36..dcfbcaa3e16 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleCpp-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleCpp-test.js.snap @@ -61,7 +61,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_optionalMetho rt, count <= 0 ? throw jsi::JSError(rt, \\"Expected argument in position 0 to be passed\\") : args[0].asObject(rt), count <= 1 ? throw jsi::JSError(rt, \\"Expected argument in position 1 to be passed\\") : 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)) + count <= 2 || args[2].isUndefined() ? std::nullopt : std::make_optional(args[2].asObject(rt).asArray(rt)) ); return jsi::Value::undefined(); } @@ -243,7 +243,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithP static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getWithWithOptionalArgs(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { auto result = static_cast(&turboModule)->getWithWithOptionalArgs( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asBool()) ); return result ? jsi::Value(std::move(*result)) : jsi::Value::null(); } @@ -591,7 +591,7 @@ static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithP static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getValueWithOptionalArg(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getValueWithOptionalArg( rt, - count <= 0 || args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) + count <= 0 || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt)) ); } static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getEnums(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) {