fix: Eliminate dependencies on @nx/node in init generator (#196)

* Fix trailing comma in function template

* plugin init generator dependency fixes

* Do not import @nx/node in the init generator

* Fix unit tests

* Add .nx/cache to .gitignore

* stupid nx dependencies messing up e2e tests

.gitignore

@@ -40,3 +40,5 @@ Thumbs.db
 
 
 e2e.log
+
+.nx/cache

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)              

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": {

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 () => {

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 () => {

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'
@@ -19,13 +19,19 @@ export async function initGenerator(
   tree: Tree,
   rawOptions: InitGeneratorOptions,
 ): Promise<GeneratorCallback> {
+  // 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

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)
 }