From 1fc27c4cba082ba3f916569c02286c396d617eec Mon Sep 17 00:00:00 2001 From: MaeIg Date: Mon, 10 Oct 2022 10:47:40 -0700 Subject: [PATCH] Extract TypeAlias logic of translateTypeAnnotation from the flow and typescript folders in parsers-primitives (#34918) Summary: This PR aims to reduce code duplication by extracting `typeAliasResolution` logic from the flow and typescript folders into a shared parsers-primitives file. It is a task of https://github.com/facebook/react-native/issues/34872: > Wrap the TypeAlias resolution lines ([Flow](https://github.com/facebook/react-native/blob/b444f0e44e0d8670139acea5f14c2de32c5e2ddc/packages/react-native-codegen/src/parsers/flow/modules/index.js#L321-L362), [TypeScript](https://github.com/facebook/react-native/blob/00b795642a6562fb52d6df12e367b84674994623/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L356-L397)) in a proper typeAliasResolution function in the parsers-primitives.js files and replace those lines with the new function. ## Changelog [Internal] [Changed] - Extract TypeAlias logic of translateTypeAnnotation from the flow and typescript folders in parsers-primitives Pull Request resolved: https://github.com/facebook/react-native/pull/34918 Test Plan: All tests are passing, with `yarn jest react-native-codegen`: image Reviewed By: rshest Differential Revision: D40223495 Pulled By: rshest fbshipit-source-id: c74b68385e59497b6a8eaa56b96a001ef447a2cd --- .../__tests__/parsers-primitives-test.js | 96 +++++++++++++++++++ .../src/parsers/flow/modules/index.js | 49 ++-------- .../src/parsers/flow/utils.js | 11 +-- .../src/parsers/parsers-primitives.js | 66 ++++++++++++- .../src/parsers/typescript/modules/index.js | 49 ++-------- .../src/parsers/typescript/utils.js | 11 +-- .../react-native-codegen/src/parsers/utils.js | 9 ++ 7 files changed, 186 insertions(+), 105 deletions(-) diff --git a/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js b/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js index 5f3f785221b475..40ec6106add7cb 100644 --- a/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js +++ b/packages/react-native-codegen/src/parsers/__tests__/parsers-primitives-test.js @@ -17,6 +17,7 @@ const { emitNumber, emitInt32, emitRootTag, + typeAliasResolution, } = require('../parsers-primitives.js'); describe('emitBoolean', () => { @@ -148,3 +149,98 @@ describe('emitDouble', () => { }); }); }); + +describe('typeAliasResolution', () => { + const objectTypeAnnotation = { + type: 'ObjectTypeAnnotation', + properties: [ + { + name: 'Foo', + optional: false, + typeAnnotation: { + type: 'StringTypeAnnotation', + }, + }, + ], + }; + + describe('when typeAliasResolutionStatus is successful', () => { + const typeAliasResolutionStatus = {successful: true, aliasName: 'Foo'}; + + describe('when nullable is true', () => { + it('returns nullable TypeAliasTypeAnnotation and map it in aliasMap', () => { + const aliasMap = {}; + const result = typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + true, + ); + + expect(aliasMap).toEqual({Foo: objectTypeAnnotation}); + expect(result).toEqual({ + type: 'NullableTypeAnnotation', + typeAnnotation: { + type: 'TypeAliasTypeAnnotation', + name: 'Foo', + }, + }); + }); + }); + + describe('when nullable is false', () => { + it('returns non nullable TypeAliasTypeAnnotation and map it in aliasMap', () => { + const aliasMap = {}; + const result = typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + false, + ); + + expect(aliasMap).toEqual({Foo: objectTypeAnnotation}); + expect(result).toEqual({ + type: 'TypeAliasTypeAnnotation', + name: 'Foo', + }); + }); + }); + }); + + describe('when typeAliasResolutionStatus is not successful', () => { + const typeAliasResolutionStatus = {successful: false}; + + describe('when nullable is true', () => { + it('returns nullable ObjectTypeAnnotation', () => { + const aliasMap = {}; + const result = typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + true, + ); + + expect(aliasMap).toEqual({}); + expect(result).toEqual({ + type: 'NullableTypeAnnotation', + typeAnnotation: objectTypeAnnotation, + }); + }); + }); + + describe('when nullable is false', () => { + it('returns non nullable ObjectTypeAnnotation', () => { + const aliasMap = {}; + const result = typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + false, + ); + + expect(aliasMap).toEqual({}); + expect(result).toEqual(objectTypeAnnotation); + }); + }); + }); +}); 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 dc0c26659c9ae4..75d0a6013fd8c5 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/index.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/index.js @@ -39,6 +39,7 @@ const { emitNumber, emitInt32, emitRootTag, + typeAliasResolution, } = require('../../parsers-primitives'); const { IncorrectlyParameterizedGenericParserError, @@ -328,48 +329,12 @@ function translateTypeAnnotation( .filter(Boolean), }; - if (!typeAliasResolutionStatus.successful) { - return wrapNullable(nullable, objectTypeAnnotation); - } - - /** - * All aliases RHS are required. - */ - aliasMap[typeAliasResolutionStatus.aliasName] = 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 wrapNullable(nullable, { - type: 'TypeAliasTypeAnnotation', - name: typeAliasResolutionStatus.aliasName, - }); + return typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + nullable, + ); } case 'BooleanTypeAnnotation': { return emitBoolean(nullable); diff --git a/packages/react-native-codegen/src/parsers/flow/utils.js b/packages/react-native-codegen/src/parsers/flow/utils.js index 6051302979148f..6c8b72c10e8230 100644 --- a/packages/react-native-codegen/src/parsers/flow/utils.js +++ b/packages/react-native-codegen/src/parsers/flow/utils.js @@ -10,6 +10,8 @@ 'use strict'; +import type {TypeAliasResolutionStatus} from '../utils'; + const {ParserError} = require('../errors'); /** @@ -55,15 +57,6 @@ 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, diff --git a/packages/react-native-codegen/src/parsers/parsers-primitives.js b/packages/react-native-codegen/src/parsers/parsers-primitives.js index 5c2c9c867513e1..b949ed535adf4c 100644 --- a/packages/react-native-codegen/src/parsers/parsers-primitives.js +++ b/packages/react-native-codegen/src/parsers/parsers-primitives.js @@ -5,19 +5,24 @@ * LICENSE file in the root directory of this source tree. * * @format - * @flow strict + * @flow strict-local */ 'use strict'; import type { + Nullable, + NativeModuleAliasMap, + NativeModuleBaseTypeAnnotation, + NativeModuleTypeAliasTypeAnnotation, + NativeModuleNumberTypeAnnotation, BooleanTypeAnnotation, DoubleTypeAnnotation, Int32TypeAnnotation, - NativeModuleNumberTypeAnnotation, - Nullable, ReservedTypeAnnotation, + ObjectTypeAnnotation, } from '../CodegenSchema'; +import type {TypeAliasResolutionStatus} from './utils'; const {wrapNullable} = require('./parsers-commons'); @@ -54,10 +59,65 @@ function emitDouble(nullable: boolean): Nullable { }); } +function typeAliasResolution( + typeAliasResolutionStatus: TypeAliasResolutionStatus, + objectTypeAnnotation: ObjectTypeAnnotation< + Nullable, + >, + aliasMap: {...NativeModuleAliasMap}, + nullable: boolean, +): + | Nullable + | Nullable>> { + if (!typeAliasResolutionStatus.successful) { + return wrapNullable(nullable, objectTypeAnnotation); + } + + /** + * All aliases RHS are required. + */ + aliasMap[typeAliasResolutionStatus.aliasName] = 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 wrapNullable(nullable, { + type: 'TypeAliasTypeAnnotation', + name: typeAliasResolutionStatus.aliasName, + }); +} + module.exports = { emitBoolean, emitDouble, emitInt32, emitNumber, emitRootTag, + typeAliasResolution, }; 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 fb5bfcef9c8e07..1747aa5f3704f8 100644 --- a/packages/react-native-codegen/src/parsers/typescript/modules/index.js +++ b/packages/react-native-codegen/src/parsers/typescript/modules/index.js @@ -39,6 +39,7 @@ const { emitNumber, emitInt32, emitRootTag, + typeAliasResolution, } = require('../../parsers-primitives'); const { IncorrectlyParameterizedGenericParserError, @@ -364,48 +365,12 @@ function translateTypeAnnotation( .filter(Boolean), }; - if (!typeAliasResolutionStatus.successful) { - return wrapNullable(nullable, objectTypeAnnotation); - } - - /** - * All aliases RHS are required. - */ - aliasMap[typeAliasResolutionStatus.aliasName] = 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 wrapNullable(nullable, { - type: 'TypeAliasTypeAnnotation', - name: typeAliasResolutionStatus.aliasName, - }); + return typeAliasResolution( + typeAliasResolutionStatus, + objectTypeAnnotation, + aliasMap, + nullable, + ); } case 'TSBooleanKeyword': { return emitBoolean(nullable); diff --git a/packages/react-native-codegen/src/parsers/typescript/utils.js b/packages/react-native-codegen/src/parsers/typescript/utils.js index 034efdbce3a1af..87ca0bed7f860a 100644 --- a/packages/react-native-codegen/src/parsers/typescript/utils.js +++ b/packages/react-native-codegen/src/parsers/typescript/utils.js @@ -10,6 +10,8 @@ 'use strict'; +import type {TypeAliasResolutionStatus} from '../utils'; + const {ParserError} = require('../errors'); const {parseTopLevelType} = require('./parseTopLevelType'); @@ -50,15 +52,6 @@ export type ASTNode = Object; const invariant = require('invariant'); -type TypeAliasResolutionStatus = - | $ReadOnly<{ - successful: true, - aliasName: string, - }> - | $ReadOnly<{ - successful: false, - }>; - function resolveTypeAnnotation( // TODO(T108222691): Use flow-types for @babel/parser typeAnnotation: $FlowFixMe, diff --git a/packages/react-native-codegen/src/parsers/utils.js b/packages/react-native-codegen/src/parsers/utils.js index 1626a68b4f6f8f..0aabd8b816732f 100644 --- a/packages/react-native-codegen/src/parsers/utils.js +++ b/packages/react-native-codegen/src/parsers/utils.js @@ -12,6 +12,15 @@ const path = require('path'); +export type TypeAliasResolutionStatus = + | $ReadOnly<{ + successful: true, + aliasName: string, + }> + | $ReadOnly<{ + successful: false, + }>; + function extractNativeModuleName(filename: string): string { // this should drop everything after the file name. For Example it will drop: // .android.js, .android.ts, .android.tsx, .ios.js, .ios.ts, .ios.tsx, .js, .ts, .tsx