From 80f85bb033d698872733bed9156f76741ded1188 Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 28 Aug 2024 18:14:22 -0700 Subject: [PATCH 1/2] Array's with unparsable element tyoe's are explicitly Any vs missing Summary: Previously the schema special cased unparseable elementType with elementType just being undefined. This causes issues for logic that requires recursively matching types. Instead of being implicit, this makes them explicitly an AnyTypeAnnotation Differential Revision: D61825742 --- .../src/CodegenSchema.d.ts | 102 +++++++++--------- .../react-native-codegen/src/CodegenSchema.js | 6 +- .../GenerateModuleObjCpp/StructCollector.js | 5 +- .../header/serializeConstantsStruct.js | 4 +- .../header/serializeRegularStruct.js | 4 +- .../GenerateModuleObjCpp/serializeMethod.js | 2 +- .../modules/__test_fixtures__/fixtures.js | 6 ++ .../parsers/__tests__/parsers-commons-test.js | 10 +- .../module-parser-snapshot-test.js.snap | 10 +- .../__tests__/module-parser-e2e-test.js | 31 ++++-- .../src/parsers/parsers-primitives.js | 3 + ...script-module-parser-snapshot-test.js.snap | 20 +++- .../typescript-module-parser-e2e-test.js | 31 ++++-- 13 files changed, 154 insertions(+), 80 deletions(-) diff --git a/packages/react-native-codegen/src/CodegenSchema.d.ts b/packages/react-native-codegen/src/CodegenSchema.d.ts index 6b06670f4b18..0bdf0e35f412 100644 --- a/packages/react-native-codegen/src/CodegenSchema.d.ts +++ b/packages/react-native-codegen/src/CodegenSchema.d.ts @@ -110,8 +110,8 @@ export interface ExtendsPropsShape { export interface EventTypeShape { readonly name: string; readonly bubblingType: - | 'direct' - | 'bubble'; + | 'direct' + | 'bubble'; readonly optional: boolean; readonly paperTopLevelNameDeprecated?: string | undefined; readonly typeAnnotation: { @@ -137,55 +137,55 @@ export type EventTypeAnnotation = export type ArrayTypeAnnotation = { readonly type: 'ArrayTypeAnnotation'; readonly elementType: - | BooleanTypeAnnotation - | StringTypeAnnotation - | DoubleTypeAnnotation - | FloatTypeAnnotation - | Int32TypeAnnotation - | { - readonly type: 'StringEnumTypeAnnotation'; - readonly default: string; - readonly options: readonly string[]; - } - | ObjectTypeAnnotation - | ReservedPropTypeAnnotation - | { - readonly type: 'ArrayTypeAnnotation'; - readonly elementType: ObjectTypeAnnotation; - }; + | BooleanTypeAnnotation + | StringTypeAnnotation + | DoubleTypeAnnotation + | FloatTypeAnnotation + | Int32TypeAnnotation + | { + readonly type: 'StringEnumTypeAnnotation'; + readonly default: string; + readonly options: readonly string[]; + } + | ObjectTypeAnnotation + | ReservedPropTypeAnnotation + | { + readonly type: 'ArrayTypeAnnotation'; + readonly elementType: ObjectTypeAnnotation; + }; } export type PropTypeAnnotation = | { - readonly type: 'BooleanTypeAnnotation'; - readonly default: boolean | null; - } + readonly type: 'BooleanTypeAnnotation'; + readonly default: boolean | null; + } | { - readonly type: 'StringTypeAnnotation'; - readonly default: string | null; - } + readonly type: 'StringTypeAnnotation'; + readonly default: string | null; + } | { - readonly type: 'DoubleTypeAnnotation'; - readonly default: number; - } + readonly type: 'DoubleTypeAnnotation'; + readonly default: number; + } | { - readonly type: 'FloatTypeAnnotation'; - readonly default: number | null; - } + readonly type: 'FloatTypeAnnotation'; + readonly default: number | null; + } | { - readonly type: 'Int32TypeAnnotation'; - readonly default: number; - } + readonly type: 'Int32TypeAnnotation'; + readonly default: number; + } | { - readonly type: 'StringEnumTypeAnnotation'; - readonly default: string; - readonly options: readonly string[]; - } + readonly type: 'StringEnumTypeAnnotation'; + readonly default: string; + readonly options: readonly string[]; + } | { - readonly type: 'Int32EnumTypeAnnotation'; - readonly default: number; - readonly options: readonly number[]; - } + readonly type: 'Int32EnumTypeAnnotation'; + readonly default: number; + readonly options: readonly number[]; + } | ReservedPropTypeAnnotation | ObjectTypeAnnotation | ArrayTypeAnnotation @@ -194,12 +194,12 @@ export type PropTypeAnnotation = export interface ReservedPropTypeAnnotation { readonly type: 'ReservedPropTypeAnnotation'; readonly name: - | 'ColorPrimitive' - | 'ImageSourcePrimitive' - | 'PointPrimitive' - | 'EdgeInsetsPrimitive' - | 'ImageRequestPrimitive' - | 'DimensionPrimitive'; + | 'ColorPrimitive' + | 'ImageSourcePrimitive' + | 'PointPrimitive' + | 'EdgeInsetsPrimitive' + | 'ImageRequestPrimitive' + | 'DimensionPrimitive'; } export type CommandTypeAnnotation = FunctionTypeAnnotation< @@ -279,7 +279,11 @@ export interface NativeModuleArrayTypeAnnotation; +export type UnsafeAnyTypeAnnotation = { + type: 'AnyTypeAnnotation', +}; + export type NativeModuleNumberTypeAnnotation = $ReadOnly<{ type: 'NumberTypeAnnotation', }>; diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/StructCollector.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/StructCollector.js index 8df8ec47dd68..69a59a453897 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/StructCollector.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/StructCollector.js @@ -93,9 +93,12 @@ class StructCollector { }); } case 'ArrayTypeAnnotation': { - if (typeAnnotation.elementType == null) { + if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') { return wrapNullable(nullable, { type: 'ArrayTypeAnnotation', + elementType: { + type: 'AnyTypeAnnotation', + }, }); } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeConstantsStruct.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeConstantsStruct.js index ad7918d686f8..8e8054c583e1 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeConstantsStruct.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeConstantsStruct.js @@ -118,7 +118,7 @@ function toObjCType( case 'GenericObjectTypeAnnotation': return wrapObjCOptional('id', isRequired); case 'ArrayTypeAnnotation': - if (typeAnnotation.elementType == null) { + if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') { return wrapObjCOptional('id', isRequired); } @@ -198,7 +198,7 @@ function toObjCValue( return value; case 'ArrayTypeAnnotation': const {elementType} = typeAnnotation; - if (elementType == null) { + if (elementType.type === 'AnyTypeAnnotation') { return value; } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeRegularStruct.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeRegularStruct.js index 9ea93a1da7a1..3e53c456572f 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeRegularStruct.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeRegularStruct.js @@ -109,7 +109,7 @@ function toObjCType( case 'GenericObjectTypeAnnotation': return wrapObjCOptional('id', isRequired); case 'ArrayTypeAnnotation': - if (typeAnnotation.elementType == null) { + if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') { return wrapObjCOptional('id', isRequired); } return wrapCxxOptional( @@ -188,7 +188,7 @@ function toObjCValue( return value; case 'ArrayTypeAnnotation': const {elementType} = typeAnnotation; - if (elementType == null) { + if (elementType.type === 'AnyTypeAnnotation') { return value; } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js index 14f0ed724c78..c06354ebc3e6 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js @@ -305,7 +305,7 @@ function getReturnObjCType( case 'TypeAliasTypeAnnotation': return wrapOptional('NSDictionary *', isRequired); case 'ArrayTypeAnnotation': - if (typeAnnotation.elementType == null) { + if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') { return wrapOptional('NSArray> *', isRequired); } 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 adbc596c38a8..f7a65fe0a562 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 @@ -947,6 +947,9 @@ const COMPLEX_OBJECTS: SchemaType = { type: 'NullableTypeAnnotation', typeAnnotation: { type: 'ArrayTypeAnnotation', + elementType: { + type: 'AnyTypeAnnotation', + }, }, }, params: [], @@ -1969,6 +1972,9 @@ const CXX_ONLY_NATIVE_MODULES: SchemaType = { type: 'FunctionTypeAnnotation', returnTypeAnnotation: { type: 'ArrayTypeAnnotation', + elementType: { + type: 'AnyTypeAnnotation', + }, }, params: [ { diff --git a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js index 408ca9a0bc30..5c2b37d2a044 100644 --- a/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js +++ b/packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js @@ -851,7 +851,10 @@ describe('buildSchema', () => { { name: 'a', optional: false, - typeAnnotation: {type: 'ArrayTypeAnnotation'}, + typeAnnotation: { + type: 'ArrayTypeAnnotation', + elementType: {type: 'AnyTypeAnnotation'}, + }, }, ], }, @@ -1258,7 +1261,10 @@ describe('buildModuleSchema', () => { { name: 'a', optional: false, - typeAnnotation: {type: 'ArrayTypeAnnotation'}, + typeAnnotation: { + type: 'ArrayTypeAnnotation', + elementType: {type: 'AnyTypeAnnotation'}, + }, }, ], returnTypeAnnotation: { 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 aa318c18c2c8..83e6f63fbd9e 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 @@ -798,14 +798,20 @@ exports[`RN Codegen Flow Parser can generate fixture NATIVE_MODULE_WITH_ARRAY_WI 'typeAnnotation': { 'type': 'FunctionTypeAnnotation', 'returnTypeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } }, 'params': [ { 'name': 'arg', 'optional': false, 'typeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } } } ] diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js index 0ac4e057decc..66b5dfc975db 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js @@ -229,8 +229,13 @@ describe('Flow Module Parser', () => { expect(paramTypeAnnotation.type).toBe('ArrayTypeAnnotation'); invariant(paramTypeAnnotation.type === 'ArrayTypeAnnotation', ''); - expect(paramTypeAnnotation.elementType).not.toBe(null); - invariant(paramTypeAnnotation.elementType != null, ''); + expect(paramTypeAnnotation.elementType.type).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + paramTypeAnnotation.elementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( paramTypeAnnotation.elementType, @@ -532,8 +537,13 @@ describe('Flow Module Parser', () => { const {elementType: nullableElementType} = property.typeAnnotation; - expect(nullableElementType).not.toBe(null); - invariant(nullableElementType != null, ''); + expect(nullableElementType.type).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + nullableElementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( @@ -802,8 +812,8 @@ describe('Flow Module Parser', () => { const arrayTypeAnnotation = returnTypeAnnotation; const {elementType} = arrayTypeAnnotation; - expect(elementType).not.toBe(null); - invariant(elementType != null, ''); + expect(elementType.type).not.toEqual('AnyTypeAnnotation'); + invariant(elementType.type !== 'AnyTypeAnnotation', ''); const [elementTypeAnnotation, isElementTypeAnnotation] = unwrapNullable(elementType); @@ -1074,8 +1084,13 @@ describe('Flow Module Parser', () => { const {elementType: nullableElementType} = property.typeAnnotation; - expect(nullableElementType).not.toBe(null); - invariant(nullableElementType != null, ''); + expect(nullableElementType.type).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + nullableElementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( diff --git a/packages/react-native-codegen/src/parsers/parsers-primitives.js b/packages/react-native-codegen/src/parsers/parsers-primitives.js index 8c05216f8416..5fead2c4acf4 100644 --- a/packages/react-native-codegen/src/parsers/parsers-primitives.js +++ b/packages/react-native-codegen/src/parsers/parsers-primitives.js @@ -466,6 +466,9 @@ function translateArrayTypeAnnotation( } catch (ex) { return wrapNullable(nullable, { type: 'ArrayTypeAnnotation', + elementType: { + type: 'AnyTypeAnnotation', + }, }); } } 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 55befcf241c2..02351a5ba78b 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 @@ -789,14 +789,20 @@ exports[`RN Codegen TypeScript Parser can generate fixture NATIVE_MODULE_WITH_AR 'typeAnnotation': { 'type': 'FunctionTypeAnnotation', 'returnTypeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } }, 'params': [ { 'name': 'arg', 'optional': false, 'typeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } } } ] @@ -869,14 +875,20 @@ exports[`RN Codegen TypeScript Parser can generate fixture NATIVE_MODULE_WITH_AR 'typeAnnotation': { 'type': 'FunctionTypeAnnotation', 'returnTypeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } }, 'params': [ { 'name': 'arg', 'optional': false, 'typeAnnotation': { - 'type': 'ArrayTypeAnnotation' + 'type': 'ArrayTypeAnnotation', + 'elementType': { + 'type': 'AnyTypeAnnotation' + } } } ] diff --git a/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/typescript-module-parser-e2e-test.js b/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/typescript-module-parser-e2e-test.js index 35862d8e7fe5..7b13f738d1f0 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/typescript-module-parser-e2e-test.js +++ b/packages/react-native-codegen/src/parsers/typescript/modules/__tests__/typescript-module-parser-e2e-test.js @@ -229,8 +229,13 @@ describe('TypeScript Module Parser', () => { expect(paramTypeAnnotation.type).toBe('ArrayTypeAnnotation'); invariant(paramTypeAnnotation.type === 'ArrayTypeAnnotation', ''); - expect(paramTypeAnnotation.elementType).not.toBe(null); - invariant(paramTypeAnnotation.elementType != null, ''); + expect(paramTypeAnnotation.elementType.type).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + paramTypeAnnotation.elementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( paramTypeAnnotation.elementType, @@ -532,9 +537,14 @@ describe('TypeScript Module Parser', () => { const {elementType: nullableElementType} = property.typeAnnotation; - expect(nullableElementType).not.toBe(null); - invariant(nullableElementType != null, ''); + expect(nullableElementType.type).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + nullableElementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( nullableElementType, @@ -800,8 +810,8 @@ describe('TypeScript Module Parser', () => { const arrayTypeAnnotation = returnTypeAnnotation; const {elementType} = arrayTypeAnnotation; - expect(elementType).not.toBe(null); - invariant(elementType != null, ''); + expect(elementType.type).not.toEqual('AnyTypeAnnotation'); + invariant(elementType.type !== 'AnyTypeAnnotation', ''); const [elementTypeAnnotation, isElementTypeAnnotation] = unwrapNullable(elementType); @@ -1073,8 +1083,13 @@ describe('TypeScript Module Parser', () => { const {elementType: nullableElementType} = property.typeAnnotation; - expect(nullableElementType).not.toBe(null); - invariant(nullableElementType != null, ''); + expect(nullableElementType).not.toEqual( + 'AnyTypeAnnotation', + ); + invariant( + nullableElementType.type !== 'AnyTypeAnnotation', + '', + ); const [elementType, isElementTypeNullable] = unwrapNullable( From b2d59f0c05a10899a21e83832c36b28e14ee9a29 Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 28 Aug 2024 18:18:50 -0700 Subject: [PATCH 2/2] Fix NativeModuleEnumMembers type (#46222) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46222 This value was typed as always being a string, even though it was containing both strings and numbers in the fixtures. This was because on this line https://fburl.com/code/9j7gh4av the input type is $FlowFixMe (from the source AST), which wasn't catching that it couldn't flow into just `string`. Changelog: [Internal] Reviewed By: makovkastar Differential Revision: D61830075 --- packages/react-native-codegen/src/CodegenSchema.js | 2 +- .../src/generators/modules/GenerateModuleH.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native-codegen/src/CodegenSchema.js b/packages/react-native-codegen/src/CodegenSchema.js index aa497f5f31b7..7752be23a4ec 100644 --- a/packages/react-native-codegen/src/CodegenSchema.js +++ b/packages/react-native-codegen/src/CodegenSchema.js @@ -297,7 +297,7 @@ export type NativeModuleNumberTypeAnnotation = $ReadOnly<{ export type NativeModuleEnumMembers = $ReadOnlyArray< $ReadOnly<{ name: string, - value: string, + value: string | number, }>, >; diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js index 783cf37f1c28..37fb00b2e54e 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js @@ -407,7 +407,7 @@ function generateEnum( const nativeEnumMemberType: NativeEnumMemberValueType = memberType === 'StringTypeAnnotation' ? 'std::string' : 'int32_t'; - const getMemberValueAppearance = (value: string) => + const getMemberValueAppearance = (value: string | number) => memberType === 'StringTypeAnnotation' ? `"${value}"` : `${value}`; const fromCases =