From 2a7b31ebdbb462655b85e7d54e968320febb2d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Klocek?= Date: Mon, 31 Aug 2020 14:06:39 +0200 Subject: [PATCH] [expo-cli] Generate-module fixes and improvements (#2548) * [generate-module] Fix regex to support 'next' and 'latest' NPM tags * [generate-module] Fix prompt iOS default name * [generate-module] Fix naming without 'Expo' prefix * [generate-module] Autodetect template java package name * Update Changelog --- CHANGELOG.md | 2 + .../generate-module/configureModule.ts | 144 ++++++++++++++---- .../commands/generate-module/fetchTemplate.ts | 2 +- .../generate-module/promptQuestionsAsync.ts | 2 +- 4 files changed, 115 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4fb5eaf62..3bcc1f8059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,11 +18,13 @@ This is the log of notable changes to Expo CLI and related packages. - [expo-cli] expo publish - Log bundles after building ([#2527](https://github.com/expo/expo-cli/pull/2527) by [@EvanBacon](https://github.com/EvanBacon)) - [expo-cli] expo eas:build - Add --skip-credentials-check option ([#2442](https://github.com/expo/expo-cli/pull/2442) by [@satya164](https://github.com/satya164)) - [expo-cli] Add a `eas:build:init` command ([#2443](https://github.com/expo/expo-cli/pull/2443) by [@satya164](https://github.com/satya164)) +- [expo-cli] expo generate-module - Support for templates with Android native unit tests ([#2548](https://github.com/expo/expo-cli/pull/2548) by [@barthap](https://github.com/barthap)) ### 🐛 Bug fixes - [expo-cli] expo upload:android - fix `--use-submission-service` not resulting in non-zero exit code when upload fails ([#2530](https://github.com/expo/expo-cli/pull/2530) by [@mymattcarroll](https://github.com/mymattcarroll)) - [expo-cli] Fix `generate-module` to support latest `expo-module-template` ([#2510](https://github.com/expo/expo-cli/pull/2510) by [@barthap](https://github.com/barthap)) +- [expo-cli] Fix `generate-module` filename generation for modules without `expo-` prefix ([#2548](https://github.com/expo/expo-cli/pull/2548) by [@barthap](https://github.com/barthap)) - [image-utils] Fix setting background color when calling `Jimp.resize` ([#2535](https://github.com/expo/expo-cli/pull/2535) by [@cruzach](https://github.com/cruzach)) ### 📦 Packages updated diff --git a/packages/expo-cli/src/commands/generate-module/configureModule.ts b/packages/expo-cli/src/commands/generate-module/configureModule.ts index 49adacd664..8f26392cf4 100644 --- a/packages/expo-cli/src/commands/generate-module/configureModule.ts +++ b/packages/expo-cli/src/commands/generate-module/configureModule.ts @@ -5,6 +5,19 @@ import path from 'path'; import CommandError from '../../CommandError'; import { ModuleConfiguration } from './ModuleConfiguration'; +// TODO (barthap): If ever updated to TS 4.0, change this to: +// type PreparedPrefixes = [nameWithExpoPrefix: string, nameWithoutExpoPrefix: string]; +type PreparedPrefixes = [string, string]; + +/** + * prepares _Expo_ prefixes for specified name + * @param name module name, e.g. JS package name + * @param prefix prefix to prepare with, defaults to _Expo_ + * @returns tuple `[nameWithPrefix: string, nameWithoutPrefix: string]` + */ +const preparePrefixes = (name: string, prefix: string = 'Expo'): PreparedPrefixes => + name.startsWith(prefix) ? [name, name.substr(prefix.length)] : [`${prefix}${name}`, name]; + const asyncForEach = async ( array: T[], callback: (value: T, index: number, array: T[]) => Promise @@ -14,11 +27,13 @@ const asyncForEach = async ( } }; +/** + * Removes specified files. If one file doesn't exist already, skips it + * @param directoryPath directory containing files to remove + * @param filenames array of filenames to remove + */ async function removeFiles(directoryPath: string, filenames: string[]) { - await asyncForEach( - filenames, - async filename => await fse.remove(path.resolve(directoryPath, filename)) - ); + await Promise.all(filenames.map(filename => fse.remove(path.resolve(directoryPath, filename)))); } /** @@ -58,13 +73,13 @@ const replaceContents = async ( directoryPath: string, replaceFunction: (contentOfSingleFile: string) => string ) => { - for (const file of walkSync(directoryPath, { nodir: true })) { - replaceContent(file.path, replaceFunction); - } + await Promise.all( + walkSync(directoryPath, { nodir: true }).map(file => replaceContent(file.path, replaceFunction)) + ); }; /** - * Replaces content in file + * Replaces content in file. Does nothing if the file doesn't exist * @param filePath - provided file * @param replaceFunction - function that converts current content into something different */ @@ -72,6 +87,10 @@ const replaceContent = async ( filePath: string, replaceFunction: (contentOfSingleFile: string) => string ) => { + if (!fse.existsSync(filePath)) { + return; + } + const content = await fse.readFile(filePath, 'utf8'); const newContent = replaceFunction(content); if (newContent !== content) { @@ -154,10 +173,12 @@ async function configureIOS( * Gets path to Android source base dir: android/src/main/[java|kotlin] * Defaults to Java path if both exist * @param androidPath path do module android/ directory + * @param flavor package flavor e.g main, test. Defaults to main * @throws INVALID_TEMPLATE if none exist + * @returns path to flavor source base directory */ -function findAndroidSourceDir(androidPath: string) { - const androidSrcPathBase = path.join(androidPath, 'src', 'main'); +function findAndroidSourceDir(androidPath: string, flavor: string = 'main'): string { + const androidSrcPathBase = path.join(androidPath, 'src', flavor); const javaExists = fse.pathExistsSync(path.join(androidSrcPathBase, 'java')); const kotlinExists = fse.pathExistsSync(path.join(androidSrcPathBase, 'kotlin')); @@ -172,6 +193,35 @@ function findAndroidSourceDir(androidPath: string) { return path.join(androidSrcPathBase, javaExists ? 'java' : 'kotlin'); } +/** + * Finds java package name based on directory structure + * @param flavorSrcPath Path to source base directory: e.g. android/src/main/java + * @returns java package name + */ +function findTemplateAndroidPackage(flavorSrcPath: string) { + const srcFiles = walkSync(flavorSrcPath, { + filter: item => item.path.endsWith('.kt') || item.path.endsWith('.java'), + nodir: true, + traverseAll: true, + }); + + if (srcFiles.length === 0) { + throw new CommandError('INVALID TEMPLATE', 'No Android source files found in the template'); + } + + // srcFiles[0] will always be at the most top-level of the package structure + const packageDirNames = path.relative(flavorSrcPath, srcFiles[0].path).split('/').slice(0, -1); + + if (packageDirNames.length === 0) { + throw new CommandError( + 'INVALID TEMPLATE', + 'Template Android sources must be within a package.' + ); + } + + return packageDirNames.join('.'); +} + /** * Prepares Android part, mainly by renaming all files and template words in files * Sets all versions in Gradle to 1.0.0 @@ -183,9 +233,12 @@ async function configureAndroid( { javaPackage, jsPackageName, viewManager }: ModuleConfiguration ) { const androidPath = path.join(modulePath, 'android'); + const [, moduleName] = preparePrefixes(jsPackageName, 'Expo'); + const androidSrcPath = findAndroidSourceDir(androidPath); + const templateJavaPackage = findTemplateAndroidPackage(androidSrcPath); - const sourceFilesPath = path.join(androidSrcPath, 'expo', 'modules', 'template'); + const sourceFilesPath = path.join(androidSrcPath, ...templateJavaPackage.split('.')); const destinationFilesPath = path.join(androidSrcPath, ...javaPackage.split('.')); // remove ViewManager from template @@ -204,13 +257,40 @@ async function configureAndroid( // Remove leaf directory content await fse.remove(sourceFilesPath); - // Cleanup all empty subdirs up to provided rootDir - await removeUponEmptyOrOnlyEmptySubdirs(path.join(androidSrcPath, 'expo')); + // Cleanup all empty subdirs up to template package root dir + await removeUponEmptyOrOnlyEmptySubdirs( + path.join(androidSrcPath, templateJavaPackage.split('.')[0]) + ); + + // prepare tests + if (fse.existsSync(path.resolve(androidPath, 'src', 'test'))) { + const androidTestPath = findAndroidSourceDir(androidPath, 'test'); + const templateTestPackage = findTemplateAndroidPackage(androidTestPath); + const testSourcePath = path.join(androidTestPath, ...templateTestPackage.split('.')); + const testDestinationPath = path.join(androidTestPath, ...javaPackage.split('.')); - const moduleName = jsPackageName.startsWith('Expo') ? jsPackageName.substring(4) : jsPackageName; + await fse.mkdirp(testDestinationPath); + await fse.copy(testSourcePath, testDestinationPath); + await fse.remove(testSourcePath); + await removeUponEmptyOrOnlyEmptySubdirs( + path.join(androidTestPath, templateTestPackage.split('.')[0]) + ); + + await replaceContents(testDestinationPath, singleFileContent => + singleFileContent.replace(new RegExp(templateTestPackage, 'g'), javaPackage) + ); + + await renameFilesWithExtensions( + testDestinationPath, + ['.kt', '.java'], + [{ from: 'ModuleTemplateModuleTest', to: `${moduleName}ModuleTest` }] + ); + } + + // Replace contents of destination files await replaceContents(androidPath, singleFileContent => singleFileContent - .replace(/expo\.modules\.template/g, javaPackage) + .replace(new RegExp(templateJavaPackage, 'g'), javaPackage) .replace(/ModuleTemplate/g, moduleName) .replace(/ExpoModuleTemplate/g, jsPackageName) ); @@ -222,7 +302,7 @@ async function configureAndroid( ); await renameFilesWithExtensions( destinationFilesPath, - ['.kt'], + ['.kt', '.java'], [ { from: 'ModuleTemplateModule', to: `${moduleName}Module` }, { from: 'ModuleTemplatePackage', to: `${moduleName}Package` }, @@ -241,9 +321,8 @@ async function configureTS( modulePath: string, { jsPackageName, viewManager }: ModuleConfiguration ) { - const moduleNameWithoutExpoPrefix = jsPackageName.startsWith('Expo') - ? jsPackageName.substr(4) - : 'Unimodule'; + const [moduleNameWithExpoPrefix, moduleName] = preparePrefixes(jsPackageName); + const tsPath = path.join(modulePath, 'src'); // remove View Manager from template @@ -261,26 +340,26 @@ async function configureTS( await renameFilesWithExtensions( path.join(tsPath, '__tests__'), ['.ts'], - [{ from: 'ModuleTemplate-test', to: `${moduleNameWithoutExpoPrefix}-test` }] + [{ from: 'ModuleTemplate-test', to: `${moduleName}-test` }] ); await renameFilesWithExtensions( tsPath, ['.tsx', '.ts'], [ - { from: 'ExpoModuleTemplateView', to: `${jsPackageName}View` }, - { from: 'ExpoModuleTemplateNativeView', to: `${jsPackageName}NativeView` }, - { from: 'ExpoModuleTemplateNativeView.web', to: `${jsPackageName}NativeView.web` }, - { from: 'ExpoModuleTemplate', to: jsPackageName }, - { from: 'ExpoModuleTemplate.web', to: `${jsPackageName}.web` }, - { from: 'ModuleTemplate', to: moduleNameWithoutExpoPrefix }, - { from: 'ModuleTemplate.types', to: `${moduleNameWithoutExpoPrefix}.types` }, + { from: 'ExpoModuleTemplateView', to: `${moduleNameWithExpoPrefix}View` }, + { from: 'ExpoModuleTemplateNativeView', to: `${moduleNameWithExpoPrefix}NativeView` }, + { from: 'ExpoModuleTemplateNativeView.web', to: `${moduleNameWithExpoPrefix}NativeView.web` }, + { from: 'ExpoModuleTemplate', to: moduleNameWithExpoPrefix }, + { from: 'ExpoModuleTemplate.web', to: `${moduleNameWithExpoPrefix}.web` }, + { from: 'ModuleTemplate', to: moduleName }, + { from: 'ModuleTemplate.types', to: `${moduleName}.types` }, ] ); await replaceContents(tsPath, singleFileContent => singleFileContent - .replace(/ExpoModuleTemplate/g, jsPackageName) - .replace(/ModuleTemplate/g, moduleNameWithoutExpoPrefix) + .replace(/ExpoModuleTemplate/g, moduleNameWithExpoPrefix) + .replace(/ModuleTemplate/g, moduleName) ); } @@ -293,15 +372,14 @@ async function configureNPM( modulePath: string, { npmModuleName, podName, jsPackageName }: ModuleConfiguration ) { - const moduleNameWithoutExpoPrefix = jsPackageName.startsWith('Expo') - ? jsPackageName.substr(4) - : 'Unimodule'; + const [, moduleName] = preparePrefixes(jsPackageName); + await replaceContent(path.join(modulePath, 'package.json'), singleFileContent => singleFileContent .replace(/expo-module-template/g, npmModuleName) .replace(/"version": "[\w.-]+"/, '"version": "1.0.0"') .replace(/ExpoModuleTemplate/g, jsPackageName) - .replace(/ModuleTemplate/g, moduleNameWithoutExpoPrefix) + .replace(/ModuleTemplate/g, moduleName) ); await replaceContent(path.join(modulePath, 'README.md'), readmeContent => readmeContent diff --git a/packages/expo-cli/src/commands/generate-module/fetchTemplate.ts b/packages/expo-cli/src/commands/generate-module/fetchTemplate.ts index b57f5f03b8..2f03aa2629 100644 --- a/packages/expo-cli/src/commands/generate-module/fetchTemplate.ts +++ b/packages/expo-cli/src/commands/generate-module/fetchTemplate.ts @@ -40,6 +40,6 @@ function isNpmPackage(template: string) { !template.match(/^_/) && // don't start with _ template.toLowerCase() === template && // only lowercase !/[~'!()*]/.test(template.split('/').slice(-1)[0]) && // don't contain any character from [~'!()*] - template.match(/^(@([^/]+?)\/)?([^/@]+)(@(\d\.\d\.\d)(-[^/@]+)?)?$/) // has shape (@scope/)?actual-package-name(@0.1.1(-tag.1)?)? + template.match(/^(@([^/]+?)\/)?([^/@]+)(@(((\d\.\d\.\d)(-[^/@]+)?)|latest|next))?$/) // has shape (@scope/)?actual-package-name(@0.1.1(-tag.1)?|tag-name)? ); } diff --git a/packages/expo-cli/src/commands/generate-module/promptQuestionsAsync.ts b/packages/expo-cli/src/commands/generate-module/promptQuestionsAsync.ts index da4c48c2d8..4e549f4a9d 100644 --- a/packages/expo-cli/src/commands/generate-module/promptQuestionsAsync.ts +++ b/packages/expo-cli/src/commands/generate-module/promptQuestionsAsync.ts @@ -17,7 +17,7 @@ const generateCocoaPodDefaultName = (moduleName: string) => { if (moduleName.toLowerCase().startsWith('expo')) { return `EX${wordsToUpperCase(moduleName.substring(4))}`; } - return wordsToUpperCase(moduleName); + return `EX${wordsToUpperCase(moduleName)}`; }; /**