From 572e1da889f2db46360c8673a6b581950384240e Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Tue, 29 Sep 2020 14:33:06 -0700 Subject: [PATCH] Fix type alias nullability resolution in module parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Consider this case: ``` type Animal = ?{| name: string, |}; type B = Animal export interface Spec extends TurboModule { +greet: (animal: B) => void; } ``` The generated output for this spec is: ``` namespace JS { namespace NativeSampleTurboModule { struct Animal { NSString *name() const; Animal(NSDictionary *const v) : _v(v) {} private: NSDictionary *_v; }; } } protocol NativeSampleTurboModuleSpec - (void)greet:(JS::NativeSampleTurboModule::Animal &)animal; end ``` Observations: 1. The codegen looks as though we wrote `+greet: (animal: ?Animal) => void;` as opposed to `+greet: (animal: B) => void;` 2. The generated struct is called `Animal`, not `B`. ## After this diff Whenever we detect a usage of a type alias, we recursively resolve it, keeping a track of whether the resolution will be nullable. In this example, we follow B to Animal, and then Animal to ?{|name: string|}. Then, we: 1. Replace the `B` in `+greet: (animal: B) => void;` with `?Animal`, 2. Pretend that `Animal = {|name: string|}`. Why do we make all type alias RHSs required? 2. This design is simpler than managing nullability in both the type alias usage, and the type alias RHS. 3. What does it mean for a C++ struct, which is what this type alias RHS will generate, to be nullable? ¯\_(ツ)_/¯. Nullability is a concept that only makes sense when talking about instances (i.e: usages) of the C++ structs. Hence, it's better to manage nullability within the actual TypeAliasTypeAnnotation nodes, and not the associated ObjectTypeAnnotations. ## Other Changes - Whenever we use the `Animal` type-alias, the e2e jest tests validate that the type alias exists in the module schema. Changelog: [Internal] Reviewed By: PeteTheHeat Differential Revision: D23225934 fbshipit-source-id: 8316dea2ec6e2d50cad90e178963c6264044f7b7 --- .../react-native-codegen/src/CodegenSchema.js | 20 +- .../__tests__/module-parser-e2e-test.js | 243 +++++++++++++----- .../src/parsers/flow/modules/index.js | 75 ++++-- .../src/parsers/flow/utils.js | 23 +- 4 files changed, 258 insertions(+), 103 deletions(-) diff --git a/packages/react-native-codegen/src/CodegenSchema.js b/packages/react-native-codegen/src/CodegenSchema.js index 425662967a2..f0d8d30d14d 100644 --- a/packages/react-native-codegen/src/CodegenSchema.js +++ b/packages/react-native-codegen/src/CodegenSchema.js @@ -266,7 +266,11 @@ export type NativeModuleSchema = $ReadOnly<{| |}>; export type NativeModuleAliasMap = { - [aliasName: string]: NativeModuleObjectTypeAnnotation, + [aliasName: string]: $ReadOnly<{| + type: 'ObjectTypeAnnotation', + properties: $ReadOnlyArray, + nullable: false, + |}>, }; export type NativeModulePropertySchema = $ReadOnly<{| @@ -290,16 +294,16 @@ export type NativeModuleFunctionTypeAnnotation = $ReadOnly<{| export type NativeModuleObjectTypeAnnotation = $ReadOnly<{| type: 'ObjectTypeAnnotation', - properties: $ReadOnlyArray< - $ReadOnly<{| - optional: boolean, - name: string, - typeAnnotation: NativeModuleParamTypeAnnotation, - |}>, - >, + properties: $ReadOnlyArray, nullable: boolean, |}>; +export type NativeModuleObjectTypeAnnotationPropertySchema = $ReadOnly<{| + optional: boolean, + name: string, + typeAnnotation: NativeModuleBaseTypeAnnotation, +|}>; + export type NativeModuleBaseTypeAnnotation = | $ReadOnly<{| type: 'StringTypeAnnotation', 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 734e456922f..e4647258fe3 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 @@ -8,7 +8,10 @@ * @format */ -import type {ReservedFunctionValueTypeName} from '../../../../CodegenSchema'; +import type { + ReservedFunctionValueTypeName, + NativeModuleSchema, +} from '../../../../CodegenSchema'; const {parseString} = require('../../index.js'); const { FlowGenericNotTypeParameterizedParserError, @@ -41,6 +44,30 @@ const RESERVED_FUNCTION_VALUE_TYPE_NAME: $ReadOnlyArray { describe('Parameter Parsing', () => { it("should fail parsing when a method has an parameter of type 'any'", () => { @@ -111,9 +138,7 @@ describe('Flow Module Parser', () => { import type {TurboModule} from 'RCTExport'; import * as TurboModuleRegistry from 'TurboModuleRegistry'; - type Animal = {| - name: string, - |}; + ${TYPE_ALIAS_DECLARATIONS} export interface Spec extends TurboModule { +useArg(${annotateArg(paramName, paramType)}): void; @@ -128,7 +153,7 @@ describe('Flow Module Parser', () => { expect(param.optional).toBe(optional); expect(param.typeAnnotation.nullable).toBe(nullable); - return param; + return [param, module]; } describe( @@ -149,14 +174,14 @@ describe('Flow Module Parser', () => { describe('Primitive types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} primitive parameter of type '${FLOW_TYPE}'`, () => { - const param = parseParamType('arg', FLOW_TYPE); + const [param] = parseParamType('arg', FLOW_TYPE); expect(param.typeAnnotation.type).toBe(PARSED_TYPE_NAME); }); }); }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Object'`, () => { - const param = parseParamType('arg', 'Object'); + const [param] = parseParamType('arg', 'Object'); expect(param.typeAnnotation.type).toBe( 'GenericObjectTypeAnnotation', ); @@ -165,7 +190,7 @@ describe('Flow Module Parser', () => { describe('Reserved Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of reserved type '${FLOW_TYPE}'`, () => { - const param = parseParamType('arg', FLOW_TYPE); + const [param] = parseParamType('arg', FLOW_TYPE); expect(param.typeAnnotation.type).toBe( 'ReservedFunctionValueTypeAnnotation', @@ -195,7 +220,10 @@ describe('Flow Module Parser', () => { paramName: string, paramType: string, ) { - const param = parseParamType(paramName, `Array<${paramType}>`); + const [param, module] = parseParamType( + paramName, + `Array<${paramType}>`, + ); expect(param.typeAnnotation.type).toBe('ArrayTypeAnnotation'); invariant( @@ -205,7 +233,7 @@ describe('Flow Module Parser', () => { expect(param.typeAnnotation.elementType).not.toBe(null); invariant(param.typeAnnotation.elementType != null, ''); - return param.typeAnnotation.elementType; + return [param.typeAnnotation.elementType, module]; } // TODO: Do we support nullable element types? @@ -213,7 +241,7 @@ describe('Flow Module Parser', () => { describe('Primitive Element Types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseParamArrayElementType( + const [elementType] = parseParamArrayElementType( 'arg', FLOW_TYPE, ); @@ -225,7 +253,7 @@ describe('Flow Module Parser', () => { describe('Reserved Element Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseParamArrayElementType( + const [elementType] = parseParamArrayElementType( 'arg', FLOW_TYPE, ); @@ -243,20 +271,24 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array'`, () => { - const elementType = parseParamArrayElementType('arg', 'Object'); + const [elementType] = parseParamArrayElementType('arg', 'Object'); expect(elementType.type).toBe('GenericObjectTypeAnnotation'); }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of some array of an alias`, () => { - const elementType = parseParamArrayElementType('arg', 'Animal'); + const [elementType, module] = parseParamArrayElementType( + 'arg', + 'Animal', + ); expect(elementType.type).toBe('TypeAliasTypeAnnotation'); invariant(elementType.type === 'TypeAliasTypeAnnotation', ''); expect(elementType.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter of type 'Array<{|foo: ?string|}>'`, () => { - const elementType = parseParamArrayElementType( + const [elementType] = parseParamArrayElementType( 'arg', '{|foo: ?string|}', ); @@ -281,7 +313,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of some type alias`, () => { - const param = parseParamType('arg', 'Animal'); + const [param, module] = parseParamType('arg', 'Animal'); expect(param.typeAnnotation.type).toBe('TypeAliasTypeAnnotation'); invariant( param.typeAnnotation.type === 'TypeAliasTypeAnnotation', @@ -289,6 +321,54 @@ describe('Flow Module Parser', () => { ); expect(param.typeAnnotation.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); + }); + + it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of some type alias that points to another type alias`, () => { + const [param, module] = parseParamType('arg', 'AnimalPointer'); + expect(param.typeAnnotation.type).toBe('TypeAliasTypeAnnotation'); + invariant( + param.typeAnnotation.type === 'TypeAliasTypeAnnotation', + '', + ); + + expect(param.typeAnnotation.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); + }); + + it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of some type alias that points to another nullable type alias`, () => { + const module = parseModule(` + import type {TurboModule} from 'RCTExport'; + import * as TurboModuleRegistry from 'TurboModuleRegistry'; + + type Animal = ?{| + name: string, + |}; + + type AnimalPointer = Animal; + + export interface Spec extends TurboModule { + +useArg(${annotateArg('arg', 'AnimalPointer')}): void; + } + export default TurboModuleRegistry.get('Foo'); + `); + + expect(module.properties[0]).not.toBe(null); + const param = module.properties[0].typeAnnotation.params[0]; + expect(param.name).toBe('arg'); + expect(param.optional).toBe(optional); + + // The TypeAliasAnnotation is called Animal, and is nullable + expect(param.typeAnnotation.type).toBe('TypeAliasTypeAnnotation'); + invariant( + param.typeAnnotation.type === 'TypeAliasTypeAnnotation', + '', + ); + expect(param.typeAnnotation.name).toBe('Animal'); + expect(param.typeAnnotation.nullable).toBe(true); + + // The Animal type alias RHS is valid, and non-null + expectAnimalTypeAliasToExist(module); }); [ @@ -323,7 +403,7 @@ describe('Flow Module Parser', () => { propName: string, propType: string, ) { - const param = parseParamType( + const [param, module] = parseParamType( 'arg', `{|${annotateProp(propName, propType)}|}`, ); @@ -348,10 +428,13 @@ describe('Flow Module Parser', () => { ); invariant(properties[0].typeAnnotation != null, ''); - return { - ...properties[0], - typeAnnotation: properties[0].typeAnnotation, - }; + return [ + { + ...properties[0], + typeAnnotation: properties[0].typeAnnotation, + }, + module, + ]; } describe( @@ -366,7 +449,7 @@ describe('Flow Module Parser', () => { describe('Props with Primitive Types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of primitive type '${FLOW_TYPE}'`, () => { - const prop = parseParamTypeObjectLiteralProp( + const [prop] = parseParamTypeObjectLiteralProp( 'prop', FLOW_TYPE, ); @@ -376,7 +459,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Object'`, () => { - const prop = parseParamTypeObjectLiteralProp( + const [prop] = parseParamTypeObjectLiteralProp( 'prop', 'Object', ); @@ -388,7 +471,7 @@ describe('Flow Module Parser', () => { describe('Props with Reserved Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of reserved type '${FLOW_TYPE}'`, () => { - const prop = parseParamTypeObjectLiteralProp( + const [prop] = parseParamTypeObjectLiteralProp( 'prop', FLOW_TYPE, ); @@ -422,7 +505,7 @@ describe('Flow Module Parser', () => { propName: string, arrayElementType: string, ) { - const property = parseParamTypeObjectLiteralProp( + const [property, module] = parseParamTypeObjectLiteralProp( 'propName', `Array<${arrayElementType}>`, ); @@ -437,12 +520,12 @@ describe('Flow Module Parser', () => { const {elementType} = property.typeAnnotation; expect(elementType).not.toBe(null); invariant(elementType != null, ''); - return elementType; + return [elementType, module]; } PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', FLOW_TYPE, ); @@ -453,7 +536,7 @@ describe('Flow Module Parser', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', FLOW_TYPE, ); @@ -471,14 +554,20 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array'`, () => { - const elementType = parseArrayElementType('prop', 'Object'); + const [elementType] = parseArrayElementType( + 'prop', + 'Object', + ); expect(elementType.type).toBe( 'GenericObjectTypeAnnotation', ); }); - it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type of some array of an aliase`, () => { - const elementType = parseArrayElementType('prop', 'Animal'); + it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type of some array of an alias`, () => { + const [elementType, module] = parseArrayElementType( + 'prop', + 'Animal', + ); expect(elementType.type).toBe('TypeAliasTypeAnnotation'); invariant( @@ -487,10 +576,11 @@ describe('Flow Module Parser', () => { ); expect(elementType.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of 'Array<{|foo: ?string|}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', '{|foo: ?string|}', ); @@ -514,7 +604,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type '{|foo: ?string|}'`, () => { - const property = parseParamTypeObjectLiteralProp( + const [property] = parseParamTypeObjectLiteralProp( 'prop', '{|foo: ?string|}', ); @@ -542,7 +632,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${PARAM_TYPE_DESCRIPTION} parameter type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of some type alias`, () => { - const property = parseParamTypeObjectLiteralProp( + const [property, module] = parseParamTypeObjectLiteralProp( 'prop', 'Animal', ); @@ -556,6 +646,7 @@ describe('Flow Module Parser', () => { ); expect(property.typeAnnotation.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); }, ); @@ -594,9 +685,9 @@ describe('Flow Module Parser', () => { const module = parseModule(` import type {TurboModule} from 'RCTExport'; import * as TurboModuleRegistry from 'TurboModuleRegistry'; - type Animal = {| - name: string, - |}; + + ${TYPE_ALIAS_DECLARATIONS} + export interface Spec extends TurboModule { +useArg(): ${annotateRet(flowType)}; } @@ -607,7 +698,7 @@ describe('Flow Module Parser', () => { const {returnTypeAnnotation} = module.properties[0].typeAnnotation; expect(returnTypeAnnotation).not.toBe(null); expect(returnTypeAnnotation.nullable).toBe(IS_RETURN_TYPE_NULLABLE); - return returnTypeAnnotation; + return [returnTypeAnnotation, module]; } describe( @@ -616,7 +707,7 @@ describe('Flow Module Parser', () => { ['Promise', 'Promise<{||}>', 'Promise<*>'].forEach( promiseFlowType => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type '${promiseFlowType}'`, () => { - const returnTypeAnnotation = parseReturnType(promiseFlowType); + const [returnTypeAnnotation] = parseReturnType(promiseFlowType); expect(returnTypeAnnotation.type).toBe( 'GenericPromiseTypeAnnotation', ); @@ -627,7 +718,7 @@ describe('Flow Module Parser', () => { describe('Primitive Types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} primitive return of type '${FLOW_TYPE}'`, () => { - const returnTypeAnnotation = parseReturnType(FLOW_TYPE); + const [returnTypeAnnotation] = parseReturnType(FLOW_TYPE); expect(returnTypeAnnotation.type).toBe(PARSED_TYPE_NAME); }); }); @@ -636,7 +727,7 @@ describe('Flow Module Parser', () => { describe('Reserved Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} reserved return of type '${FLOW_TYPE}'`, () => { - const returnTypeAnnotation = parseReturnType(FLOW_TYPE); + const [returnTypeAnnotation] = parseReturnType(FLOW_TYPE); expect(returnTypeAnnotation.type).toBe( 'ReservedFunctionValueTypeAnnotation', ); @@ -661,7 +752,7 @@ describe('Flow Module Parser', () => { }); function parseArrayElementReturnType(flowType: string) { - const returnTypeAnnotation = parseReturnType( + const [returnTypeAnnotation, module] = parseReturnType( 'Array' + (flowType != null ? `<${flowType}>` : ''), ); expect(returnTypeAnnotation.type).toBe('ArrayTypeAnnotation'); @@ -673,7 +764,7 @@ describe('Flow Module Parser', () => { const {elementType} = returnTypeAnnotation; expect(elementType).not.toBe(null); invariant(elementType != null, ''); - return elementType; + return [elementType, module]; } // TODO: Do we support nullable element types? @@ -681,7 +772,9 @@ describe('Flow Module Parser', () => { describe('Primitive Element Types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementReturnType(FLOW_TYPE); + const [elementType, module] = parseArrayElementReturnType( + FLOW_TYPE, + ); expect(elementType.type).toBe(PARSED_TYPE_NAME); }); }); @@ -690,7 +783,7 @@ describe('Flow Module Parser', () => { describe('Reserved Element Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementReturnType(FLOW_TYPE); + const [elementType] = parseArrayElementReturnType(FLOW_TYPE); expect(elementType.type).toBe( 'ReservedFunctionValueTypeAnnotation', ); @@ -705,19 +798,22 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array'`, () => { - const elementType = parseArrayElementReturnType('Object'); + const [elementType] = parseArrayElementReturnType('Object'); expect(elementType.type).toBe('GenericObjectTypeAnnotation'); }); - it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of some array of an aliase`, () => { - const elementType = parseArrayElementReturnType('Animal'); + it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of some array of an alias`, () => { + const [elementType, module] = parseArrayElementReturnType( + 'Animal', + ); expect(elementType.type).toBe('TypeAliasTypeAnnotation'); invariant(elementType.type === 'TypeAliasTypeAnnotation', ''); expect(elementType.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Array<{|foo: ?string|}>'`, () => { - const elementType = parseArrayElementReturnType( + const [elementType] = parseArrayElementReturnType( '{|foo: ?string|}', ); expect(elementType.type).toBe('ObjectTypeAnnotation'); @@ -739,13 +835,14 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of some type alias`, () => { - const returnTypeAnnotation = parseReturnType('Animal'); + const [returnTypeAnnotation, module] = parseReturnType('Animal'); expect(returnTypeAnnotation.type).toBe('TypeAliasTypeAnnotation'); invariant( returnTypeAnnotation.type === 'TypeAliasTypeAnnotation', '', ); expect(returnTypeAnnotation.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); it(`should not parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Function'`, () => { @@ -755,7 +852,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return of type 'Object'`, () => { - const returnTypeAnnotation = parseReturnType('Object'); + const [returnTypeAnnotation] = parseReturnType('Object'); expect(returnTypeAnnotation.type).toBe( 'GenericObjectTypeAnnotation', ); @@ -765,7 +862,7 @@ describe('Flow Module Parser', () => { // TODO: Inexact vs exact object literals? it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an empty object literal`, () => { - const returnTypeAnnotation = parseReturnType('{||}'); + const [returnTypeAnnotation] = parseReturnType('{||}'); expect(returnTypeAnnotation.type).toBe('ObjectTypeAnnotation'); invariant( returnTypeAnnotation.type === 'ObjectTypeAnnotation', @@ -809,7 +906,7 @@ describe('Flow Module Parser', () => { propName: string, propType: string, ) { - const returnTypeAnnotation = parseReturnType( + const [returnTypeAnnotation, module] = parseReturnType( `{|${annotateProp(propName, propType)}|}`, ); expect(returnTypeAnnotation.type).toBe('ObjectTypeAnnotation'); @@ -831,10 +928,13 @@ describe('Flow Module Parser', () => { expect(property.typeAnnotation).not.toBe(null); expect(property.typeAnnotation?.nullable).toBe(nullable); invariant(property.typeAnnotation != null, ''); - return { - ...property, - typeAnnotation: property.typeAnnotation, - }; + return [ + { + ...property, + typeAnnotation: property.typeAnnotation, + }, + module, + ]; } describe( @@ -853,7 +953,7 @@ describe('Flow Module Parser', () => { describe('Props with Primitive Types', () => { PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of primitive type '${FLOW_TYPE}'`, () => { - const property = parseObjectLiteralReturnTypeProp( + const [property] = parseObjectLiteralReturnTypeProp( 'prop', FLOW_TYPE, ); @@ -865,7 +965,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Object'`, () => { - const property = parseObjectLiteralReturnTypeProp( + const [property] = parseObjectLiteralReturnTypeProp( 'prop', 'Object', ); @@ -878,7 +978,7 @@ describe('Flow Module Parser', () => { describe('Props with Reserved Types', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of reserved type '${FLOW_TYPE}'`, () => { - const property = parseObjectLiteralReturnTypeProp( + const [property] = parseObjectLiteralReturnTypeProp( 'prop', FLOW_TYPE, ); @@ -913,7 +1013,10 @@ describe('Flow Module Parser', () => { propName: string, arrayElementType: string, ) { - const property = parseObjectLiteralReturnTypeProp( + const [ + property, + module, + ] = parseObjectLiteralReturnTypeProp( propName, `Array<${arrayElementType}>`, ); @@ -929,12 +1032,12 @@ describe('Flow Module Parser', () => { const {elementType} = property.typeAnnotation; expect(elementType).not.toBe(null); invariant(elementType != null, ''); - return elementType; + return [elementType, module]; } PRIMITIVES.forEach(([FLOW_TYPE, PARSED_TYPE_NAME]) => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', FLOW_TYPE, ); @@ -944,7 +1047,7 @@ describe('Flow Module Parser', () => { RESERVED_FUNCTION_VALUE_TYPE_NAME.forEach(FLOW_TYPE => { it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array<${FLOW_TYPE}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', FLOW_TYPE, ); @@ -962,7 +1065,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', 'Object', ); @@ -973,7 +1076,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type of some array of an aliase`, () => { - const elementType = parseArrayElementType( + const [elementType, module] = parseArrayElementType( 'prop', 'Animal', ); @@ -983,10 +1086,11 @@ describe('Flow Module Parser', () => { '', ); expect(elementType.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of type 'Array<{|foo: ?string|}>'`, () => { - const elementType = parseArrayElementType( + const [elementType] = parseArrayElementType( 'prop', '{|foo: ?string|}', ); @@ -1014,7 +1118,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of '{|foo: ?string|}'`, () => { - const property = parseObjectLiteralReturnTypeProp( + const [property] = parseObjectLiteralReturnTypeProp( 'prop', '{|foo: ?string|}', ); @@ -1046,7 +1150,7 @@ describe('Flow Module Parser', () => { }); it(`should parse methods that have ${RETURN_TYPE_DESCRIPTION} return type of an object literal with ${PROP_TYPE_DESCRIPTION} prop of some type alias`, () => { - const property = parseObjectLiteralReturnTypeProp( + const [property, module] = parseObjectLiteralReturnTypeProp( 'prop', 'Animal', ); @@ -1061,6 +1165,7 @@ describe('Flow Module Parser', () => { ); expect(property.typeAnnotation.name).toBe('Animal'); + expectAnimalTypeAliasToExist(module); }); }, ); 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 98dc32c5abd..31de4e511f8 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/index.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/index.js @@ -40,10 +40,11 @@ function translateTypeAnnotation( types: TypeDeclarationMap, aliasMap: NativeModuleAliasMap, ): NativeModuleTypeAnnotation { - const {nullable, typeAnnotation} = resolveTypeAnnotation( - flowTypeAnnotation, - types, - ); + const { + nullable, + typeAnnotation, + typeAliasResolutionStatus, + } = resolveTypeAnnotation(flowTypeAnnotation, types); switch (typeAnnotation.type) { case 'GenericTypeAnnotation': { @@ -161,8 +162,7 @@ function translateTypeAnnotation( } } case 'ObjectTypeAnnotation': { - const objectTypeAnnotation = { - nullable, + const objectTypeAnnotationPartial = { type: 'ObjectTypeAnnotation', properties: typeAnnotation.properties.map(property => { const {optional} = property; @@ -179,30 +179,55 @@ function translateTypeAnnotation( }), }; - if (flowTypeAnnotation.type === 'GenericTypeAnnotation') { - aliasMap[flowTypeAnnotation.id.name] = objectTypeAnnotation; + if (!typeAliasResolutionStatus.successful) { return { - nullable: false, - type: 'TypeAliasTypeAnnotation', - name: flowTypeAnnotation.id.name, + nullable, + ...objectTypeAnnotationPartial, }; } - if ( - flowTypeAnnotation.type === 'NullableTypeAnnotation' && - flowTypeAnnotation.typeAnnotation.type === 'GenericTypeAnnotation' - ) { - aliasMap[ - flowTypeAnnotation.typeAnnotation.id.name - ] = objectTypeAnnotation; - return { - nullable: true, - type: 'TypeAliasTypeAnnotation', - name: flowTypeAnnotation.typeAnnotation.id.name, - }; - } + /** + * All aliases RHS are required. + */ + aliasMap[typeAliasResolutionStatus.aliasName] = { + nullable: false, + ...objectTypeAnnotationPartial, + }; - return objectTypeAnnotation; + /** + * Nullability of type aliases is transitive. + * + * Consider this case: + * + * type Animal = ?{| + * name: string, + * |}; + * + * type B = Animal + * + * export interface Spec extends TurboModule { + * +greet: (animal: B) => void; + * } + * + * In this case, we follow B to Animal, and then Animal to ?{|name: string|}. + * + * We: + * 1. Replace `+greet: (animal: B) => void;` with `+greet: (animal: ?Animal) => void;`, + * 2. Pretend that Animal = {|name: string|}. + * + * Why do we do this? + * 1. In ObjC, we need to generate a struct called Animal, not B. + * 2. This design is simpler than managing nullability within both the type alias usage, and the type alias RHS. + * 3. What does it mean for a C++ struct, which is what this type alias RHS will generate, to be nullable? ¯\_(ツ)_/¯ + * Nullability is a concept that only makes sense when talking about instances (i.e: usages) of the C++ structs. + * Hence, it's better to manage nullability within the actual TypeAliasTypeAnnotation nodes, and not the + * associated ObjectTypeAnnotations. + */ + return { + nullable: nullable, + type: 'TypeAliasTypeAnnotation', + name: typeAliasResolutionStatus.aliasName, + }; } case 'BooleanTypeAnnotation': { return { diff --git a/packages/react-native-codegen/src/parsers/flow/utils.js b/packages/react-native-codegen/src/parsers/flow/utils.js index 87ec1894c06..15b13c59499 100644 --- a/packages/react-native-codegen/src/parsers/flow/utils.js +++ b/packages/react-native-codegen/src/parsers/flow/utils.js @@ -25,11 +25,24 @@ export type ASTNode = Object; const invariant = require('invariant'); +type TypeAliasResolutionStatus = + | $ReadOnly<{| + successful: true, + aliasName: string, + |}> + | $ReadOnly<{| + successful: false, + |}>; + function resolveTypeAnnotation( // TODO(T71778680): This is an Flow TypeAnnotation. Flow-type this typeAnnotation: $FlowFixMe, types: TypeDeclarationMap, -): {nullable: boolean, typeAnnotation: $FlowFixMe} { +): { + nullable: boolean, + typeAnnotation: $FlowFixMe, + typeAliasResolutionStatus: TypeAliasResolutionStatus, +} { invariant( typeAnnotation != null, 'resolveTypeAnnotation(): typeAnnotation cannot be null', @@ -37,12 +50,19 @@ function resolveTypeAnnotation( let node = typeAnnotation; let nullable = false; + let typeAliasResolutionStatus: TypeAliasResolutionStatus = { + successful: false, + }; for (;;) { if (node.type === 'NullableTypeAnnotation') { nullable = true; node = node.typeAnnotation; } else if (node.type === 'GenericTypeAnnotation') { + typeAliasResolutionStatus = { + successful: true, + aliasName: node.id.name, + }; const resolvedTypeAnnotation = types[node.id.name]; if (resolvedTypeAnnotation == null) { break; @@ -61,6 +81,7 @@ function resolveTypeAnnotation( return { nullable: nullable, typeAnnotation: node, + typeAliasResolutionStatus, }; }