From 45aedc032b4b25534e2884c60822c88ba9913a7d Mon Sep 17 00:00:00 2001 From: Simon M Date: Fri, 29 Mar 2024 00:58:26 +0000 Subject: [PATCH 1/2] Simplify init generator & cleanup unit & e2e tests --- e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts | 35 ------------------ .../src/generators/function/function.spec.ts | 11 ------ .../src/generators/init/init.spec.ts | 12 ------ .../nx-firebase/src/generators/init/init.ts | 28 +++----------- .../generators/init/lib/add-dependencies.ts | 37 +------------------ .../src/generators/init/lib/index.ts | 1 - .../generators/init/lib/normalize-options.ts | 10 ----- .../src/generators/init/schema.d.ts | 4 -- .../src/generators/init/schema.json | 19 +--------- 9 files changed, 9 insertions(+), 148 deletions(-) delete mode 100644 packages/nx-firebase/src/generators/init/lib/normalize-options.ts diff --git a/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts b/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts index 5d7a37d1..bac5f0a9 100644 --- a/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts +++ b/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts @@ -142,10 +142,6 @@ describe('nx-firebase e2e', () => { // should not be initially set const packageJson = readJson(`package.json`) expect(packageJson.devDependencies['@nx/node']).toBeUndefined() - expect(packageJson.devDependencies['@nx/esbuild']).toBeUndefined() - expect(packageJson.devDependencies['@nx/linter']).toBeUndefined() - expect(packageJson.devDependencies['@nx/js']).toBeUndefined() - expect(packageJson.devDependencies['@nx/jest']).toBeUndefined() }) it( @@ -163,16 +159,6 @@ describe('nx-firebase e2e', () => { expect(packageJson.devDependencies['firebase-tools']).toBeDefined() //SM: Mar'24: our plugin init generator now only add @nx/node expect(packageJson.devDependencies['@nx/node']).toBeDefined() - // @nx/node package brings in @nx/js - // https://github.com/nrwl/nx/blob/fb90767af87c77955f8b8b7cace7cd0b5e3be27d/packages/node/package.json#L32 - // not in 16.8.0 it doesnt - expect(packageJson.devDependencies['@nx/js']).not.toBeDefined() - // @nx/jest, @nx/eslint are package.json dependencies in later versions of Nx - expect(packageJson.devDependencies['@nx/eslint']).not.toBeDefined() - expect(packageJson.devDependencies['@nx/jest']).not.toBeDefined() - - //SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option - expect(packageJson.devDependencies['@nx/esbuild']).not.toBeDefined() }) }) @@ -271,19 +257,6 @@ describe('nx-firebase e2e', () => { validateProjectConfig(appData) - // // should still not see any nx/node related packages when generating an app - // // since we dont run the node generator in app generator - // const packageJson = readJson(`package.json`) - // // running the application generator runs the init generator, which adds @nx/node and these dependencies of @nx/node - // // SM: not in 16.8.0 it doesnt - // expect(packageJson.devDependencies['@nx/linter']).toBeDefined() - // expect(packageJson.devDependencies['@nx/eslint']).not.toBeDefined() - // // jest and js IS added somehow for 16.8.0 - // expect(packageJson.devDependencies['@nx/jest']).toBeDefined() - // expect(packageJson.devDependencies['@nx/js']).toBeDefined() - // //SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option - // expect(packageJson.devDependencies['@nx/esbuild']).not.toBeDefined() - // cleanup - app await cleanAppAsync(appData) }) @@ -396,14 +369,6 @@ describe('nx-firebase e2e', () => { validateFunctionConfig(functionData, appData) - // const packageJson = readJson(`package.json`) - // // running the function generator runs the node apo generator, which adds these dependencies of @nx/node - // expect(packageJson.devDependencies['@nx/eslint']).toBeDefined() - // expect(packageJson.devDependencies['@nx/jest']).toBeDefined() - // expect(packageJson.devDependencies['@nx/js']).toBeDefined() - // //SM: Mar'24: esbuild is added by @nx/node when functions are generated, depending on bundler option - // expect(packageJson.devDependencies['@nx/esbuild']).toBeDefined() - // cleanup await cleanFunctionAsync(functionData) await cleanAppAsync(appData) diff --git a/packages/nx-firebase/src/generators/function/function.spec.ts b/packages/nx-firebase/src/generators/function/function.spec.ts index 592ee3ee..91761534 100644 --- a/packages/nx-firebase/src/generators/function/function.spec.ts +++ b/packages/nx-firebase/src/generators/function/function.spec.ts @@ -9,7 +9,6 @@ import { import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing' import { functionGenerator } from './function' import applicationGenerator from '../application/application' -import { workspaceNxVersion } from '../../utils' describe('function generator', () => { let tree: Tree @@ -122,16 +121,6 @@ describe('function generator', () => { }), ) expect(project.targets.serve).toBeUndefined() - - // check that function generator ran the @nx/node init generator - // which adds the necessary dependencies for a node project - const packageJson = readJson(tree, 'package.json') - const nxVersion = workspaceNxVersion.version - expect(packageJson.devDependencies['@nx/linter']).toBe(nxVersion) - expect(packageJson.devDependencies['@nx/jest']).toBe(nxVersion) - expect(packageJson.devDependencies['@nx/esbuild']).toBe(nxVersion) - expect(packageJson.devDependencies['@nx/js']).toBe(nxVersion) - }) it('should update tags', async () => { diff --git a/packages/nx-firebase/src/generators/init/init.spec.ts b/packages/nx-firebase/src/generators/init/init.spec.ts index c2cd52c6..e32b8757 100644 --- a/packages/nx-firebase/src/generators/init/init.spec.ts +++ b/packages/nx-firebase/src/generators/init/init.spec.ts @@ -119,8 +119,6 @@ describe('init generator', () => { expect(packageJson.dependencies['firebase']).toBe(testVersion) expect(packageJson.dependencies['firebase-admin']).toBe(testVersion) expect(packageJson.dependencies['firebase-functions']).toBe(testVersion) - // expect(packageJson.dependencies['tslib']).toBe(testVersion) - expect(packageJson.devDependencies['firebase-functions-test']).toBe( testVersion, ) @@ -131,16 +129,6 @@ describe('init generator', () => { expect(packageJson.devDependencies['@nx/node']).toBe(nxVersion) }) - //SM: Mar'24 - init generator does not add jest config - // it('should add jest config when unitTestRunner is jest', async () => { - // await initGenerator(tree, { unitTestRunner: 'jest' }) - // expect(tree.exists('jest.config.ts')).toBe(true) - // }) - - // it('should not add jest config when unitTestRunner is none', async () => { - // await initGenerator(tree, { unitTestRunner: 'none' }) - // expect(tree.exists('jest.config.ts')).toBe(false) - // }) // describe('--skipFormat', () => { // it('should format files by default', async () => { diff --git a/packages/nx-firebase/src/generators/init/init.ts b/packages/nx-firebase/src/generators/init/init.ts index d863c326..43e89378 100644 --- a/packages/nx-firebase/src/generators/init/init.ts +++ b/packages/nx-firebase/src/generators/init/init.ts @@ -1,39 +1,23 @@ import type { GeneratorCallback, Tree } from '@nx/devkit' -import { convertNxGenerator, runTasksInSerial } from '@nx/devkit' -// import { initGenerator as nodeInitGenerator } from '@nx/node' -import { addDependencies, normalizeOptions } from './lib' +import { addDependencies } from './lib' import { addGitIgnore, addNxIgnore } from './lib/add-git-ignore-entry' import type { InitGeneratorOptions } from './schema' /** - * `nx g @simondotm/nx-firebase:init` is based on the `@nx/nest` plugin - * which in turn is based on the `@nx/node` plugin - * + * `nx g @simondotm/nx-firebase:init` + * * Ensures the necessary firebase packages are installed in the nx workspace - * The `@nx/node` init generate also ensures jest configs + * It also adds `@nx/node` if it is not already installed * - * Docs say its for internal use only, but nest uses it, so we use it :) - * https://nx.dev/packages/node/generators/init */ export async function initGenerator( tree: Tree, - rawOptions: InitGeneratorOptions, + _options: InitGeneratorOptions, ): Promise { - // console.log('initGenerator') - const tasks: GeneratorCallback[] = [] - const options = normalizeOptions(rawOptions) - tasks.push(addDependencies(tree)) addGitIgnore(tree) addNxIgnore(tree) - - // no need to run the node init generator here. - // that will happen when a firebase app is generated - // console.log('running nodeInitGenerator') - // const nodeInitTask = await nodeInitGenerator(tree, options) - // tasks.push(nodeInitTask) - return runTasksInSerial(...tasks) + return addDependencies(tree) } export default initGenerator -export const initSchematic = convertNxGenerator(initGenerator) diff --git a/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts b/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts index 7ed0cc18..02292757 100644 --- a/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts +++ b/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts @@ -6,7 +6,6 @@ import { } from '@nx/devkit' import { workspaceNxVersion } from '../../../utils' import { - // tsLibVersion, firebaseVersion, firebaseAdminVersion, firebaseFunctionsVersion, @@ -23,23 +22,15 @@ export function addDependencies(tree: Tree): GeneratorCallback { // This is atypical for Nx plugins that usually migrate versions automatically // however the nx-firebase plugin is not (currently) opinionated about which version is needed, // so this ensures workspaces retain control over their firebase versions. - const packageJson = readJson(tree, 'package.json') //readRootPackageJson() + const packageJson = readJson(tree, 'package.json') function addDependencyIfNotPresent( packageName: string, packageVersion: string, ) { if (!packageJson.dependencies || !packageJson.dependencies[packageName]) { - // console.log('adding dependency', packageName, packageVersion) dependencies[packageName] = packageVersion } - // else { - // console.log( - // 'dependency already exists', - // packageName, - // packageJson.dependencies[packageName], - // ) - // } } function addDevDependencyIfNotPresent( packageName: string, @@ -49,16 +40,8 @@ export function addDependencies(tree: Tree): GeneratorCallback { !packageJson.devDependencies || !packageJson.devDependencies[packageName] ) { - // console.log('adding dev dependency', packageName, packageVersion) devDependencies[packageName] = packageVersion } - // else { - // console.log( - // 'dev dependency already exists', - // packageName, - // packageJson.devDependencies[packageName], - // ) - // } } // firebase dependencies @@ -80,26 +63,10 @@ export function addDependencies(tree: Tree): GeneratorCallback { // since Nx doesn't kill processes cleanly atm addDevDependencyIfNotPresent('kill-port', killportVersion) - // TODO: find out if Nx devkit adds these versions even if they already exist - // for now, only add them if they aren't in the workspace already at the same version as the host workspace - // from: - // https://github.com/nrwl/nx/blob/5b7dba1cb78cabcf631129b4ce8163406b9c1328/packages/devkit/src/utils/package-json.ts#L84 - // - - // These dependencies are required by the plugin internals, most likely already in the host workspace - // but add them if not. They are added with the same version that the host workspace is using. - // This is cleaner than using peerDeps. - // SM Dec'23: @nx/devkit is a peer dependency now. No need to install via plugin. - // addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version) - - // console.log('workspaceNxVersion', workspaceNxVersion) - // @nx/node is used by the plugin function generator as a proxy for creating a typescript app // since users have to create a firebase app before they generate a function, we can be sure // this plugin init will have been run before the function generator that requires @nx/node is used + // we defer to @nx/node to install its own plugins such as @nx/eslint, @nx/jest, @nx/js, @nx/esbuild, @nx/webpack etc. addDevDependencyIfNotPresent('@nx/node', workspaceNxVersion.version) - - // @nx/node has @nx/eslint, @nx/jest, @nx/js as dependencies, so they will come with @nx/node - // @nx/node plugin initialiser will install @nx/esbuild or @nx/webpack depending on the bundler option used return addDependenciesToPackageJson(tree, dependencies, devDependencies) } diff --git a/packages/nx-firebase/src/generators/init/lib/index.ts b/packages/nx-firebase/src/generators/init/lib/index.ts index 51775ca4..8f1475d8 100644 --- a/packages/nx-firebase/src/generators/init/lib/index.ts +++ b/packages/nx-firebase/src/generators/init/lib/index.ts @@ -1,3 +1,2 @@ export * from './add-dependencies' -export * from './normalize-options' export * from './add-git-ignore-entry' diff --git a/packages/nx-firebase/src/generators/init/lib/normalize-options.ts b/packages/nx-firebase/src/generators/init/lib/normalize-options.ts deleted file mode 100644 index abecbef1..00000000 --- a/packages/nx-firebase/src/generators/init/lib/normalize-options.ts +++ /dev/null @@ -1,10 +0,0 @@ -import type { InitGeneratorOptions } from '../schema' - -export function normalizeOptions( - options: InitGeneratorOptions, -): InitGeneratorOptions { - return { - ...options, - unitTestRunner: options.unitTestRunner ?? 'jest', - } -} diff --git a/packages/nx-firebase/src/generators/init/schema.d.ts b/packages/nx-firebase/src/generators/init/schema.d.ts index 07567cdf..e106bbc8 100644 --- a/packages/nx-firebase/src/generators/init/schema.d.ts +++ b/packages/nx-firebase/src/generators/init/schema.d.ts @@ -1,6 +1,2 @@ export interface InitGeneratorOptions { - unitTestRunner?: 'jest' | 'none' - skipFormat?: boolean - js?: boolean - rootProject?: boolean } diff --git a/packages/nx-firebase/src/generators/init/schema.json b/packages/nx-firebase/src/generators/init/schema.json index dcc0b4ae..e8a74c1a 100644 --- a/packages/nx-firebase/src/generators/init/schema.json +++ b/packages/nx-firebase/src/generators/init/schema.json @@ -4,23 +4,6 @@ "title": "Init Firebase Plugin", "description": "Init Firebase Plugin.", "type": "object", - "properties": { - "unitTestRunner": { - "description": "Adds the specified unit test runner.", - "type": "string", - "enum": ["jest", "none"], - "default": "jest" - }, - "skipFormat": { - "description": "Skip formatting files.", - "type": "boolean", - "default": false - }, - "js": { - "type": "boolean", - "default": false, - "description": "Use JavaScript instead of TypeScript" - } - }, + "properties": {}, "required": [] } From 13e35e0d90e907bec8f53bd28e26c7d88b125d9b Mon Sep 17 00:00:00 2001 From: Simon M Date: Fri, 29 Mar 2024 01:06:06 +0000 Subject: [PATCH 2/2] lint fix --- packages/nx-firebase/src/generators/init/init.ts | 2 +- packages/nx-firebase/src/generators/init/schema.d.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/nx-firebase/src/generators/init/init.ts b/packages/nx-firebase/src/generators/init/init.ts index 43e89378..8393ce6e 100644 --- a/packages/nx-firebase/src/generators/init/init.ts +++ b/packages/nx-firebase/src/generators/init/init.ts @@ -12,7 +12,7 @@ import type { InitGeneratorOptions } from './schema' */ export async function initGenerator( tree: Tree, - _options: InitGeneratorOptions, + options: InitGeneratorOptions, ): Promise { addGitIgnore(tree) addNxIgnore(tree) diff --git a/packages/nx-firebase/src/generators/init/schema.d.ts b/packages/nx-firebase/src/generators/init/schema.d.ts index e106bbc8..697169b7 100644 --- a/packages/nx-firebase/src/generators/init/schema.d.ts +++ b/packages/nx-firebase/src/generators/init/schema.d.ts @@ -1,2 +1 @@ -export interface InitGeneratorOptions { -} +export type InitGeneratorOptions = unknown