Skip to content

Fix codegen output for object with indexer #35344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
Closed

Fix codegen output for object with indexer #35344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a6a7432
[TS] Fix dictionary type schema output
ZihanChen-MSFT Nov 14, 2022
baffa9f
[TS] Delete unnecessary test cases
ZihanChen-MSFT Nov 14, 2022
f55ee35
[Flow] Fix dictionary type schema output
ZihanChen-MSFT Nov 14, 2022
11e39c2
[Flow] Delete unnecessary test cases
ZihanChen-MSFT Nov 14, 2022
d76e478
yarn prettier
ZihanChen-MSFT Nov 14, 2022
d2f6506
yarn flow-check-ios
ZihanChen-MSFT Nov 14, 2022
2d36d51
Fix code review comments
ZihanChen-MSFT Nov 15, 2022
1ebab2d
...
ZihanChen-MSFT Nov 15, 2022
dd46fe7
Fix code review comments
ZihanChen-MSFT Nov 17, 2022
f0fc95e
Update module-parser-snapshot-test.js.snap
ZihanChen-MSFT Nov 29, 2022
5f11adc
Check property type of indexed object
ZihanChen-MSFT Nov 29, 2022
a319914
yarn prettier
ZihanChen-MSFT Nov 29, 2022
49c8252
Fix code review comments
ZihanChen-MSFT Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 4 additions & 61 deletions packages/react-native-codegen/src/parsers/__tests__/parsers-commons-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,18 +225,7 @@ describe('isObjectProperty', () => {
expect(result).toEqual(true);
});

it("returns 'true' if 'property.type' is 'ObjectTypeIndexer'", () => {
const result = isObjectProperty(
{
type: 'ObjectTypeIndexer',
...propertyStub,
},
language,
);
expect(result).toEqual(true);
});

