Skip to content

Commit

Permalink
Array's with unparsable element type's are explicitly Any vs missing (#…
Browse files Browse the repository at this point in the history 
…46221)

Summary:
Pull Request resolved: #46221

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

Changelog: [Internal]

Differential Revision: D61825742
  • Loading branch information
@elicwhite @facebook-github-bot
elicwhite authored and facebook-github-bot committed Aug 27, 2024
1 parent 851037d commit 1d86bb1
Show file tree
Hide file tree
Showing 12 changed files with 101 additions and 31 deletions.
6 changes: 5 additions & 1 deletion packages/react-native-codegen/src/CodegenSchema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,13 @@ export type NativeModuleArrayTypeAnnotation<
* TODO(T72031674): Migrate all our NativeModule specs to not use
* invalid Array ElementTypes. Then, make the elementType required.
*/
elementType?: T,
elementType: T | UnsafeAnyTypeAnnotation,
}>;

export type UnsafeAnyTypeAnnotation = {
type: 'AnyTypeAnnotation',
};

export type NativeModuleNumberTypeAnnotation = $ReadOnly<{
type: 'NumberTypeAnnotation',
}>;
Expand Down
5 changes: 4 additions & 1 deletion packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/StructCollector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
});
}

Expand Down
4 changes: 2 additions & 2 deletions ...ve-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeConstantsStruct.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ function toObjCType(
case 'GenericObjectTypeAnnotation':
return wrapObjCOptional('id<NSObject>', isRequired);
case 'ArrayTypeAnnotation':
if (typeAnnotation.elementType == null) {
if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') {
return wrapObjCOptional('id<NSObject>', isRequired);
}

Expand Down Expand Up @@ -198,7 +198,7 @@ function toObjCValue(
return value;
case 'ArrayTypeAnnotation':
const {elementType} = typeAnnotation;
if (elementType == null) {
if (elementType.type === 'AnyTypeAnnotation') {
return value;
}

Expand Down
4 changes: 2 additions & 2 deletions ...tive-codegen/src/generators/modules/GenerateModuleObjCpp/header/serializeRegularStruct.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function toObjCType(
case 'GenericObjectTypeAnnotation':
return wrapObjCOptional('id<NSObject>', isRequired);
case 'ArrayTypeAnnotation':
if (typeAnnotation.elementType == null) {
if (typeAnnotation.elementType.type === 'AnyTypeAnnotation') {
return wrapObjCOptional('id<NSObject>', isRequired);
}
return wrapCxxOptional(
Expand Down Expand Up @@ -188,7 +188,7 @@ function toObjCValue(
return value;
case 'ArrayTypeAnnotation':
const {elementType} = typeAnnotation;
if (elementType == null) {
if (elementType.type === 'AnyTypeAnnotation') {
return value;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<id<NSObject>> *', isRequired);
}

Expand Down
6 changes: 6 additions & 0 deletions packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,9 @@ const COMPLEX_OBJECTS: SchemaType = {
type: 'NullableTypeAnnotation',
typeAnnotation: {
type: 'ArrayTypeAnnotation',
elementType: {
type: 'AnyTypeAnnotation',
},
},
},
params: [],
Expand Down Expand Up @@ -1969,6 +1972,9 @@ const CXX_ONLY_NATIVE_MODULES: SchemaType = {
type: 'FunctionTypeAnnotation',
returnTypeAnnotation: {
type: 'ArrayTypeAnnotation',
elementType: {
type: 'AnyTypeAnnotation',
},
},
params: [
{
Expand Down
10 changes: 8 additions & 2 deletions packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,10 @@ describe('buildSchema', () => {
{
name: 'a',
optional: false,
typeAnnotation: {type: 'ArrayTypeAnnotation'},
typeAnnotation: {
type: 'ArrayTypeAnnotation',
elementType: {type: 'AnyTypeAnnotation'},
},
},
],
},
Expand Down Expand Up @@ -1258,7 +1261,10 @@ describe('buildModuleSchema', () => {
{
name: 'a',
optional: false,
typeAnnotation: {type: 'ArrayTypeAnnotation'},
typeAnnotation: {
type: 'ArrayTypeAnnotation',
elementType: {type: 'AnyTypeAnnotation'},
},
},
],
returnTypeAnnotation: {
Expand Down
10 changes: 8 additions & 2 deletions ...egen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
}
]
Expand Down
31 changes: 23 additions & 8 deletions packages/react-native-codegen/src/parsers/flow/modules/__tests__/module-parser-e2e-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
paramTypeAnnotation.elementType,
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(elementType);
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
Expand Down
3 changes: 3 additions & 0 deletions packages/react-native-codegen/src/parsers/parsers-primitives.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@ function translateArrayTypeAnnotation(
} catch (ex) {
return wrapNullable(nullable, {
type: 'ArrayTypeAnnotation',
elementType: {
type: 'AnyTypeAnnotation',
},
});
}
}
Expand Down
20 changes: 16 additions & 4 deletions ...typescript/modules/__tests__/__snapshots__/typescript-module-parser-snapshot-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
}
]
Expand Down Expand Up @@ -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'
}
}
}
]
Expand Down
31 changes: 23 additions & 8 deletions ...ive-codegen/src/parsers/typescript/modules/__tests__/typescript-module-parser-e2e-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
paramTypeAnnotation.elementType,
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
nullableElementType,
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(elementType);
Expand Down Expand Up @@ -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<NativeModuleBaseTypeAnnotation>(
Expand Down

0 comments on commit 1d86bb1

Please sign in to comment.