Skip to content

Commit

Permalink
refactor module's platform verification (#34961)
Browse files Browse the repository at this point in the history
Summary:
Part of #34872

> [Assigned to youedd] Extract the nameToValidate logic ([Flow](https://github.com/facebook/react-native/blob/main/packages/react-native-codegen/src/parsers/flow/modules/index.js#L704-L713), [TypeScript](https://github.com/facebook/react-native/blob/f353119113d6fc85491765ba1e90ac83cb00fd61/packages/react-native-codegen/src/parsers/typescript/modules/index.js#L738-L747)) into a shared function into the parser/utils.js file. Notice that you may need a callback to update the cxx boolean.

## 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
-->

[Changed] [Internal] - refactor parser module platform verification

Pull Request resolved: #34961

Test Plan:
`yarn jest react-native-codegen`

![image](https://user-images.githubusercontent.com/19575877/195425654-46f9e560-efd3-4945-b913-c6d9f6bb7ec2.png)

Reviewed By: cipolleschi

Differential Revision: D40319245

Pulled By: skinsshark

fbshipit-source-id: bc36cdcfa06047e1acee0d638f5756b6462d76d1
  • Loading branch information
youedd authored and facebook-github-bot committed Oct 17, 2022
1 parent a63a4fa commit 633498f
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 26 deletions.
66 changes: 66 additions & 0 deletions packages/react-native-codegen/src/parsers/__tests__/utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
const {
extractNativeModuleName,
createParserErrorCapturer,
verifyPlatforms,
visit,
} = require('../utils.js');
const {ParserError} = require('../errors');
Expand Down Expand Up @@ -117,6 +118,71 @@ describe('createParserErrorCapturer', () => {
});
});

describe('verifyPlatforms', () => {
it('exclude android given an iOS only module', () => {
let result = verifyPlatforms('NativeSampleTurboModule', [
'SampleTurboModuleIOS',
]);

expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['android']);

result = verifyPlatforms('NativeSampleTurboModuleIOS', [
'SampleTurboModule',
]);
expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['android']);

result = verifyPlatforms('NativeSampleTurboModuleIOS', [
'SampleTurboModuleIOS',
]);
expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['android']);
});

it('exclude iOS given an android only module', () => {
let result = verifyPlatforms('NativeSampleTurboModule', [
'SampleTurboModuleAndroid',
]);

expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['iOS']);

result = verifyPlatforms('NativeSampleTurboModuleAndroid', [
'SampleTurboModule',
]);
expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['iOS']);

result = verifyPlatforms('NativeSampleTurboModuleAndroid', [
'SampleTurboModuleAndroid',
]);
expect(result.cxxOnly).toBe(false);
expect(result.excludedPlatforms).toEqual(['iOS']);
});

it('exclude iOS and android given a Cxx only module', () => {
let result = verifyPlatforms('NativeSampleTurboModule', [
'SampleTurboModuleCxx',
]);

expect(result.cxxOnly).toBe(true);
expect(result.excludedPlatforms).toEqual(['iOS', 'android']);

result = verifyPlatforms('NativeSampleTurboModuleCxx', [
'SampleTurboModule',
]);
expect(result.cxxOnly).toBe(true);
expect(result.excludedPlatforms).toEqual(['iOS', 'android']);

result = verifyPlatforms('NativeSampleTurboModuleCxx', [
'SampleTurboModuleCxx',
]);
expect(result.cxxOnly).toBe(true);
expect(result.excludedPlatforms).toEqual(['iOS', 'android']);
});
});

describe('visit', () => {
describe('when the astNode is null', () => {
it("doesn't call the visitor function", () => {
Expand Down
18 changes: 5 additions & 13 deletions packages/react-native-codegen/src/parsers/flow/modules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const {
UnsupportedObjectPropertyValueTypeAnnotationParserError,
IncorrectModuleRegistryCallArgumentTypeParserError,
} = require('../../errors.js');
const {verifyPlatforms} = require('../../utils');

const {
throwIfModuleInterfaceNotFound,
Expand Down Expand Up @@ -691,19 +692,10 @@ function buildModuleSchema(
// Eventually this should be made explicit in the Flow type itself.
// Also check the hasteModuleName for platform suffix.
// Note: this shape is consistent with ComponentSchema.
let cxxOnly = false;
const excludedPlatforms = [];
const namesToValidate = [...moduleNames, hasteModuleName];
namesToValidate.forEach(name => {
if (name.endsWith('Android')) {
excludedPlatforms.push('iOS');
} else if (name.endsWith('IOS')) {
excludedPlatforms.push('android');
} else if (name.endsWith('Cxx')) {
cxxOnly = true;
excludedPlatforms.push('iOS', 'android');
}
});
const {cxxOnly, excludedPlatforms} = verifyPlatforms(
hasteModuleName,
moduleNames,
);

// $FlowFixMe[missing-type-arg]
return (moduleSpec.body.properties: $ReadOnlyArray<$FlowFixMe>)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const {
UnsupportedObjectPropertyValueTypeAnnotationParserError,
IncorrectModuleRegistryCallArgumentTypeParserError,
} = require('../../errors.js');
const {verifyPlatforms} = require('../../utils');

const {
throwIfUntypedModule,
Expand Down Expand Up @@ -705,19 +706,10 @@ function buildModuleSchema(
// Eventually this should be made explicit in the Flow type itself.
// Also check the hasteModuleName for platform suffix.
// Note: this shape is consistent with ComponentSchema.
let cxxOnly = false;
const excludedPlatforms = [];
const namesToValidate = [...moduleNames, hasteModuleName];
namesToValidate.forEach(name => {
if (name.endsWith('Android')) {
excludedPlatforms.push('iOS');
} else if (name.endsWith('IOS')) {
excludedPlatforms.push('android');
} else if (name.endsWith('Cxx')) {
cxxOnly = true;
excludedPlatforms.push('iOS', 'android');
}
});
const {cxxOnly, excludedPlatforms} = verifyPlatforms(
hasteModuleName,
moduleNames,
);

// $FlowFixMe[missing-type-arg]
return (moduleSpec.body.body: $ReadOnlyArray<$FlowFixMe>)
Expand Down
37 changes: 37 additions & 0 deletions packages/react-native-codegen/src/parsers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,42 @@ function createParserErrorCapturer(): [
return [errors, guard];
}

function verifyPlatforms(
hasteModuleName: string,
moduleNames: string[],
): $ReadOnly<{
cxxOnly: boolean,
excludedPlatforms: Array<'iOS' | 'android'>,
}> {
let cxxOnly = false;
const excludedPlatforms = new Set<'iOS' | 'android'>();
const namesToValidate = [...moduleNames, hasteModuleName];

namesToValidate.forEach(name => {
if (name.endsWith('Android')) {
excludedPlatforms.add('iOS');
return;
}

if (name.endsWith('IOS')) {
excludedPlatforms.add('android');
return;
}

if (name.endsWith('Cxx')) {
cxxOnly = true;
excludedPlatforms.add('iOS');
excludedPlatforms.add('android');
return;
}
});

return {
cxxOnly,
excludedPlatforms: Array.from(excludedPlatforms),
};
}

// TODO(T108222691): Use flow-types for @babel/parser
function visit(
astNode: $FlowFixMe,
Expand Down Expand Up @@ -86,5 +122,6 @@ function visit(
module.exports = {
extractNativeModuleName,
createParserErrorCapturer,
verifyPlatforms,
visit,
};

0 comments on commit 633498f

Please sign in to comment.