From 6718500eaaeb92b8a74320dcee961ac96f6f12fa Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Wed, 4 May 2022 04:04:10 -0700 Subject: [PATCH] Support the `type == all` properly Summary: This Diff introduces some changes in the CodeGen to properly generate the types in the right folder. ## Issue The codegen on iOS defines the output folder once, before creating the generated code. When the code we have to generate is just a TurboModule (TM) or a Fabric Component (FC), this mechanism works properly. However, if a library has to generate both TM and FC, actually using the library type `all`, all the code is generated using the TurboModules' output folder. (**Note:** Android only works in this way) This generates invalid code because all the FC's `#import` directives assumes that the code is generated in the FC output path which, in this case, is not. ## Solution The adopted solution moves the responsibility to decide where the files has to be generated to the CodeGen step instead of in the preparatory phases. The two paths are precomputed in the `generate-artifacts.js` script (the entry point for the CodeGen) and they are passed to all the scripts that requires them. Once they reach the `RNCodegen.js` file, the generators creates the files and save them in the proper paths. ## Changelog [iOS][Changed] - CodeGen now supports the `"all"` library type. Reviewed By: cortinico, dmitryrykun Differential Revision: D35820848 fbshipit-source-id: ce7f5393936e2ae17f8b2c970f6a011d27f641f2 --- .../src/generators/RNCodegen.js | 98 ++++++++++++------- .../generators/__test_fixtures__/fixtures.js | 82 ++++++++++++++++ .../generators/__tests__/RNCodegen-test.js | 69 +++++++++++++ scripts/generate-artifacts.js | 64 ++++++++---- scripts/generate-specs-cli.js | 70 ++++++++++--- 5 files changed, 317 insertions(+), 66 deletions(-) create mode 100644 packages/react-native-codegen/src/generators/__test_fixtures__/fixtures.js create mode 100644 packages/react-native-codegen/src/generators/__tests__/RNCodegen-test.js diff --git a/packages/react-native-codegen/src/generators/RNCodegen.js b/packages/react-native-codegen/src/generators/RNCodegen.js index 6444b2cfa87a18..d9487e8832fe5f 100644 --- a/packages/react-native-codegen/src/generators/RNCodegen.js +++ b/packages/react-native-codegen/src/generators/RNCodegen.js @@ -48,6 +48,8 @@ type LibraryOptions = $ReadOnly<{ outputDirectory: string, packageName?: string, // Some platforms have a notion of package, which should be configurable. assumeNonnull: boolean, + componentsOutputDir?: string, // optional for backward compatibility + modulesOutputDir?: string, // optional for backward compatibility }>; type SchemasOptions = $ReadOnly<{ @@ -134,36 +136,39 @@ const SCHEMAS_GENERATORS = { ], }; -function writeMapToFiles(map: Map, outputDir: string) { +type CodeGenFile = { + name: string, + content: string, + outputDir: string, +}; + +function writeMapToFiles(map: Array) { let success = true; - map.forEach((contents: string, fileName: string) => { + map.forEach(file => { try { - const location = path.join(outputDir, fileName); + const location = path.join(file.outputDir, file.name); const dirName = path.dirname(location); if (!fs.existsSync(dirName)) { fs.mkdirSync(dirName, {recursive: true}); } - fs.writeFileSync(location, contents); + fs.writeFileSync(location, file.content); } catch (error) { success = false; - console.error(`Failed to write ${fileName} to ${outputDir}`, error); + console.error(`Failed to write ${file.name} to ${file.outputDir}`, error); } }); return success; } -function checkFilesForChanges( - map: Map, - outputDir: string, -): boolean { +function checkFilesForChanges(generated: Array): boolean { let hasChanged = false; - map.forEach((contents: string, fileName: string) => { - const location = path.join(outputDir, fileName); + generated.forEach(file => { + const location = path.join(file.outputDir, file.name); const currentContents = fs.readFileSync(location, 'utf8'); - if (currentContents !== contents) { - console.error(`- ${fileName} has changed`); + if (currentContents !== file.content) { + console.error(`- ${file.name} has changed`); hasChanged = true; } @@ -172,6 +177,16 @@ function checkFilesForChanges( return !hasChanged; } +function checkOrWriteFiles( + generatedFiles: Array, + test: void | boolean, +): boolean { + if (test === true) { + return checkFilesForChanges(generatedFiles); + } + return writeMapToFiles(generatedFiles); +} + module.exports = { generate( { @@ -180,27 +195,42 @@ module.exports = { outputDirectory, packageName, assumeNonnull, + componentsOutputDir, + modulesOutputDir, }: LibraryOptions, {generators, test}: LibraryConfig, ): boolean { schemaValidator.validate(schema); - const generatedFiles = []; + const outputFoldersForGenerators = { + componentsIOS: componentsOutputDir ?? outputDirectory, // fallback for backward compatibility + modulesIOS: modulesOutputDir ?? outputDirectory, // fallback for backward compatibility + descriptors: outputDirectory, + events: outputDirectory, + props: outputDirectory, + componentsAndroid: outputDirectory, + modulesAndroid: outputDirectory, + modulesCxx: outputDirectory, + tests: outputDirectory, + 'shadow-nodes': outputDirectory, + }; + + const generatedFiles: Array = []; + for (const name of generators) { for (const generator of LIBRARY_GENERATORS[name]) { - generatedFiles.push( - ...generator(libraryName, schema, packageName, assumeNonnull), + generator(libraryName, schema, packageName, assumeNonnull).forEach( + (contents: string, fileName: string) => { + generatedFiles.push({ + name: fileName, + content: contents, + outputDir: outputFoldersForGenerators[name], + }); + }, ); } } - - const filesToUpdate = new Map([...generatedFiles]); - - if (test === true) { - return checkFilesForChanges(filesToUpdate, outputDirectory); - } - - return writeMapToFiles(filesToUpdate, outputDirectory); + return checkOrWriteFiles(generatedFiles, test); }, generateFromSchemas( {schemas, outputDirectory}: SchemasOptions, @@ -210,20 +240,20 @@ module.exports = { schemaValidator.validate(schemas[libraryName]), ); - const generatedFiles = []; + const generatedFiles: Array = []; + for (const name of generators) { for (const generator of SCHEMAS_GENERATORS[name]) { - generatedFiles.push(...generator(schemas)); + generator(schemas).forEach((contents: string, fileName: string) => { + generatedFiles.push({ + name: fileName, + content: contents, + outputDir: outputDirectory, + }); + }); } } - - const filesToUpdate = new Map([...generatedFiles]); - - if (test === true) { - return checkFilesForChanges(filesToUpdate, outputDirectory); - } - - return writeMapToFiles(filesToUpdate, outputDirectory); + return checkOrWriteFiles(generatedFiles, test); }, generateViewConfig({libraryName, schema}: LibraryOptions): string { schemaValidator.validate(schema); diff --git a/packages/react-native-codegen/src/generators/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/generators/__test_fixtures__/fixtures.js new file mode 100644 index 00000000000000..b8659768393350 --- /dev/null +++ b/packages/react-native-codegen/src/generators/__test_fixtures__/fixtures.js @@ -0,0 +1,82 @@ +/** + * 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'; + +import type {SchemaType} from '../../CodegenSchema.js'; + +const SCHEMA_WITH_TM_AND_FC: SchemaType = { + modules: { + ColoredView: { + type: 'Component', + components: { + ColoredView: { + extendsProps: [ + { + type: 'ReactNativeBuiltInType', + knownTypeName: 'ReactNativeCoreViewProps', + }, + ], + events: [], + props: [ + { + name: 'color', + optional: false, + typeAnnotation: { + type: 'StringTypeAnnotation', + default: null, + }, + }, + ], + commands: [], + }, + }, + }, + NativeCalculator: { + type: 'NativeModule', + aliases: {}, + spec: { + properties: [ + { + name: 'add', + optional: false, + typeAnnotation: { + type: 'FunctionTypeAnnotation', + returnTypeAnnotation: { + type: 'PromiseTypeAnnotation', + }, + params: [ + { + name: 'a', + optional: false, + typeAnnotation: { + type: 'NumberTypeAnnotation', + }, + }, + { + name: 'b', + optional: false, + typeAnnotation: { + type: 'NumberTypeAnnotation', + }, + }, + ], + }, + }, + ], + }, + moduleNames: ['Calculator'], + }, + }, +}; + +module.exports = { + all: SCHEMA_WITH_TM_AND_FC, +}; diff --git a/packages/react-native-codegen/src/generators/__tests__/RNCodegen-test.js b/packages/react-native-codegen/src/generators/__tests__/RNCodegen-test.js new file mode 100644 index 00000000000000..a529379d72eecf --- /dev/null +++ b/packages/react-native-codegen/src/generators/__tests__/RNCodegen-test.js @@ -0,0 +1,69 @@ +/** + * 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. + * + * @emails oncall+react_native + * @flow strict-local + * @format + */ + +'use strict'; + +const rnCodegen = require('../RNCodegen.js'); +const fixture = require('../__test_fixtures__/fixtures.js'); +const path = require('path'); + +const invalidDirectory = 'invalid/'; +const packageName = 'na'; +const componentsOutputDir = 'react/renderer/components/library'; +const modulesOutputDir = 'library'; + +describe('RNCodegen.generate', () => { + it('when type `all`', () => { + const expectedPaths = { + 'library.h': modulesOutputDir, + 'library-generated.mm': modulesOutputDir, + 'ShadowNodes.h': componentsOutputDir, + 'ShadowNodes.cpp': componentsOutputDir, + 'Props.h': componentsOutputDir, + 'Props.cpp': componentsOutputDir, + 'RCTComponentViewHelpers.h': componentsOutputDir, + 'EventEmitters.h': componentsOutputDir, + 'EventEmitters.cpp': componentsOutputDir, + 'ComponentDescriptors.h': componentsOutputDir, + }; + + jest.mock('fs', () => ({ + existsSync: location => { + return true; + }, + writeFileSync: (location, content) => { + let receivedDir = path.dirname(location); + let receivedBasename = path.basename(location); + + let expectedPath = expectedPaths[receivedBasename]; + expect(receivedDir).toEqual(expectedPath); + }, + })); + + const res = rnCodegen.generate( + { + libraryName: 'library', + schema: fixture.all, + outputDirectory: invalidDirectory, + packageName: packageName, + assumeNonnull: true, + componentsOutputDir: componentsOutputDir, + modulesOutputDir: modulesOutputDir, + }, + { + generators: ['componentsIOS', 'modulesIOS'], + test: false, + }, + ); + + expect(res).toBeTruthy(); + }); +}); diff --git a/scripts/generate-artifacts.js b/scripts/generate-artifacts.js index 18d3a50c69e402..2feb2d9e2bcf40 100644 --- a/scripts/generate-artifacts.js +++ b/scripts/generate-artifacts.js @@ -81,6 +81,10 @@ function executeNodeScript(script) { execSync(`${NODE} ${script}`); } +function isDirEmpty(dirPath) { + return fs.readdirSync(dirPath).length === 0; +} + function main(appRootDir, outputPath) { if (appRootDir == null) { console.error('Missing path to React Native application'); @@ -233,14 +237,19 @@ function main(appRootDir, outputPath) { library.libraryPath, library.config.jsSrcsDir, ); - const pathToOutputDirIOS = path.join( - iosOutputDir, - library.config.type === 'components' - ? 'react/renderer/components' - : './', - library.config.name, - ); - const pathToTempOutputDir = path.join(tmpDir, 'out'); + function composePath(intermediate) { + return path.join(iosOutputDir, intermediate, library.config.name); + } + + const outputDirsIOS = { + components: composePath('react/renderer/components'), + nativeModules: composePath('./'), + }; + + const tempOutputDirs = { + components: path.join(tmpDir, 'out', 'components'), + nativeModules: path.join(tmpDir, 'out', 'nativeModules'), + }; console.log(`\n\n[Codegen] >>>>> Processing ${library.config.name}`); // Generate one schema for the entire library... @@ -259,21 +268,38 @@ function main(appRootDir, outputPath) { const libraryTypeArg = library.config.type ? `--libraryType ${library.config.type}` : ''; - fs.mkdirSync(pathToTempOutputDir, {recursive: true}); + + Object.entries(tempOutputDirs).forEach(([type, dirPath]) => { + fs.mkdirSync(dirPath, {recursive: true}); + }); + + const deprecated_outputDir = + library.config.type === 'components' + ? tempOutputDirs.components + : tempOutputDirs.nativeModules; + executeNodeScript( - `${path.join( - RN_ROOT, - 'scripts', - 'generate-specs-cli.js', - )} --platform ios --schemaPath ${pathToSchema} --outputDir ${pathToTempOutputDir} --libraryName ${ - library.config.name - } ${libraryTypeArg}`, + `${path.join(RN_ROOT, 'scripts', 'generate-specs-cli.js')} \ + --platform ios \ + --schemaPath ${pathToSchema} \ + --outputDir ${deprecated_outputDir} \ + --componentsOutputDir ${tempOutputDirs.components} \ + --modulesOutputDirs ${tempOutputDirs.nativeModules} \ + --libraryName ${library.config.name} \ + ${libraryTypeArg}`, ); // Finally, copy artifacts to the final output directory. - fs.mkdirSync(pathToOutputDirIOS, {recursive: true}); - execSync(`cp -R ${pathToTempOutputDir}/* ${pathToOutputDirIOS}`); - console.log(`[Codegen] Generated artifacts: ${pathToOutputDirIOS}`); + Object.entries(outputDirsIOS).forEach(([type, dirPath]) => { + const outDir = tempOutputDirs[type]; + if (isDirEmpty(outDir)) { + return; // cp fails if we try to copy something empty. + } + + fs.mkdirSync(dirPath, {recursive: true}); + execSync(`cp -R ${outDir}/* ${dirPath}`); + console.log(`[Codegen] Generated artifacts: ${dirPath}`); + }); // Filter the react native core library out. // In the future, core library and third party library should diff --git a/scripts/generate-specs-cli.js b/scripts/generate-specs-cli.js index 550b363f371c92..91c6039d81f7d0 100644 --- a/scripts/generate-specs-cli.js +++ b/scripts/generate-specs-cli.js @@ -36,7 +36,7 @@ const argv = yargs .option('o', { alias: 'outputDir', describe: - 'Path to directory where native code source files should be saved.', + 'DEPRECATED - Path to directory where native code source files should be saved.', }) .option('n', { alias: 'libraryName', @@ -53,6 +53,14 @@ const argv = yargs describe: 'all, components, or modules.', default: 'all', }) + .option('c', { + alias: 'componentsOutputDir', + describe: 'Output directory for the codeGen for Fabric Components', + }) + .option('m', { + alias: 'modulesOutputDirs', + describe: 'Output directory for the codeGen for TurboModules', + }) .usage('Usage: $0 ') .demandOption( ['platform', 'schemaPath', 'outputDir'], @@ -74,35 +82,67 @@ const GENERATORS = { }, }; -function generateSpec( - platform, - schemaPath, +function deprecated_createOutputDirectoryIfNeeded( outputDirectory, libraryName, - packageName, - libraryType, ) { + if (!outputDirectory) { + outputDirectory = path.resolve(__dirname, '..', 'Libraries', libraryName); + } + mkdirp.sync(outputDirectory); +} + +function createFolderIfDefined(folder) { + if (folder) { + mkdirp.sync(folder); + } +} + +/** + * This function read a JSON schema from a path and parses it. + * It throws if the schema don't exists or it can't be parsed. + * + * @parameter schemaPath: the path to the schema + * @return a valid schema + * @throw an Error if the schema doesn't exists in a given path or if it can't be parsed. + */ +function readAndParseSchema(schemaPath) { const schemaText = fs.readFileSync(schemaPath, 'utf-8'); if (schemaText == null) { throw new Error(`Can't find schema at ${schemaPath}`); } - if (!outputDirectory) { - outputDirectory = path.resolve(__dirname, '..', 'Libraries', libraryName); - } - mkdirp.sync(outputDirectory); - - let schema; try { - schema = JSON.parse(schemaText); + return JSON.parse(schemaText); } catch (err) { throw new Error(`Can't parse schema to JSON. ${schemaPath}`); } +} +function validateLibraryType(libraryType) { if (GENERATORS[libraryType] == null) { throw new Error(`Invalid library type. ${libraryType}`); } +} + +function generateSpec( + platform, + schemaPath, + outputDirectory, + libraryName, + packageName, + libraryType, + componentsOutputDir, + modulesOutputDirs, +) { + validateLibraryType(libraryType); + + let schema = readAndParseSchema(schemaPath); + + createFolderIfDefined(componentsOutputDir); + createFolderIfDefined(modulesOutputDirs); + deprecated_createOutputDirectoryIfNeeded(outputDirectory, libraryName); RNCodegen.generate( { @@ -110,6 +150,8 @@ function generateSpec( schema, outputDirectory, packageName, + componentsOutputDir, + modulesOutputDirs, }, { generators: GENERATORS[libraryType][platform], @@ -140,6 +182,8 @@ function main() { argv.libraryName, argv.javaPackageName, argv.libraryType, + argv.componentsOutputDir, + argv.modulesOutputDirs, ); }