Skip to content

Commit

Permalink
Replace ternary in assertGenericTypeAnnotationHasExactlyOneTypeParame…
Browse files Browse the repository at this point in the history
…ter with typeParameterInstantiation attribute in parser (facebook#35157)

Summary:
Part of facebook#34872:
> Create a new function typeParameterInstantiation in the [parsers.js file](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parser.js) and add documentation to it. Implement it properly in the [FlowParser.js](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/flow/parser.js#L15) and in the [TypeScriptParser.js](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/typescript/parser.js#L15). Update the signature of [assertGenericTypeAnnotationHasExactlyOneTypeParameter](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L67) function to take the Parser instead of the language and use the new function in place of the [ternary operator ?:](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/parsers-commons.js#L83).

There are 3 things I'm not sure about:
1. The issue suggests to create a new function. For this case I believe an attribute is simpler. Is there a reason to prefer a function ?
2. To update the tests I had to create a mocked parser. I created a new file `parserMock` (I took example on [AnimatedMock](https://github.com/facebook/react-native/blob/main/Libraries/Animated/AnimatedMock.js)). Does it seem ok ?
3. I'm not sure what to add in the documentation of `typeParameterInstantiation`

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Internal] [Changed] - Replace ternary in assertGenericTypeAnnotationHasExactlyOneTypeParameter with typeParameterInstantiation attribute in parser

Pull Request resolved: facebook#35157

Test Plan: I tested using Jest and Flow commands.

Reviewed By: rshest

Differential Revision: D40889856

Pulled By: cipolleschi

fbshipit-source-id: 8d9a8e087852f98dcc3fc0ecf1d4a7153f482ce7
  • Loading branch information
Antoine Doubovetzky authored and OlimpiaZurek committed May 22, 2023
1 parent fb080b1 commit 34451e8
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ const {
} = require('../parsers-commons.js');
const {UnsupportedUnionTypeAnnotationParserError} = require('../errors');
import type {UnionTypeAnnotationMemberType} from '../../CodegenSchema';
import {MockedParser} from '../parserMock';

const parser = new MockedParser();

describe('wrapNullable', () => {
describe('when nullable is true', () => {
Expand Down Expand Up @@ -101,7 +104,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotation,
'Flow',
parser,
),
).not.toThrow();
});
Expand All @@ -117,14 +120,14 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotation,
'Flow',
parser,
),
).toThrowErrorMatchingInlineSnapshot(
`"Module testModuleName: Generic 'typeAnnotationName' must have type parameters."`,
);
});

it('throws an error if typeAnnotation.typeParameters.type is not TypeParameterInstantiation when language is Flow', () => {
it('throws an error if typeAnnotation.typeParameters.type is not equal to parser.typeParameterInstantiation', () => {
const flowTypeAnnotation = {
typeParameters: {
type: 'wrongType',
Expand All @@ -138,36 +141,14 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
flowTypeAnnotation,
'Flow',
parser,
),
).toThrowErrorMatchingInlineSnapshot(
`"assertGenericTypeAnnotationHasExactlyOneTypeParameter: Type parameters must be an AST node of type 'TypeParameterInstantiation'"`,
);
});

it('throws an error if typeAnnotation.typeParameters.type is not TSTypeParameterInstantiation when language is TypeScript', () => {
const typeScriptTypeAnnotation = {
typeParameters: {
type: 'wrongType',
params: [1],
},
typeName: {
name: 'typeAnnotationName',
},
};
expect(() =>
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeScriptTypeAnnotation,
'TypeScript',
),
).toThrowErrorMatchingInlineSnapshot(
`"assertGenericTypeAnnotationHasExactlyOneTypeParameter: Type parameters must be an AST node of type 'TSTypeParameterInstantiation'"`,
);
});

it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for Flow", () => {
const language: ParserType = 'Flow';
it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter", () => {
const typeAnnotationWithTwoParams = {
typeParameters: {
params: [1, 2],
Expand All @@ -181,7 +162,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotationWithTwoParams,
language,
parser,
),
).toThrowErrorMatchingInlineSnapshot(
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
Expand All @@ -200,48 +181,7 @@ describe('assertGenericTypeAnnotationHasExactlyOneTypeParameter', () => {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotationWithNoParams,
language,
),
).toThrowErrorMatchingInlineSnapshot(
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
);
});

