From e37ce9877ab4f32c733df3917439af3ba5839cb8 Mon Sep 17 00:00:00 2001 From: Simon M Date: Tue, 19 Dec 2023 11:18:05 +0000 Subject: [PATCH 1/6] Fix trailing comma in function template --- .../src/generators/function/files/package.json__tmpl__ | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nx-firebase/src/generators/function/files/package.json__tmpl__ b/packages/nx-firebase/src/generators/function/files/package.json__tmpl__ index dc2b9a47..07132205 100644 --- a/packages/nx-firebase/src/generators/function/files/package.json__tmpl__ +++ b/packages/nx-firebase/src/generators/function/files/package.json__tmpl__ @@ -5,7 +5,7 @@ "test": "nx test <%= projectName %>", "lint": "nx lint <%= projectName %>", "build": "nx build <%= projectName %>", - "deploy": "nx deploy <%= projectName %>", + "deploy": "nx deploy <%= projectName %>" }, "type": "<%= moduleType %>", "engines": { From cbc039ae979ad7edeadc7c006186f04f4162ffeb Mon Sep 17 00:00:00 2001 From: Simon M Date: Tue, 26 Mar 2024 08:34:24 +0000 Subject: [PATCH 2/6] plugin init generator dependency fixes --- .../nx-firebase/src/generators/init/init.ts | 12 ++++-- .../generators/init/lib/add-dependencies.ts | 38 +++++++++++++++---- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/packages/nx-firebase/src/generators/init/init.ts b/packages/nx-firebase/src/generators/init/init.ts index 8b35aeee..828357a6 100644 --- a/packages/nx-firebase/src/generators/init/init.ts +++ b/packages/nx-firebase/src/generators/init/init.ts @@ -19,13 +19,19 @@ export async function initGenerator( tree: Tree, rawOptions: InitGeneratorOptions, ): Promise { + // console.log('initGenerator') + const tasks: GeneratorCallback[] = [] const options = normalizeOptions(rawOptions) - const nodeInitTask = await nodeInitGenerator(tree, options) - const installPackagesTask = addDependencies(tree) + tasks.push(addDependencies(tree)) addGitIgnore(tree) addNxIgnore(tree) - return runTasksInSerial(nodeInitTask, installPackagesTask) + // 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) } export default 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 886eba61..7ed0cc18 100644 --- a/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts +++ b/packages/nx-firebase/src/generators/init/lib/add-dependencies.ts @@ -29,17 +29,36 @@ export function addDependencies(tree: Tree): GeneratorCallback { packageName: string, packageVersion: string, ) { - if (!packageJson.dependencies[packageName]) { + 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, packageVersion: string, ) { - if (!packageJson.devDependencies[packageName]) { + if ( + !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 @@ -70,14 +89,17 @@ export function addDependencies(tree: Tree): GeneratorCallback { // 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. - addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version) + // SM Dec'23: @nx/devkit is a peer dependency now. No need to install via plugin. + // addDevDependencyIfNotPresent('@nx/devkit', workspaceNxVersion.version) - // used by the plugin function generator as a proxy for creating a typescript app + // 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 addDevDependencyIfNotPresent('@nx/node', workspaceNxVersion.version) - addDevDependencyIfNotPresent('@nx/linter', workspaceNxVersion.version) - addDevDependencyIfNotPresent('@nx/jest', workspaceNxVersion.version) - addDevDependencyIfNotPresent('@nx/esbuild', workspaceNxVersion.version) - addDevDependencyIfNotPresent('@nx/js', 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) } From 7bc203bb98e0485f8314c13e8d5a819f6a3dd401 Mon Sep 17 00:00:00 2001 From: Simon M Date: Wed, 27 Mar 2024 09:07:50 +0000 Subject: [PATCH 3/6] Do not import @nx/node in the init generator --- packages/nx-firebase/src/generators/init/init.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nx-firebase/src/generators/init/init.ts b/packages/nx-firebase/src/generators/init/init.ts index 828357a6..d863c326 100644 --- a/packages/nx-firebase/src/generators/init/init.ts +++ b/packages/nx-firebase/src/generators/init/init.ts @@ -1,6 +1,6 @@ import type { GeneratorCallback, Tree } from '@nx/devkit' import { convertNxGenerator, runTasksInSerial } from '@nx/devkit' -import { initGenerator as nodeInitGenerator } from '@nx/node' +// import { initGenerator as nodeInitGenerator } from '@nx/node' import { addDependencies, normalizeOptions } from './lib' import { addGitIgnore, addNxIgnore } from './lib/add-git-ignore-entry' import type { InitGeneratorOptions } from './schema' From 51a7d97c39d228df19198ab610c8585749c2bb6c Mon Sep 17 00:00:00 2001 From: Simon M Date: Thu, 28 Mar 2024 10:38:11 +0000 Subject: [PATCH 4/6] Fix unit tests --- .../src/generators/function/function.spec.ts | 11 ++++++++ .../src/generators/init/init.spec.ts | 27 +++++++++---------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/packages/nx-firebase/src/generators/function/function.spec.ts b/packages/nx-firebase/src/generators/function/function.spec.ts index 91761534..592ee3ee 100644 --- a/packages/nx-firebase/src/generators/function/function.spec.ts +++ b/packages/nx-firebase/src/generators/function/function.spec.ts @@ -9,6 +9,7 @@ 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 @@ -121,6 +122,16 @@ 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 1c1ff8c4..c2cd52c6 100644 --- a/packages/nx-firebase/src/generators/init/init.spec.ts +++ b/packages/nx-firebase/src/generators/init/init.spec.ts @@ -84,7 +84,6 @@ describe('init generator', () => { expect(packageJson.dependencies['firebase-functions']).toBe( firebaseFunctionsVersion, ) - expect(packageJson.dependencies['tslib']).toBeDefined() expect(packageJson.devDependencies['firebase-functions-test']).toBe( firebaseFunctionsTestVersion, @@ -93,6 +92,9 @@ describe('init generator', () => { firebaseToolsVersion, ) expect(packageJson.devDependencies['kill-port']).toBe(killportVersion) + + expect(packageJson.dependencies['tslib']).not.toBeDefined() + }) it('should only add dependencies if not already present', async () => { @@ -127,23 +129,18 @@ describe('init generator', () => { const nxVersion = workspaceNxVersion.version expect(packageJson.devDependencies['@nx/node']).toBe(nxVersion) - 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 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' }) + //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) + // }) - expect(tree.exists('jest.config.ts')).toBe(false) - }) + // 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 () => { From d981fa1acc3fa5295f8c13efa6c4c53b44721261 Mon Sep 17 00:00:00 2001 From: Simon M Date: Thu, 28 Mar 2024 12:54:42 +0000 Subject: [PATCH 5/6] Add .nx/cache to .gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index a061de7d..981c4504 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,5 @@ Thumbs.db e2e.log + +.nx/cache From 68a13251a16c45256f3c6963c46d76670e9cb287 Mon Sep 17 00:00:00 2001 From: Simon M Date: Thu, 28 Mar 2024 12:55:23 +0000 Subject: [PATCH 6/6] stupid nx dependencies messing up e2e tests --- e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts b/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts index b93ae0a1..5d7a37d1 100644 --- a/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts +++ b/e2e/nx-firebase-e2e/tests/nx-firebase.spec.ts @@ -161,11 +161,18 @@ describe('nx-firebase e2e', () => { packageJson.devDependencies['firebase-functions-test'], ).toBeDefined() expect(packageJson.devDependencies['firebase-tools']).toBeDefined() + //SM: Mar'24: our plugin init generator now only add @nx/node expect(packageJson.devDependencies['@nx/node']).toBeDefined() - expect(packageJson.devDependencies['@nx/esbuild']).toBeDefined() - expect(packageJson.devDependencies['@nx/linter']).toBeDefined() - expect(packageJson.devDependencies['@nx/js']).toBeDefined() - expect(packageJson.devDependencies['@nx/jest']).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() }) }) @@ -264,6 +271,19 @@ 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) }) @@ -376,6 +396,14 @@ 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)