From c006722e6cdbe02711cb50ea7a739e0d4d81c7e7 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 24 May 2022 19:35:29 -0700 Subject: [PATCH] Fix nullability lost on readonly types Summary: This fixes an issue where nullability was lost on readonly types in TurboModules specs. The "outside" nullability would be ignored in favor of only respecting the "inside" nullability (i.e. `?$ReadOnly<{}>` vs `$ReadOnly`). It now ensure either is propagated correctly. Changelog: [General][Fixed] Fix nullability lost on readonly types in TurboModule specs Reviewed By: RSNara Differential Revision: D36615346 fbshipit-source-id: 42408d9298703f4c382657f573480d2574d6c973 --- .../modules/__test_fixtures__/fixtures.js | 41 +++++++++++- .../GenerateModuleCpp-test.js.snap | 4 ++ .../GenerateModuleH-test.js.snap | 9 +++ .../modules/__test_fixtures__/fixtures.js | 6 +- .../module-parser-snapshot-test.js.snap | 66 +++++++++++++++++++ .../src/parsers/flow/modules/index.js | 18 +++-- .../modules/__test_fixtures__/fixtures.js | 4 ++ ...script-module-parser-snapshot-test.js.snap | 66 +++++++++++++++++++ .../src/parsers/typescript/modules/index.js | 18 +++-- 9 files changed, 216 insertions(+), 16 deletions(-) diff --git a/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js index 5c11544020f8ec..16f6bd517ba44d 100644 --- a/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js @@ -1506,7 +1506,20 @@ const CXX_ONLY_NATIVE_MODULES: SchemaType = { modules: { NativeSampleTurboModule: { type: 'NativeModule', - aliases: {}, + aliases: { + ObjectAlias: { + type: 'ObjectTypeAnnotation', + properties: [ + { + name: 'x', + optional: false, + typeAnnotation: { + type: 'NumberTypeAnnotation', + }, + }, + ], + }, + }, spec: { properties: [ { @@ -1528,6 +1541,32 @@ const CXX_ONLY_NATIVE_MODULES: SchemaType = { ], }, }, + { + name: 'getNullableNumberFromNullableAlias', + optional: false, + typeAnnotation: { + type: 'FunctionTypeAnnotation', + returnTypeAnnotation: { + type: 'NullableTypeAnnotation', + typeAnnotation: { + type: 'NumberTypeAnnotation', + }, + }, + params: [ + { + name: 'a', + optional: false, + typeAnnotation: { + type: 'NullableTypeAnnotation', + typeAnnotation: { + type: 'TypeAliasTypeAnnotation', + name: 'ObjectAlias', + }, + }, + }, + ], + }, + }, ], }, moduleNames: ['SampleTurboModuleCxx'], 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 34baec11bd61e3..02017d09bf4e6f 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 @@ -108,10 +108,14 @@ namespace react { static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getMixed(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { return static_cast(&turboModule)->getMixed(rt, jsi::Value(rt, args[0])); } +static jsi::Value __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getNullableNumberFromNullableAlias(jsi::Runtime &rt, TurboModule &turboModule, const jsi::Value* args, size_t count) { + return static_cast(&turboModule)->getNullableNumberFromNullableAlias(rt, args[0].isNull() || args[0].isUndefined() ? std::nullopt : std::make_optional(args[0].asObject(rt))); +} NativeSampleTurboModuleCxxSpecJSI::NativeSampleTurboModuleCxxSpecJSI(std::shared_ptr jsInvoker) : TurboModule(\\"SampleTurboModuleCxx\\", jsInvoker) { methodMap_[\\"getMixed\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getMixed}; + methodMap_[\\"getNullableNumberFromNullableAlias\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleCxxSpecJSI_getNullableNumberFromNullableAlias}; } diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap index 64ce2ccd861741..7fce7ea931084a 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleH-test.js.snap @@ -208,6 +208,7 @@ protected: public: virtual jsi::Value getMixed(jsi::Runtime &rt, jsi::Value arg) = 0; + virtual std::optional getNullableNumberFromNullableAlias(jsi::Runtime &rt, std::optional a) = 0; }; @@ -237,6 +238,14 @@ private: return bridging::callFromJs( rt, &T::getMixed, jsInvoker_, instance_, std::move(arg)); } + std::optional getNullableNumberFromNullableAlias(jsi::Runtime &rt, std::optional a) override { + static_assert( + bridging::getParameterCount(&T::getNullableNumberFromNullableAlias) == 2, + \\"Expected getNullableNumberFromNullableAlias(...) to have 2 parameters\\"); + + return bridging::callFromJs>( + rt, &T::getNullableNumberFromNullableAlias, jsInvoker_, instance_, std::move(a)); + } private: T *instance_; diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js index 1583f3f2daa051..cbbc156c29f97c 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js @@ -161,7 +161,8 @@ export type ObjectAlias = {| y: number, label: string, truthy: boolean, -|} +|}; +export type ReadOnlyAlias = $ReadOnly; export interface Spec extends TurboModule { // Exported methods. @@ -169,6 +170,9 @@ export interface Spec extends TurboModule { +getVoid: () => Void; +getArray: (a: Array) => {| a: B |}; +getStringFromAlias: (a: ObjectAlias) => string; + +getStringFromNullableAlias: (a: ?ObjectAlias) => string; + +getStringFromReadOnlyAlias: (a: ReadOnlyAlias) => string; + +getStringFromNullableReadOnlyAlias: (a: ?ReadOnlyAlias) => string; } export default TurboModuleRegistry.getEnforcing('SampleTurboModule'); diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap index 66a8b5f4a87fa5..aef18a0ef6c783 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap +++ b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap @@ -251,6 +251,72 @@ exports[`RN Codegen Flow Parser can generate fixture NATIVE_MODULE_WITH_ALIASES } ] } + }, + { + 'name': 'getStringFromNullableAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'NullableTypeAnnotation', + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + } + ] + } + }, + { + 'name': 'getStringFromReadOnlyAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + ] + } + }, + { + 'name': 'getStringFromNullableReadOnlyAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'NullableTypeAnnotation', + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + } + ] + } } ] }, diff --git a/packages/react-native-codegen/src/parsers/flow/modules/index.js b/packages/react-native-codegen/src/parsers/flow/modules/index.js index 307ffb01854ee4..10153679a65767 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/index.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/index.js @@ -173,14 +173,18 @@ function translateTypeAnnotation( typeAnnotation, ); - return translateTypeAnnotation( - hasteModuleName, - typeAnnotation.typeParameters.params[0], - types, - aliasMap, - tryParse, - cxxOnly, + const [paramType, isParamNullable] = unwrapNullable( + translateTypeAnnotation( + hasteModuleName, + typeAnnotation.typeParameters.params[0], + types, + aliasMap, + tryParse, + cxxOnly, + ), ); + + return wrapNullable(nullable || isParamNullable, paramType); } case 'Stringish': { return wrapNullable(nullable, { diff --git a/packages/react-native-codegen/src/parsers/typescript/modules/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/parsers/typescript/modules/__test_fixtures__/fixtures.js index 665e33e0b0eebd..2d466d293d78b1 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/parsers/typescript/modules/__test_fixtures__/fixtures.js @@ -146,6 +146,7 @@ export type ObjectAlias = { label: string; truthy: boolean; }; +export type ReadOnlyAlias = Readonly; export interface Spec extends TurboModule { // Exported methods. @@ -153,6 +154,9 @@ export interface Spec extends TurboModule { readonly getVoid: () => Void; readonly getArray: (a: Array) => {a: B}; readonly getStringFromAlias: (a: ObjectAlias) => string; + readonly getStringFromNullableAlias: (a: ObjectAlias | null) => string; + readonly getStringFromReadOnlyAlias: (a: ReadOnlyAlias) => string; + readonly getStringFromNullableReadOnlyAlias: (a: ReadOnlyAlias | null) => string; } export default TurboModuleRegistry.getEnforcing('SampleTurboModule'); diff --git a/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/__snapshots__/typescript-module-parser-snapshot-test.js.snap b/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/__snapshots__/typescript-module-parser-snapshot-test.js.snap index faafd1d1334b19..8a49cb12a3d0e2 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/__snapshots__/typescript-module-parser-snapshot-test.js.snap +++ b/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/__snapshots__/typescript-module-parser-snapshot-test.js.snap @@ -249,6 +249,72 @@ exports[`RN Codegen TypeScript Parser can generate fixture NATIVE_MODULE_WITH_AL } ] } + }, + { + 'name': 'getStringFromNullableAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'NullableTypeAnnotation', + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + } + ] + } + }, + { + 'name': 'getStringFromReadOnlyAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + ] + } + }, + { + 'name': 'getStringFromNullableReadOnlyAlias', + 'optional': false, + 'typeAnnotation': { + 'type': 'FunctionTypeAnnotation', + 'returnTypeAnnotation': { + 'type': 'StringTypeAnnotation' + }, + 'params': [ + { + 'name': 'a', + 'optional': false, + 'typeAnnotation': { + 'type': 'NullableTypeAnnotation', + 'typeAnnotation': { + 'type': 'TypeAliasTypeAnnotation', + 'name': 'ObjectAlias' + } + } + } + ] + } } ] }, diff --git a/packages/react-native-codegen/src/parsers/typescript/modules/index.js b/packages/react-native-codegen/src/parsers/typescript/modules/index.js index 96ff34e5a549e3..66041f980dfa91 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/index.js +++ b/packages/react-native-codegen/src/parsers/typescript/modules/index.js @@ -173,14 +173,18 @@ function translateTypeAnnotation( typeAnnotation, ); - return translateTypeAnnotation( - hasteModuleName, - typeAnnotation.typeParameters.params[0], - types, - aliasMap, - tryParse, - cxxOnly, + const [paramType, isParamNullable] = unwrapNullable( + translateTypeAnnotation( + hasteModuleName, + typeAnnotation.typeParameters.params[0], + types, + aliasMap, + tryParse, + cxxOnly, + ), ); + + return wrapNullable(nullable || isParamNullable, paramType); } case 'Stringish': { return wrapNullable(nullable, {