it("throws an IncorrectlyParameterizedGenericParserError if typeParameters don't have 1 exactly parameter for TS", () => {
const language: ParserType = 'TypeScript';
const typeAnnotationWithTwoParams = {
typeParameters: {
params: [1, 2],
type: 'TSTypeParameterInstantiation',
},
typeName: {
name: 'typeAnnotationName',
},
};
expect(() =>
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotationWithTwoParams,
language,
),
).toThrowErrorMatchingInlineSnapshot(
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
);

const typeAnnotationWithNoParams = {
typeParameters: {
params: [],
type: 'TSTypeParameterInstantiation',
},
typeName: {
name: 'typeAnnotationName',
},
};
expect(() =>
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
moduleName,
typeAnnotationWithNoParams,
language,
parser,
),
).toThrowErrorMatchingInlineSnapshot(
`"Module testModuleName: Generic 'typeAnnotationName' must have exactly one type parameter."`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const {
emitMixedTypeAnnotation,
typeAliasResolution,
} = require('../parsers-primitives.js');
import {MockedParser} from '../parserMock';

const parser = new MockedParser();

describe('emitBoolean', () => {
describe('when nullable is true', () => {
Expand Down Expand Up @@ -334,7 +337,7 @@ describe('typeAliasResolution', () => {

describe('emitPromise', () => {
const moduleName = 'testModuleName';
const language = 'Flow';

describe("when typeAnnotation doesn't have exactly one typeParameter", () => {
const typeAnnotation = {
typeParameters: {
Expand All @@ -348,7 +351,7 @@ describe('emitPromise', () => {
it('throws an IncorrectlyParameterizedGenericParserError error', () => {
const nullable = false;
expect(() =>
emitPromise(moduleName, typeAnnotation, language, nullable),
emitPromise(moduleName, typeAnnotation, parser, nullable),
).toThrow();
});
});
Expand All @@ -370,7 +373,7 @@ describe('emitPromise', () => {
const result = emitPromise(
moduleName,
typeAnnotation,
language,
parser,
nullable,
);
const expected = {
Expand All @@ -389,7 +392,7 @@ describe('emitPromise', () => {
const result = emitPromise(
moduleName,
typeAnnotation,
language,
parser,
nullable,
);
const expected = {
Expand Down
11 changes: 3 additions & 8 deletions packages/react-native-codegen/src/parsers/flow/modules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,19 +184,14 @@ function translateTypeAnnotation(
return emitRootTag(nullable);
}
case 'Promise': {
return emitPromise(
hasteModuleName,
typeAnnotation,
language,
nullable,
);
return emitPromise(hasteModuleName, typeAnnotation, parser, nullable);
}
case 'Array':
case '$ReadOnlyArray': {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
hasteModuleName,
typeAnnotation,
language,
parser,
);
return translateArrayTypeAnnotation(
Expand All @@ -213,7 +208,7 @@ function translateTypeAnnotation(
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
hasteModuleName,
typeAnnotation,
language,
parser,
);
const [paramType, isParamNullable] = unwrapNullable(
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-codegen/src/parsers/flow/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type {ParserType} from '../errors';
import type {Parser} from '../parser';

class FlowParser implements Parser {
typeParameterInstantiation: string = 'TypeParameterInstantiation';

getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string {
return maybeEnumDeclaration.body.type
.replace('EnumNumberBody', 'NumberTypeAnnotation')
Expand Down
5 changes: 5 additions & 0 deletions packages/react-native-codegen/src/parsers/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ import type {ParserType} from './errors';
* It exposes all the methods that contain language-specific logic.
*/
export interface Parser {
/**
* This is the TypeParameterInstantiation value
*/
typeParameterInstantiation: string;

/**
* Given a type declaration, it possibly returns the name of the Enum type.
* @parameter maybeEnumDeclaration: an object possibly containing an Enum declaration.
Expand Down
36 changes: 36 additions & 0 deletions packages/react-native-codegen/src/parsers/parserMock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict
* @format
*/

'use strict';

import type {Parser} from './parser';
import type {ParserType} from './errors';

export class MockedParser implements Parser {
typeParameterInstantiation: string = 'TypeParameterInstantiation';

getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string {
return maybeEnumDeclaration.body.type
.replace('EnumNumberBody', 'NumberTypeAnnotation')
.replace('EnumStringBody', 'StringTypeAnnotation');
}

isEnumDeclaration(maybeEnumDeclaration: $FlowFixMe): boolean {
return maybeEnumDeclaration.type === 'EnumDeclaration';
}

language(): ParserType {
return 'Flow';
}

nameForGenericTypeAnnotation(typeAnnotation: $FlowFixMe): string {
return typeAnnotation.id.name;
}
}
11 changes: 4 additions & 7 deletions packages/react-native-codegen/src/parsers/parsers-commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,17 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter(
* TODO(T108222691): Use flow-types for @babel/parser
*/
typeAnnotation: $FlowFixMe,
language: ParserType,
parser: Parser,
) {
if (typeAnnotation.typeParameters == null) {
throw new MissingTypeParameterGenericParserError(
moduleName,
typeAnnotation,
language,
parser.language(),
);
}

const typeAnnotationType =
language === 'TypeScript'
? 'TSTypeParameterInstantiation'
: 'TypeParameterInstantiation';
const typeAnnotationType = parser.typeParameterInstantiation;

invariant(
typeAnnotation.typeParameters.type === typeAnnotationType,
Expand All @@ -101,7 +98,7 @@ function assertGenericTypeAnnotationHasExactlyOneTypeParameter(
throw new MoreThanOneTypeParameterGenericParserError(
moduleName,
typeAnnotation,
language,
parser.language(),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import type {
NamedShape,
} from '../CodegenSchema';
import type {ParserType} from './errors';
import type {Parser} from './parser';
import type {
ParserErrorCapturer,
TypeAliasResolutionStatus,
Expand Down Expand Up @@ -174,13 +175,13 @@ function typeAliasResolution(
function emitPromise(
hasteModuleName: string,
typeAnnotation: $FlowFixMe,
language: ParserType,
parser: Parser,
nullable: boolean,
): Nullable<NativeModulePromiseTypeAnnotation> {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
hasteModuleName,
typeAnnotation,
language,
parser,
);
return wrapNullable(nullable, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,14 @@ function translateTypeAnnotation(
return emitRootTag(nullable);
}
case 'Promise': {
return emitPromise(
hasteModuleName,
typeAnnotation,
language,
nullable,
);
return emitPromise(hasteModuleName, typeAnnotation, parser, nullable);
}
case 'Array':
case 'ReadonlyArray': {
assertGenericTypeAnnotationHasExactlyOneTypeParameter(
hasteModuleName,
typeAnnotation,
language,
parser,
);

return translateArrayTypeAnnotation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type {ParserType} from '../errors';
import type {Parser} from '../parser';

class TypeScriptParser implements Parser {
typeParameterInstantiation: string = 'TSTypeParameterInstantiation';

getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string {
if (maybeEnumDeclaration.members[0].initializer) {
return maybeEnumDeclaration.members[0].initializer.type
Expand Down

0 comments on commit 34451e8

Please sign in to comment.