it("returns 'false' if 'property.type' is not 'ObjectTypeProperty' or 'ObjectTypeIndexer'", () => {
it("returns 'false' if 'property.type' is not 'ObjectTypeProperty'", () => {
const result = isObjectProperty(
{
type: 'notObjectTypeProperty',
Expand All @@ -261,18 +250,7 @@ describe('isObjectProperty', () => {
expect(result).toEqual(true);
});

it("returns 'true' if 'property.type' is 'TSIndexSignature'", () => {
const result = isObjectProperty(
{
type: 'TSIndexSignature',
...propertyStub,
},
language,
);
expect(result).toEqual(true);
});

it("returns 'false' if 'property.type' is not 'TSPropertySignature' or 'TSIndexSignature'", () => {
it("returns 'false' if 'property.type' is not 'TSPropertySignature'", () => {
const result = isObjectProperty(
{
type: 'notTSPropertySignature',
Expand All @@ -295,7 +273,7 @@ describe('parseObjectProperty', () => {

describe("when 'language' is 'Flow'", () => {
const language: ParserType = 'Flow';
it("throws an 'UnsupportedObjectPropertyTypeAnnotationParserError' error if 'property.type' is not 'ObjectTypeProperty' or 'ObjectTypeIndexer'.", () => {
it("throws an 'UnsupportedObjectPropertyTypeAnnotationParserError' error if 'property.type' is not 'ObjectTypeProperty'.", () => {
const property = {
type: 'notObjectTypeProperty',
typeAnnotation: {
Expand Down Expand Up @@ -329,7 +307,7 @@ describe('parseObjectProperty', () => {

describe("when 'language' is 'TypeScript'", () => {
const language: ParserType = 'TypeScript';
it("throws an 'UnsupportedObjectPropertyTypeAnnotationParserError' error if 'property.type' is not 'TSPropertySignature' or 'TSIndexSignature'.", () => {
it("throws an 'UnsupportedObjectPropertyTypeAnnotationParserError' error if 'property.type' is not 'TSPropertySignature'.", () => {
const property = {
type: 'notTSPropertySignature',
typeAnnotation: {
Expand Down Expand Up @@ -358,40 +336,5 @@ describe('parseObjectProperty', () => {
),
).toThrow(expected);
});

it("returns a 'NativeModuleBaseTypeAnnotation' object with 'typeAnnotation.type' equal to 'GenericObjectTypeAnnotation', if 'property.type' is 'TSIndexSignature'.", () => {
const property = {
type: 'TSIndexSignature',
typeAnnotation: {
type: 'TSIndexSignature',
typeAnnotation: 'TSIndexSignature',
},
key: {
name: 'testKeyName',
},
value: 'wrongValue',
name: 'wrongName',
parameters: [{name: 'testName'}],
};
const result = parseObjectProperty(
property,
moduleName,
types,
aliasMap,
tryParse,
cxxOnly,
nullable,
typeScriptTranslateTypeAnnotation,
typeScriptParser,
);
const expected = {
name: 'testName',
optional: false,
typeAnnotation: wrapNullable(nullable, {
type: 'GenericObjectTypeAnnotation',
}),
};
expect(result).toEqual(expected);
});
});
});
43 changes: 0 additions & 43 deletions packages/react-native-codegen/src/parsers/__tests__/parsers-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,6 @@ describe('FlowParser', () => {
});
});

describe('when propertyOrIndex is ObjectTypeIndexer', () => {
it('returns indexer name', () => {
const indexer = {
type: 'ObjectTypeIndexer',
id: {
name: 'indexerName',
},
};

const expected = 'indexerName';

expect(parser.getKeyName(indexer, hasteModuleName)).toEqual(expected);
});

it('returns `key` if indexer has no name', () => {
const indexer = {
type: 'ObjectTypeIndexer',
id: {},
};

const expected = 'key';

expect(parser.getKeyName(indexer, hasteModuleName)).toEqual(expected);
});
});

describe('when propertyOrIndex is not ObjectTypeProperty or ObjectTypeIndexer', () => {
it('throw UnsupportedObjectPropertyTypeAnnotationParserError', () => {
const indexer = {
Expand Down Expand Up @@ -113,23 +87,6 @@ describe('TypeScriptParser', () => {
});
});

describe('when propertyOrIndex is TSIndexSignature', () => {
it('returns indexer name', () => {
const indexer = {
type: 'TSIndexSignature',
parameters: [
{
name: 'indexerName',
},
],
};

const expected = 'indexerName';

expect(parser.getKeyName(indexer, hasteModuleName)).toEqual(expected);
});
});

describe('when propertyOrIndex is not TSPropertySignature or TSIndexSignature', () => {
it('throw UnsupportedObjectPropertyTypeAnnotationParserError', () => {
const indexer = {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,8 @@ export interface Spec extends TurboModule {
returnObjectArray(): Promise<Array<Object>>;
returnNullableNumber(): Promise<number | null>;
returnEmpty(): Promise<empty>;
returnIndex(): Promise<{ [string]: 'authorized' | 'denied' | 'undetermined' | true | false }>;
returnUnsupportedIndex(): Promise<{ [string]: 'authorized' | 'denied' | 'undetermined' | true | false }>;
returnSupportedIndex(): Promise<{ [string]: CustomObject }>;
returnEnum() : Promise<Season>;
returnObject() : Promise<CustomObject>;
}
Expand Down
68 changes: 17 additions & 51 deletions ...egen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-snapshot-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,32 +126,14 @@ exports[`RN Codegen Flow Parser can generate fixture CXX_ONLY_NATIVE_MODULE 1`]
'typeAnnotation': {
'type': 'FunctionTypeAnnotation',
'returnTypeAnnotation': {
'type': 'ObjectTypeAnnotation',
'properties': [
{
'name': 'b',
'optional': false,
'typeAnnotation': {
'type': 'GenericObjectTypeAnnotation'
}
}
]
'type': 'GenericObjectTypeAnnotation'
},
'params': [
{
'name': 'arg',
'optional': false,
'typeAnnotation': {
'type': 'ObjectTypeAnnotation',
'properties': [
{
'name': 'a',
'optional': false,
'typeAnnotation': {
'type': 'GenericObjectTypeAnnotation'
}
}
]
'type': 'GenericObjectTypeAnnotation'
}
}
]
Expand All @@ -163,32 +145,14 @@ exports[`RN Codegen Flow Parser can generate fixture CXX_ONLY_NATIVE_MODULE 1`]
'typeAnnotation': {
'type': 'FunctionTypeAnnotation',
'returnTypeAnnotation': {
'type': 'ObjectTypeAnnotation',
'properties': [
{
'name': 'key',
'optional': false,
'typeAnnotation': {
'type': 'GenericObjectTypeAnnotation'
}
}
]
'type': 'GenericObjectTypeAnnotation'
},
'params': [
{
'name': 'arg',
'optional': false,
'typeAnnotation': {
'type': 'ObjectTypeAnnotation',
'properties': [
{
'name': 'key',
'optional': false,
'typeAnnotation': {
'type': 'GenericObjectTypeAnnotation'
}
}
]
'type': 'GenericObjectTypeAnnotation'
}
}
]
Expand Down Expand Up @@ -1810,23 +1774,25 @@ exports[`RN Codegen Flow Parser can generate fixture PROMISE_WITH_COMMONLY_USED_
}
},
{
'name': 'returnIndex',
'name': 'returnUnsupportedIndex',
'optional': false,
'typeAnnotation': {
'type': 'FunctionTypeAnnotation',
'returnTypeAnnotation': {
'type': 'PromiseTypeAnnotation'
},
'params': []
}
},
{
'name': 'returnSupportedIndex',
'optional': false,
'typeAnnotation': {
'type': 'FunctionTypeAnnotation',
'returnTypeAnnotation': {
'type': 'PromiseTypeAnnotation',
'elementType': {
'type': 'ObjectTypeAnnotation',
'properties': [
{
'name': 'key',
'optional': false,
'typeAnnotation': {
'type': 'GenericObjectTypeAnnotation'
}
}
]
'type': 'GenericObjectTypeAnnotation'
}
},
'params': []
Expand Down
25 changes: 24 additions & 1 deletion packages/react-native-codegen/src/parsers/flow/modules/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,28 @@ function translateTypeAnnotation(
}
}
case 'ObjectTypeAnnotation': {
// if there is any indexer, then it is a dictionary
if (typeAnnotation.indexers) {
const indexers = typeAnnotation.indexers.filter(
member => member.type === 'ObjectTypeIndexer',
);
if (indexers.length > 0) {
// check the property type to prevent developers from using unsupported types
// the return value from `translateTypeAnnotation` is unused
const propertyType = indexers[0].value;
translateTypeAnnotation(
hasteModuleName,
propertyType,
types,
aliasMap,
tryParse,
cxxOnly,
);
// no need to do further checking
return emitObject(nullable);
}
}

const objectTypeAnnotation = {
type: 'ObjectTypeAnnotation',
// $FlowFixMe[missing-type-arg]
Expand Down Expand Up @@ -238,8 +260,9 @@ function translateTypeAnnotation(
case 'MixedTypeAnnotation': {
if (cxxOnly) {
return emitMixed(nullable);
} else {
return emitObject(nullable);
}
// Fallthrough
}
default: {
throw new UnsupportedTypeAnnotationParserError(
Expand Down
27 changes: 13 additions & 14 deletions packages/react-native-codegen/src/parsers/flow/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,20 @@ const {
class FlowParser implements Parser {
typeParameterInstantiation: string = 'TypeParameterInstantiation';

getKeyName(propertyOrIndex: $FlowFixMe, hasteModuleName: string): string {
switch (propertyOrIndex.type) {
case 'ObjectTypeProperty':
return propertyOrIndex.key.name;
case 'ObjectTypeIndexer':
// flow index name is optional
return propertyOrIndex.id?.name ?? 'key';
default:
throw new UnsupportedObjectPropertyTypeAnnotationParserError(
hasteModuleName,
propertyOrIndex,
propertyOrIndex.type,
this.language(),
);
isProperty(property: $FlowFixMe): boolean {
return property.type === 'ObjectTypeProperty';
}

getKeyName(property: $FlowFixMe, hasteModuleName: string): string {
if (!this.isProperty(property)) {
throw new UnsupportedObjectPropertyTypeAnnotationParserError(
hasteModuleName,
property,
property.type,
this.language(),
);
}
return property.key.name;
}

getMaybeEnumMemberType(maybeEnumDeclaration: $FlowFixMe): string {
Expand Down
12 changes: 8 additions & 4 deletions packages/react-native-codegen/src/parsers/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ export interface Parser {
typeParameterInstantiation: string;

/**
* Given a property or an index declaration, it returns the key name.
* @parameter propertyOrIndex: an object containing a property or an index declaration.
* Given a declaration, it returns true if it is a property
*/
isProperty(property: $FlowFixMe): boolean;
/**
* Given a property declaration, it returns the key name.
* @parameter property: an object containing a property declaration.
* @parameter hasteModuleName: a string with the native module name.
* @returns: the key name.
* @throws if propertyOrIndex does not contain a property or an index declaration.
* @throws if property does not contain a property declaration.
*/
getKeyName(propertyOrIndex: $FlowFixMe, hasteModuleName: string): string;
getKeyName(property: $FlowFixMe, hasteModuleName: string): 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
Loading