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]

Reviewed By: makovkastar

Differential Revision: D61825742
  • Loading branch information
elicwhite authored and facebook-github-bot committed Aug 29, 2024
1 parent 20e3f45 commit 7634809
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 80 deletions.
102 changes: 53 additions & 49 deletions packages/react-native-codegen/src/CodegenSchema.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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<PropTypeAnnotation>
| ReservedPropTypeAnnotation
| {
readonly type: 'ArrayTypeAnnotation';
readonly elementType: ObjectTypeAnnotation<PropTypeAnnotation>;
};
| BooleanTypeAnnotation
| StringTypeAnnotation
| DoubleTypeAnnotation
| FloatTypeAnnotation
| Int32TypeAnnotation
| {
readonly type: 'StringEnumTypeAnnotation';
readonly default: string;
readonly options: readonly string[];
}
| ObjectTypeAnnotation<PropTypeAnnotation>
| ReservedPropTypeAnnotation
| {
readonly type: 'ArrayTypeAnnotation';
readonly elementType: ObjectTypeAnnotation<PropTypeAnnotation>;
};
}

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<PropTypeAnnotation>
| ArrayTypeAnnotation
Expand All @@ -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<
Expand Down Expand Up @@ -279,7 +279,11 @@ export interface NativeModuleArrayTypeAnnotation<T extends Nullable<NativeModule
* TODO(T72031674): Migrate all our NativeModule specs to not use
* invalid Array ElementTypes. Then, make the elementType required.
*/
readonly elementType?: T | undefined;
readonly elementType: T | UnsafeAnyTypeAnnotation;
}

export interface UnsafeAnyTypeAnnotation {
readonly type: 'AnyTypeAnnotation',
}

export interface NativeModuleStringTypeAnnotation {
Expand Down Expand Up @@ -375,7 +379,7 @@ export type NativeModuleEventEmitterTypeAnnotation =
| NativeModuleEventEmitterBaseTypeAnnotation
| {
readonly type: 'ArrayTypeAnnotation';
readonly elementType: NativeModuleEventEmitterBaseTypeAnnotation | {type: string};
readonly elementType: NativeModuleEventEmitterBaseTypeAnnotation | { type: string };
};

export type NativeModuleBaseTypeAnnotation =
Expand Down
6 changes: 5 additions & 1 deletion packages/react-native-codegen/src/CodegenSchema.js
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
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
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
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
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
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
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
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
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
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
Loading

0 comments on commit 7634809

Please sign in to comment.