From 7680bdeb4f96a8092393372a59c77a9d7b729cae Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Fri, 7 Oct 2022 03:21:17 -0700 Subject: [PATCH] Implement filtering for platform specific spec files Summary: This diff helps the library maintainer to keep their spec file platform specific if some specs make no sense in one platform or in the other. We are filtering the spec files when we need to create the Schema. The diff modifies also the call sites in the `scripts` (for iOS) and in the `gradle-plugin` (for Android). It also adds tests for the new functions in the CLI. The change is completely additive and it should not change any pre-existing behaviour. ## Changelog [General][Added] - Add support for platform-specific specs Reviewed By: cortinico Differential Revision: D40008581 fbshipit-source-id: b7fcf6d38f85fe10e4e00002d3c6f2910abdbe35 --- .../combine/__tests__/combine-utils-test.js | 208 ++++++++++++++++++ .../cli/combine/combine-js-to-schema-cli.js | 21 +- .../src/cli/combine/combine-utils.js | 85 +++++++ .../src/parsers/__tests__/utils-test.js | 65 ++++++ .../src/parsers/flow/index.js | 8 +- .../src/parsers/typescript/index.js | 8 +- .../react-native-codegen/src/parsers/utils.js | 23 ++ .../react/tasks/GenerateCodegenSchemaTask.kt | 2 + .../tasks/GenerateCodegenSchemaTaskTest.kt | 2 + .../codegen/generate-artifacts-executor.js | 2 +- 10 files changed, 397 insertions(+), 27 deletions(-) create mode 100644 packages/react-native-codegen/src/cli/combine/__tests__/combine-utils-test.js create mode 100644 packages/react-native-codegen/src/cli/combine/combine-utils.js create mode 100644 packages/react-native-codegen/src/parsers/__tests__/utils-test.js create mode 100644 packages/react-native-codegen/src/parsers/utils.js diff --git a/packages/react-native-codegen/src/cli/combine/__tests__/combine-utils-test.js b/packages/react-native-codegen/src/cli/combine/__tests__/combine-utils-test.js new file mode 100644 index 00000000000000..84acd9ee556251 --- /dev/null +++ b/packages/react-native-codegen/src/cli/combine/__tests__/combine-utils-test.js @@ -0,0 +1,208 @@ +/** + * 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-local + * @format + * @oncall react_native + */ + +'use-strict'; + +const {parseArgs, filterJSFile} = require('../combine-utils.js'); + +describe('parseArgs', () => { + const nodeBin = 'node'; + const combineApp = 'app'; + const schemaJson = 'schema.json'; + const specFile1 = 'NativeSpec.js'; + const specFile2 = 'SpecNativeComponent.js'; + + describe('when no platform provided', () => { + it('returns null platform, schema and fileList', () => { + const {platform, outfile, fileList} = parseArgs([ + nodeBin, + combineApp, + schemaJson, + specFile1, + specFile2, + ]); + + expect(platform).toBeNull(); + expect(outfile).toBe(schemaJson); + expect(fileList).toStrictEqual([specFile1, specFile2]); + }); + }); + + describe('when platform passed with --platform', () => { + it('returns the platform, the schema and the fileList', () => { + const {platform, outfile, fileList} = parseArgs([ + nodeBin, + combineApp, + '--platform', + 'ios', + schemaJson, + specFile1, + specFile2, + ]); + + expect(platform).toBe('ios'); + expect(outfile).toBe(schemaJson); + expect(fileList).toStrictEqual([specFile1, specFile2]); + }); + }); + + describe('when platform passed with -p', () => { + it('returns the platform, the schema and the fileList', () => { + const {platform, outfile, fileList} = parseArgs([ + nodeBin, + combineApp, + '-p', + 'android', + schemaJson, + specFile1, + specFile2, + ]); + + expect(platform).toBe('android'); + expect(outfile).toBe(schemaJson); + expect(fileList).toStrictEqual([specFile1, specFile2]); + }); + }); +}); + +describe('filterJSFile', () => { + describe('When the file is not a Spec file', () => { + it('when no platform is passed, return false', () => { + const file = 'anyJSFile.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + + it('when ios is passed and the file is iOS specific, return false', () => { + const file = 'anyJSFile.ios.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + + it('when android is passed and the file is android specific, return false', () => { + const file = 'anyJSFile.android.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is NativeUIManager', () => { + it('returns false', () => { + const file = 'NativeUIManager.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is NativeSampleTurboModule', () => { + it('returns false', () => { + const file = 'NativeSampleTurboModule.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is a test file', () => { + it('returns false', () => { + const file = '__tests__/NativeModule-test.js'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is a TS type def', () => { + it('returns false', () => { + const file = 'NativeModule.d.ts'; + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is valid and it is platform agnostic', () => { + const file = 'NativeModule.js'; + it('if the platform is null, returns true', () => { + const result = filterJSFile(file); + expect(result).toBeTruthy(); + }); + it('if the platform is ios, returns true', () => { + const result = filterJSFile(file, 'ios'); + expect(result).toBeTruthy(); + }); + it('if the platform is android, returns true', () => { + const result = filterJSFile(file, 'android'); + expect(result).toBeTruthy(); + }); + it('if the platform is windows, returns false', () => { + const result = filterJSFile(file, 'windows'); + expect(result).toBeTruthy(); + }); + }); + + describe('When the file is valid and it is iOS specific', () => { + const file = 'MySampleNativeComponent.ios.js'; + it('if the platform is null, returns false', () => { + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + it('if the platform is ios, returns true', () => { + const result = filterJSFile(file, 'ios'); + expect(result).toBeTruthy(); + }); + it('if the platform is android, returns false', () => { + const result = filterJSFile(file, 'android'); + expect(result).toBeFalsy(); + }); + it('if the platform is windows, returns false', () => { + const result = filterJSFile(file, 'windows'); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is valid and it is Android specific', () => { + const file = 'MySampleNativeComponent.android.js'; + it('if the platform is null, returns false', () => { + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + it('if the platform is ios, returns false', () => { + const result = filterJSFile(file, 'ios'); + expect(result).toBeFalsy(); + }); + it('if the platform is android, returns true', () => { + const result = filterJSFile(file, 'android'); + expect(result).toBeTruthy(); + }); + it('if the platform is windows, returns false', () => { + const result = filterJSFile(file, 'windows'); + expect(result).toBeFalsy(); + }); + }); + + describe('When the file is valid and it is Windows specific', () => { + const file = 'MySampleNativeComponent.windows.js'; + it('if the platform is null, returns false', () => { + const result = filterJSFile(file); + expect(result).toBeFalsy(); + }); + it('if the platform is ios, returns false', () => { + const result = filterJSFile(file, 'ios'); + expect(result).toBeFalsy(); + }); + it('if the platform is android, returns false', () => { + const result = filterJSFile(file, 'android'); + expect(result).toBeFalsy(); + }); + it('if the platform is windows, returns true', () => { + const result = filterJSFile(file, 'windows'); + expect(result).toBeTruthy(); + }); + }); +}); diff --git a/packages/react-native-codegen/src/cli/combine/combine-js-to-schema-cli.js b/packages/react-native-codegen/src/cli/combine/combine-js-to-schema-cli.js index 8831fb25be7a5f..00fadf3dd66107 100644 --- a/packages/react-native-codegen/src/cli/combine/combine-js-to-schema-cli.js +++ b/packages/react-native-codegen/src/cli/combine/combine-js-to-schema-cli.js @@ -14,24 +14,9 @@ const combine = require('./combine-js-to-schema'); const fs = require('fs'); const glob = require('glob'); -const path = require('path'); +const {parseArgs, filterJSFile} = require('./combine-utils'); -const [outfile, ...fileList] = process.argv.slice(2); - -function filterJSFile(file: string) { - return ( - /^(Native.+|.+NativeComponent)/.test(path.basename(file)) && - // NativeUIManager will be deprecated by Fabric UIManager. - // For now, ignore this spec completely because the types are not fully supported. - !file.endsWith('NativeUIManager.js') && - // NativeSampleTurboModule is for demo purpose. It should be added manually to the - // app for now. - !file.endsWith('NativeSampleTurboModule.js') && - !file.includes('__tests') && - // Ignore TypeScript type declaration files. - !file.endsWith('.d.ts') - ); -} +const {platform, outfile, fileList} = parseArgs(process.argv); const allFiles = []; fileList.forEach(file => { @@ -40,7 +25,7 @@ fileList.forEach(file => { .sync(`${file}/**/*.{js,ts,tsx}`, { nodir: true, }) - .filter(filterJSFile); + .filter(element => filterJSFile(element, platform)); allFiles.push(...dirFiles); } else if (filterJSFile(file)) { allFiles.push(file); diff --git a/packages/react-native-codegen/src/cli/combine/combine-utils.js b/packages/react-native-codegen/src/cli/combine/combine-utils.js new file mode 100644 index 00000000000000..5dad6e041b6b99 --- /dev/null +++ b/packages/react-native-codegen/src/cli/combine/combine-utils.js @@ -0,0 +1,85 @@ +/** + * 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-local + * @format + * @oncall react_native + */ + +'use strict'; + +const path = require('path'); + +function parseArgs(args: string[]): { + platform: ?string, + outfile: string, + fileList: string[], +} { + if (args.length > 2 && ['-p', '--platform'].indexOf(args[2]) >= 0) { + const [outfile, ...fileList] = args.slice(4); + return { + platform: args[3], + outfile, + fileList, + }; + } + + const [outfile, ...fileList] = args.slice(2); + return { + platform: null, + outfile, + fileList, + }; +} + +/** + * This function is used by the CLI to decide whether a JS/TS file has to be processed or not by the Codegen. + * Parameters: + * - file: the path to the file + * - currentPlatform: the current platform for which we are creating the specs + * Returns: `true` if the file can be used to generate some code; `false` otherwise + * + */ +function filterJSFile(file: string, currentPlatform: ?string): boolean { + const isSpecFile = /^(Native.+|.+NativeComponent)/.test(path.basename(file)); + const isNotNativeUIManager = !file.endsWith('NativeUIManager.js'); + const isNotNativeSampleTurboModule = !file.endsWith( + 'NativeSampleTurboModule.js', + ); + const isNotTest = !file.includes('__tests'); + const isNotTSTypeDefinition = !file.endsWith('.d.ts'); + + const isValidCandidate = + isSpecFile && + isNotNativeUIManager && + isNotNativeSampleTurboModule && + isNotTest && + isNotTSTypeDefinition; + + const filenameComponents = path.basename(file).split('.'); + const isPlatformAgnostic = filenameComponents.length === 2; + + if (currentPlatform == null) { + // need to accept only files that are platform agnostic + return isValidCandidate && isPlatformAgnostic; + } + + // If a platform is passed, accept both platform agnostic specs... + if (isPlatformAgnostic) { + return isValidCandidate; + } + + // ...and specs that share the same platform as the one passed. + // specfiles must follow the pattern: [.].(js|ts|tsx) + const filePlatform = + filenameComponents.length > 2 ? filenameComponents[1] : 'unknown'; + return isValidCandidate && currentPlatform === filePlatform; +} + +module.exports = { + parseArgs, + filterJSFile, +}; diff --git a/packages/react-native-codegen/src/parsers/__tests__/utils-test.js b/packages/react-native-codegen/src/parsers/__tests__/utils-test.js new file mode 100644 index 00000000000000..72e170cbf7ffab --- /dev/null +++ b/packages/react-native-codegen/src/parsers/__tests__/utils-test.js @@ -0,0 +1,65 @@ +/** + * 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. + * + * @format + * @oncall react_native + */ + +'use strict'; +const {extractNativeModuleName} = require('../utils.js'); + +describe('extractnativeModuleName', () => { + it('return filename when it ends with .js', () => { + const filename = '/some_folder/NativeModule.js'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .ts', () => { + const filename = '/some_folder/NativeModule.ts'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .tsx', () => { + const filename = '/some_folder/NativeModule.tsx'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .android.js', () => { + const filename = '/some_folder/NativeModule.android.js'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .android.ts', () => { + const filename = '/some_folder/NativeModule.android.ts'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .android.tsx', () => { + const filename = '/some_folder/NativeModule.android.tsx'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .ios.js', () => { + const filename = '/some_folder/NativeModule.ios.ts'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .ios.ts', () => { + const filename = '/some_folder/NativeModule.ios.ts'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .ios.tsx', () => { + const filename = '/some_folder/NativeModule.ios.tsx'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); + it('return filename when it ends with .windows.js', () => { + const filename = '/some_folder/NativeModule.windows.js'; + const nativeModuleName = extractNativeModuleName(filename); + expect(nativeModuleName).toBe('NativeModule'); + }); +}); diff --git a/packages/react-native-codegen/src/parsers/flow/index.js b/packages/react-native-codegen/src/parsers/flow/index.js index 23e1a5c6aa6aa1..441ec3a5815319 100644 --- a/packages/react-native-codegen/src/parsers/flow/index.js +++ b/packages/react-native-codegen/src/parsers/flow/index.js @@ -14,7 +14,7 @@ import type {SchemaType} from '../../CodegenSchema.js'; // $FlowFixMe[untyped-import] there's no flowtype flow-parser const flowParser = require('flow-parser'); const fs = require('fs'); -const path = require('path'); +const {extractNativeModuleName} = require('../utils.js'); const {buildComponentSchema} = require('./components'); const {wrapComponentSchema} = require('./components/schema'); const {buildModuleSchema} = require('./modules'); @@ -88,11 +88,11 @@ function buildSchema(contents: string, filename: ?string): SchemaType { if (filename === undefined || filename === null) { throw new Error('Filepath expected while parasing a module'); } - const hasteModuleName = path.basename(filename).replace(/\.js$/, ''); + const nativeModuleName = extractNativeModuleName(filename); const [parsingErrors, tryParse] = createParserErrorCapturer(); const schema = tryParse(() => - buildModuleSchema(hasteModuleName, ast, tryParse), + buildModuleSchema(nativeModuleName, ast, tryParse), ); if (parsingErrors.length > 0) { @@ -112,7 +112,7 @@ function buildSchema(contents: string, filename: ?string): SchemaType { 'When there are no parsing errors, the schema should not be null', ); - return wrapModuleSchema(schema, hasteModuleName); + return wrapModuleSchema(schema, nativeModuleName); } default: return {modules: {}}; diff --git a/packages/react-native-codegen/src/parsers/typescript/index.js b/packages/react-native-codegen/src/parsers/typescript/index.js index 92f41635cc7687..4b585fb623e7ad 100644 --- a/packages/react-native-codegen/src/parsers/typescript/index.js +++ b/packages/react-native-codegen/src/parsers/typescript/index.js @@ -13,7 +13,7 @@ import type {SchemaType} from '../../CodegenSchema.js'; const babelParser = require('@babel/parser'); const fs = require('fs'); -const path = require('path'); +const {extractNativeModuleName} = require('../utils.js'); const {buildComponentSchema} = require('./components'); const {wrapComponentSchema} = require('./components/schema'); const {buildModuleSchema} = require('./modules'); @@ -98,12 +98,12 @@ function buildSchema(contents: string, filename: ?string): SchemaType { if (filename === undefined || filename === null) { throw new Error('Filepath expected while parasing a module'); } - const hasteModuleName = path.basename(filename).replace(/\.tsx?$/, ''); + const nativeModuleName = extractNativeModuleName(filename); const [parsingErrors, tryParse] = createParserErrorCapturer(); const schema = tryParse(() => - buildModuleSchema(hasteModuleName, ast, tryParse), + buildModuleSchema(nativeModuleName, ast, tryParse), ); if (parsingErrors.length > 0) { @@ -123,7 +123,7 @@ function buildSchema(contents: string, filename: ?string): SchemaType { 'When there are no parsing errors, the schema should not be null', ); - return wrapModuleSchema(schema, hasteModuleName); + return wrapModuleSchema(schema, nativeModuleName); } default: return {modules: {}}; diff --git a/packages/react-native-codegen/src/parsers/utils.js b/packages/react-native-codegen/src/parsers/utils.js new file mode 100644 index 00000000000000..1626a68b4f6f8f --- /dev/null +++ b/packages/react-native-codegen/src/parsers/utils.js @@ -0,0 +1,23 @@ +/** + * 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-local + * @format + */ + +'use strict'; + +const path = require('path'); + +function extractNativeModuleName(filename: string): string { + // this should drop everything after the file name. For Example it will drop: + // .android.js, .android.ts, .android.tsx, .ios.js, .ios.ts, .ios.tsx, .js, .ts, .tsx + return path.basename(filename).split('.')[0]; +} + +module.exports = { + extractNativeModuleName, +}; diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTask.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTask.kt index 056185f03e8a8d..89748e1fcbdfeb 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTask.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTask.kt @@ -69,6 +69,8 @@ abstract class GenerateCodegenSchemaTask : Exec() { .get() .asFile .absolutePath, + "--platform", + "android", generatedSchemaFile.get().asFile.absolutePath, jsRootDir.asFile.get().absolutePath, )) diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTaskTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTaskTest.kt index c9508747f53639..6b51434a6055b1 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTaskTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenSchemaTaskTest.kt @@ -147,6 +147,8 @@ class GenerateCodegenSchemaTaskTest { listOf( "--verbose", File(codegenDir, "lib/cli/combine/combine-js-to-schema-cli.js").toString(), + "--platform", + "android", File(outputDir, "schema.json").toString(), jsRootDir.toString(), ), diff --git a/scripts/codegen/generate-artifacts-executor.js b/scripts/codegen/generate-artifacts-executor.js index 6fc398a8bcb9d6..e1e13518d20290 100644 --- a/scripts/codegen/generate-artifacts-executor.js +++ b/scripts/codegen/generate-artifacts-executor.js @@ -319,7 +319,7 @@ function generateSchema(tmpDir, library, node, codegenCliPath) { 'cli', 'combine', 'combine-js-to-schema-cli.js', - )} ${pathToSchema} ${pathToJavaScriptSources}`, + )} --platform ios ${pathToSchema} ${pathToJavaScriptSources}`, ); console.log(`[Codegen] Generated schema: ${pathToSchema}`); return pathToSchema;